Interest in enabling -Werror by default

Hi,

It seems that enabling -Werror by default is within reach for lldb now. There currently are three warnings that remain with gcc 5.1 on Linux, and the build is clean of warnings with clang.

There are two instances of type range limitations on comparisons in asserts, and one instance of string formatting which has a GNU incompatibility.

Is there any interest in enabling -Werror by default to help keep the build clean going forward?

I would be happy if we can keep lldb warning free but I don’t think enabling -Werror is a good idea for 2 reasons:

  • We are using a lot of different compiler and keeping the codebase warning free on all of them might not be feasible especially for the less used, older gcc versions.
  • Neither llvm nor clang have -Werror enabled so if we enable it then a clang/llvm change can break our build with a warning when it is hard to justify a revert and a fix might not be trivial.

In short term I would prefer to just create a policy saying everybody should write warning free code for lldb (I think it already kind of exists) and we as a community try to ensure it during code review and with fixing the possible things what slip through. In the longer term I would be happy to see -Werror turned on for llvm and clang first and then we can follow up with lldb but making this change will require a lot of discussion and might get some push back.

If a developer adds a new warning to Clang (or enhances an existing
one), and this breaks the LLDB build - whose responsibility is it to
fix it?

Alex

I would be happy if we can keep lldb warning free but I don't think enabling
-Werror is a good idea for 2 reasons:
* We are using a lot of different compiler and keeping the codebase warning
free on all of them might not be feasible especially for the less used,
older gcc versions.
* Neither llvm nor clang have -Werror enabled so if we enable it then a
clang/llvm change can break our build with a warning when it is hard to
justify a revert and a fix might not be trivial.

In short term I would prefer to just create a policy saying everybody should
write warning free code for lldb (I think it already kind of exists) and we
as a community try to ensure it during code review and with fixing the
possible things what slip through. In the longer term I would be happy to
see -Werror turned on for llvm and clang first and then we can follow up
with lldb but making this change will require a lot of discussion and might
get some push back.

Enabling -Werror for LLVM and Clang by default for everyone is
generally not an option. LLVM gets built by many users with new
compilers, old compilers and broken compilers that we have never heard
about. Breaking their build just because the compiler decided to emit
a warning provides no value, only pain. I'm happy with forcing -Werror
on developers, but I don't know how to express that in the build
system.

I would be happy if we can keep lldb warning free but I don’t think enabling -Werror is a good idea for 2 reasons:

  • We are using a lot of different compiler and keeping the codebase warning free on all of them might not be feasible especially for the less used, older gcc versions.
  • Neither llvm nor clang have -Werror enabled so if we enable it then a clang/llvm change can break our build with a warning when it is hard to justify a revert and a fix might not be trivial.

Err, sorry. I meant by default on the build bots (IIRC, some (many?) of the build bots do build with -Werror for LLVM and clang). Yes, a new warning in clang could cause issues in LLDB, though the same thing exists for the LLVM/clang dependency. Since this would be on the build bots, it should get resolved rather quickly.

If you want to enable it only on the bots then I think we can decide it on a bot by bot bases. For me the main question is who will be responsible for fixing a warning introduced by a change in llvm or clang causing a build failure because of a warning (especially when the fix is non trivial)?

If you want to enable it only on the bots then I think we can decide it on a bot by bot bases. For me the main question is who will be responsible for fixing a warning introduced by a change in llvm or clang causing a build failure because of a warning (especially when the fix is non trivial)?

I think that the same policy as LLVM/clang should apply here. The person making the change would be responsible for ensuring that nothing breaks as a result of their change. The same situation exists when working on interfaces that effect clang: a fix for a warning introduced by a change in LLVM may be non-trivial in clang.

Just to be clear, I’m merely suggesting this as an option. If it is deemed too burdensome by most of the common committers, we state so and not do this.

You’re talking about doing it on a per-bot basis and not a global policy, but just throwing in that on the MSVC side at least, we’re not warning free right now and it’s not trivial tog et warning free without disabling some warnings (which I don’t want to do either)

NetBSD builds with GCC 4.8.2 and it emits few warnings for LLDB.

Before enabling -Werror please first iterate over build logs and help
to squash them. For example it detects undefined behavior IIRC for a
Darwin code part.

You're talking about doing it on a per-bot basis and not a global policy,
but just throwing in that on the MSVC side at least, we're not warning free
right now and it's not trivial tog et warning free without disabling some
warnings (which I don't want to do either)

Correct, it would be enabling it on a per-bot basis. We could do it such
that we only enable it on bots that run a certain GCC version or newer. If
at some point we become warning free on MSVC, and decide that we want to
enable it on the bots so that people not building with MSVC normally know
that they have introduced a warning, we could do so at that time under MSVC.

Im just wondering if there is any interest in this at all. If there is, we
can do this as logistically possible. It doesn't have to be all or nothing.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

NetBSD builds with GCC 4.8.2 and it emits few warnings for LLDB.

Before enabling -Werror please first iterate over build logs and help
to squash them. For example it detects undefined behavior IIRC for a
Darwin code part.

Interesting. On Linux, lldb had many warnings, and over time, I've managed
to get mots of them cleaned up. Right now, there are a couple of
-Wtype-limits warnings and one -Wformat warning. Is there a build bot that
can be used to monitor what those warnings are? If there aren't any
buildbots, then this would be of no consequence since we wouldn't turn it
on for user builds.

I wish I had caught what I wrote versus what I was thinking before hitting
send :-(.

I think the Linux-x86_64 build using clang is mostly warning free (1 warning on http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake) but it isn’t true for most of the other configuration.

I think -Werror can be enabled on the buildbots on a case by case bases depending on the decision of the owner/maintainer. The main reason I think it this way because a change like this will increase the number of build failures what will give more work to the buildbot maintainer primarily because most buildbot don’t send out failure messages (flakiness) and I am not convinced that the community will fix some warning based on a report from a build bot.

As a partial maintainer of 5 different buildbots I don’t want to enable -Werror on any them as I think it will be too much additional maintenance work compared to the benefit unless we enforce -Werror on local builds as well (e.g. use -Werror if compiling with clang on an x86_64 platform).