Enabling warnings as errors

What would it take to enable warnings as errors on all platforms?

I’ve done alot of work getting the windows build warning free, but it seems like very time I sync the code on linux or Mac, there’s many many new warnings introduced. I’m always vigilant about fixing warnings in my own code, it would be nice if everyone else had the same level of vigilance. If warnings broke the build, then I think that would be a good motivator to fix your warnings.

Thoughts?

I feel conflicted about this. On the one hand, lldb's build has
significantly more warnings than LLVM and Clang, so it would be great
to clean this up. On the other hand, I've been bitten by libraries
that have this enabled in release builds because compilers introduce
new warnings. I think I would be fine with this if it were not enabled
in tagged releases (not sure how to do that though).

There are so many compilers and each compiler is good at detecting things that other compilers don't warn about.

So I don't think we should enable warnings as errors, but I do think we should strive to keep warnings down to as low a level as possible.

Greg

There are alot of compilers, but only a small number that we support building with. And really, the burden would only be on making sure that the compiler you personally build with builds warning free (which, unfortunately, even that doesn’t always happen). If someone checks something in on Mac that introduces warnings on Windows with MSVC, then a Windows person would need to fix it, but I don’t think that’s too big of a deal (well, I don’t mind anyway). The difference is just that “strive” would become “require”. Sometimes you can’t fix warnings for whatever reason, but you could still disable them in that case.

I guess in an ideal world I would agree with you, that we should strive to keep warnings down to as low a level as possible. But in practice I think it’s too easy to ignore them, and so people do.

Right now llvm does this just via a bot that checks for errors and leaves it up to the developers to get it clean. It would definitely be nice to get lldb at least clean with clang and the warning options from llvm and I’ll check in (or send out if they’re less obvious) a few patches to do that shortly.

-eric

There are so many compilers and each compiler is good at detecting things
that other compilers don't warn about.

You can easily disable warnings that don't have value. We do this all the
time in LLVM.

So I don't think we should enable warnings as errors, but I do think we
should strive to keep warnings down to as low a level as possible.

I think there should be a WERROR mode in the CMake build, and build bots
should run with it. That way releases etc aren't impacted by new warnings,
but developers *do* notice when they introduce a warning.

In the clang/llvm world, I often get a small handful of warnings when building locally, almost always due to my local compiler having buggy warnings. If -Werror was enabled by default in the clang/llvm build, I’d have a broken build fairly often.

Maybe we could detect compiler versions and only enable -Werror for compilers that are in some whitelist.

My vote is for this:

I think there should be a WERROR mode in the CMake build, and build bots should run with it. That way releases etc aren’t impacted by new warnings, but developers do notice when they introduce a warning.

In particular, warning with clang.

With the caveat of this:

You can easily disable warnings that don’t have value. We do this all the time in LLVM.

where there are likely a handful of warnings I’d definitely vote for turning off (like the full-coverage-enum case with default present). We can discuss these as we break warnings that we think are worthwhile (and fix them) or not (and then disable the warning).

-Todd