6 ms·

> Enabling -Werror in distributions virtually guarantees that they won't build: Even if you took care of all compiler warnings in all compilers for which you en

> Enabling -Werror in distributions virtually guarantees that they won't build: Even if you took care of all compiler warnings in all compilers for which you enable this switch, it's just a question of time till a new compiler version comes along that has a new warning message and - BOOM. This is a feature. This triggers a review of the new warnings introduced by the new compiler version, to see if they have found bugs, or should be squelched per false positive, or should be squelched project-wide. > Two recent examples are squid and qemu-linaro, two packages I had to compile from source as Debian GNU/Linux didn't have (good enough, or any) packages for them. The fundamental problem here is using a contributor's build environment (where I want -Werror to get them to contribute warning-induced bugfixes, or squelches/ignores if they're really false positives) for a non-contributor's task (where they just want the final binary, and aren't going to bother reporting the warnings even if I want them.) Non-contributors building from source isn't a relevant niche to me (I generally ship binaries), but I can sympathize with the request for a warnings-as-warnings fast build path for them. But make -Werror the default build path, for developers. This needs to be opt-out, not opt-in.

andrewchambers 10y ago
> This is a feature. This triggers a review of the new warnings introduced by the new compiler version, to see if they have found bugs, or should be squelched per false positive, or should be squelched project-wide. The thing is, the software already had these bugs, and worked well enough for most people despite them, its not worth breaking the build over unless you are the developer.
- brian-armstrong 10y ago
  Those bugs could be buffer overruns or similar security problems. Just because the program appears to work doesn't mean it should be used.
  - andrewchambers 10y ago
    They could be, or they could be nothing, and the program might not even be exposed to untrusted input anyway. We can have a discussion about perhaps splitting programs into security classes, though I would prefer the upstream people use the cutting edge security analysis, then backport patches with security advisories.
csl 10y ago
It might be okay for your project, but there are many cases where warnings seriously get in the way (e.g., indirect throw/longjump but leaving out a return statement). Besides, the warnings depend on the compiler (which may be non-gcc/clang/vs) — and may not even make sense on a given system. So shipping with -Werror is not really a good idea. Put the important stuff in the tests.
- Nullabillity 10y ago
  > indirect throw/longjump but leaving out a return statement Hopefully a modern language would have a divergence type.
- MaulingMonkey 10y ago
  > It might be okay for your project, but there are many cases where warnings seriously get in the way (e.g., indirect throw/longjump but leaving out a return statement). I'd prefer to explicitly annotate the indirectly throwing function as non-returning, when possible. Longjumps are rare enough for me I'd be willing to individually suppress them, or suppress them over a range of code where they happen to be common. I've seen return warnings catch enough bugs that I believe this will save more time than it spends in the long run. If I didn't, I'd explicitly lower that individual warning's severity. I will also lower the warning's severity on a given compiler, if (but only if) I can't properly annotate it away on a given compiler. > Besides, the warnings depend on the compiler (which may be non-gcc/clang/vs) Again, feature. More coverage. Good. > and may not even make sense on a given system. In which case, upon reviewing said warning and determining said warning indeed does not make sense, said warning can be individually suppressed as mentioned. > Put the important stuff in the tests. This is a statically typed language. Successfully compiling is the first test. Successfully passing static analysis is the second test. Undefined behavior bugs can lead to some very subtle behavior that can be very difficult to unit test for, and tends to be quite sensitive to things like compiler flags, which call site they were inlined at, allocation patterns, etc. to even manifest in the first place. Given how nasty they can be to debug, they're also some of the bugs I'm most interested in catching - ideally before I commit, definitely before I ship. Taking full advantage of compiler diagnostics is way easier than e.g. writing enough unit tests to catch all UB caused from falling off the end of a non-void function without a return statement.
skybrian 10y ago
This all depends on how much your compiler changes and how often you upgrade the compiler. Do you fix all the new warnings before upgrading? If it's a large codebase, that can take a while. Meanwhile your continuous build can't use the new compiler. (Or in practice, that's when you stop using something like -Werror and start disabling the new warnings.) For most open source projects, it's probably not that big a deal. But for something large like a monorepo, we're talking about hundreds of millions of lines of code that worked before. People should be able to use the new compiler while someone goes back and looks at the new warnings. Assuming it's a priority. This task isn't automatically more important than other work.
- MaulingMonkey 10y ago
  > But for something large like a monorepo, we're talking about hundreds of millions of lines of code that worked before. Unless you're also monobuild (a nightmare at 100 MLOC no matter how you slice it, although I've heard enough horror stories that I bet such exists), you don't need to upgrade every project at the same time. And if you can't afford the dev time to fix that many warnings, you probably can't afford the QA time to verify the compiler upgrade didn't introduce any "new" bugs from e.g. previously benign undefined behavior that the optimizer can take advantage of now in new optimizations. Although this presupposes you've got decent control over pinning your compiler versions. My experience has been that compiler upgrades generally don't introduce too many new warnings - I'll generally fix all the new warnings before switching over the CI servers, yes, although I've only dealt with <1MLOC or so at a time. New compilers may introduce too many warnings to fix all at once, in which case I may temporarily squelch or downgrade the specific "lower priority" warning types (read: those that seem to be mostly false positives) and establish corresponding work items for going through the codebase to fix those individual warnings and unsquelching the warning type, as a means to unblock whatever high priority thing is requiring the new compiler in the first place. ...in other words, it may be sufficient in the short term to review the new types of warnings, which is mostly independent of codebase size, if only to create a backlog to prioritize and triage. And hey, maybe each new warning type is useless, in which case you've found a bunch of warnings to disable so they don't distract your fellow developers.
gkya 10y ago
The post agrees your comment: "Now, some people (and I feel this group is growing) are going a step too far, by enabling -Werror __unconditionally__ in their software packages. And what applies to me - not being able to anticipate warnings in other compilers - of course applies to them as well" "squid requires me to __doctor around in its configure__ script, and even __patch some Makefiles__, while qemu-linaro has a central location for it." The complaint boils down to "-Werror is a development tool, so use it like that".
mkj 10y ago
Sure, but do you opt-in or opt-out of being a developer versus just someone trying to build a package?

andrewchambers 10y ago

> This is a feature. This triggers a review of the new warnings introduced by the new compiler version, to see if they have found bugs, or should be squelched per false positive, or should be squelched project-wide. The thing is, the software already had these bugs, and worked well enough for most people despite them, its not worth breaking the build over unless you are the developer.

brian-armstrong 10y ago
Those bugs could be buffer overruns or similar security problems. Just because the program appears to work doesn't mean it should be used.
- andrewchambers 10y ago
  They could be, or they could be nothing, and the program might not even be exposed to untrusted input anyway. We can have a discussion about perhaps splitting programs into security classes, though I would prefer the upstream people use the cutting edge security analysis, then backport patches with security advisories.

csl 10y ago

It might be okay for your project, but there are many cases where warnings seriously get in the way (e.g., indirect throw/longjump but leaving out a return statement). Besides, the warnings depend on the compiler (which may be non-gcc/clang/vs) — and may not even make sense on a given system. So shipping with -Werror is not really a good idea. Put the important stuff in the tests.

Nullabillity 10y ago
> indirect throw/longjump but leaving out a return statement Hopefully a modern language would have a divergence type.
MaulingMonkey 10y ago
> It might be okay for your project, but there are many cases where warnings seriously get in the way (e.g., indirect throw/longjump but leaving out a return statement). I'd prefer to explicitly annotate the indirectly throwing function as non-returning, when possible. Longjumps are rare enough for me I'd be willing to individually suppress them, or suppress them over a range of code where they happen to be common. I've seen return warnings catch enough bugs that I believe this will save more time than it spends in the long run. If I didn't, I'd explicitly lower that individual warning's severity. I will also lower the warning's severity on a given compiler, if (but only if) I can't properly annotate it away on a given compiler. > Besides, the warnings depend on the compiler (which may be non-gcc/clang/vs) Again, feature. More coverage. Good. > and may not even make sense on a given system. In which case, upon reviewing said warning and determining said warning indeed does not make sense, said warning can be individually suppressed as mentioned. > Put the important stuff in the tests. This is a statically typed language. Successfully compiling is the first test. Successfully passing static analysis is the second test. Undefined behavior bugs can lead to some very subtle behavior that can be very difficult to unit test for, and tends to be quite sensitive to things like compiler flags, which call site they were inlined at, allocation patterns, etc. to even manifest in the first place. Given how nasty they can be to debug, they're also some of the bugs I'm most interested in catching - ideally before I commit, definitely before I ship. Taking full advantage of compiler diagnostics is way easier than e.g. writing enough unit tests to catch all UB caused from falling off the end of a non-void function without a return statement.

skybrian 10y ago

This all depends on how much your compiler changes and how often you upgrade the compiler. Do you fix all the new warnings before upgrading? If it's a large codebase, that can take a while. Meanwhile your continuous build can't use the new compiler. (Or in practice, that's when you stop using something like -Werror and start disabling the new warnings.) For most open source projects, it's probably not that big a deal. But for something large like a monorepo, we're talking about hundreds of millions of lines of code that worked before. People should be able to use the new compiler while someone goes back and looks at the new warnings. Assuming it's a priority. This task isn't automatically more important than other work.

MaulingMonkey 10y ago
> But for something large like a monorepo, we're talking about hundreds of millions of lines of code that worked before. Unless you're also monobuild (a nightmare at 100 MLOC no matter how you slice it, although I've heard enough horror stories that I bet such exists), you don't need to upgrade every project at the same time. And if you can't afford the dev time to fix that many warnings, you probably can't afford the QA time to verify the compiler upgrade didn't introduce any "new" bugs from e.g. previously benign undefined behavior that the optimizer can take advantage of now in new optimizations. Although this presupposes you've got decent control over pinning your compiler versions. My experience has been that compiler upgrades generally don't introduce too many new warnings - I'll generally fix all the new warnings before switching over the CI servers, yes, although I've only dealt with <1MLOC or so at a time. New compilers may introduce too many warnings to fix all at once, in which case I may temporarily squelch or downgrade the specific "lower priority" warning types (read: those that seem to be mostly false positives) and establish corresponding work items for going through the codebase to fix those individual warnings and unsquelching the warning type, as a means to unblock whatever high priority thing is requiring the new compiler in the first place. ...in other words, it may be sufficient in the short term to review the new types of warnings, which is mostly independent of codebase size, if only to create a backlog to prioritize and triage. And hey, maybe each new warning type is useless, in which case you've found a bunch of warnings to disable so they don't distract your fellow developers.

gkya 10y ago

The post agrees your comment: "Now, some people (and I feel this group is growing) are going a step too far, by enabling -Werror __unconditionally__ in their software packages. And what applies to me - not being able to anticipate warnings in other compilers - of course applies to them as well" "squid requires me to __doctor around in its configure__ script, and even __patch some Makefiles__, while qemu-linaro has a central location for it." The complaint boils down to "-Werror is a development tool, so use it like that".

mkj 10y ago

Sure, but do you opt-in or opt-out of being a developer versus just someone trying to build a package?