4 ms·

The fundamental confusion of the article (and come commenters here) is whether you primarily release code or binaries. If your primary product is a binary (eve

by jmull 5y ago

The fundamental confusion of the article (and come commenters here) is whether you primarily release code or binaries. If your primary product is a binary (even if you release code too for reference or to be open), by all means, use -Werror (and strictly set the supported compiler). If your product is code (even if you release binaries too as a convenience), -Werror in the release is an impediment to your users because you've introduced a dependency on a specific compiler version. Think of what will happen when a user tries to use another source library using -Werror... but pinned to a different compiler or compiler version. You've created a problem with your project that your users will have to solve. You could dictate the the build environment of your users, but that's simply a roundabout way to actually be in the business of releasing binaries. For some projects, I think you can decide to dictate the use of -Werror (and compiler version) for certain code changes processes, but that's a judgement call, with pros and cons. A strength of open source is that the distinction between user and contributer can be blurry. E.g., imagine a user runs your library on a platform you have neither the time nor expertise to support, finds and fixes some issues and is willing to contribute those changes back... are you sure you want to put them in the position of addressing errors from a toolchain they don't know or understand? Maybe. But maybe not.

pizlonator 5y ago
You hit the nail on the head! And the same exact critique applies to folks saying “-Werror is harmful”. That statement is false unless you also add the limiting principle: “-Werror is harmful in software shipped as source that users build themselves”. This is a classic case of folks talking past each other. Kinda fun to watch actually.
jcelerier 5y ago
> because you've introduced a dependency on a specific compiler version No, if you get warnings, it means that your code is (and was already) wrong. And if you wrote wrong code you already have an implicit dependency on your compiler version, as more recent versions with more advanced static analysis may turn a semi-harmless unrecognized UB as a very harmful recognized one (and thus warn). I had this case a few times already, see a simple repro : https://gcc.godbolt.org/z/jY69sas8K https://gcc.godbolt.org/z/jY69sas8K -Werror just makes this dependency explicit.
- saagarjha 5y ago
  > No, if you get warnings, it means that your code is (and was already) wrong. No, that's not right, otherwise they wouldn't be warnings. The whole point of them existing is the compiler trying to alert you of a suspicious construct, but it's not confident enough to be sure it's a bug. Sometimes I want to use assignment for its result–that's not a bug, it's just something that is often done accidentally.
  - tialaramex 5y ago
    There's a big language problem here. It's not the whole of the issue, but it's certainly a big component. In C and C++ there are a huge number of things that are legal, and thus must not cause an error diagnostic (out of the box) - and yet are obviously not what anybody should do (and so a warning diagnostic is appropriate if the compiler can successfully diagnose them). You give the example of "sometimes I want to use assignment for its result". A better language outright forbids this. If you want the result, write that, if you want an assignment, write that, if you want both, write both so that it's clear what you intended, and let the optimiser turn your clear program into efficient machine code rather than trying to out-guess the machine. Ideally warnings would not be red flags. If warnings are stuff like "This variable's name start with a capital letter when that's contrary to standard style guides." or "You keep saying 15 in this program, have you considered just defining a constant instead for whatever 15 is?" then you're in a better place, but unfortunately in C the warning is more likely to be "99.99% of the time this code is a buffer overflow, so, you probably don't mean that" or "It looks like this does X doesn't it? But, it actually does nothing at all, and for complicated reasons we can't make it do X, so, I really hope you didn't mean X."
    - josefx 5y ago
      > but unfortunately in C the warning is more likely to be "99.99% of the time this code is a buffer overflow, so, you probably don't mean that" In my experience it is more likely that 0.01% of the time this signed to unsigned conversion (the one in the if block checking value ranges) can cause issues, please explicitly cast it to silence the warning.
SAI_Peregrinus 5y ago
The issue IMO isn't -Werror, it's -Wall, -Wextra, and -Weverything. If you individually specify the warnings you want, other compiler versions won't have a different list. So -Werror won't cause your build to fail because some other compiler (or version) had a different -Wall list.
- steerablesafe 5y ago
  That's not guaranteed. Compilers do fix existing warnings that missed some existing cases.
MichaelMoser123 5y ago
You can still have a separate makefile target for -Wall -Werror, even if you are releasing the source code; Alternatively you can add it this check conditioned on the presence of an optional environment variable, and set that environment variable in your CI pipeline. Also don't forget to add -Wextra, if you are at it! I don't quite understand the controversy, this is just another code checking tool, just like lint. Also: did anyone write an article titled "'consider harmful' is considered harmful" yet? I am considering writing something like this. I think this genre has a tendency to state an opinion as the final truth with a capital T, and because of that, it tends to provoke argument out of nothing.

by jmull 5y ago

pizlonator 5y ago

You hit the nail on the head! And the same exact critique applies to folks saying “-Werror is harmful”. That statement is false unless you also add the limiting principle: “-Werror is harmful in software shipped as source that users build themselves”. This is a classic case of folks talking past each other. Kinda fun to watch actually.

jcelerier 5y ago

> because you've introduced a dependency on a specific compiler version No, if you get warnings, it means that your code is (and was already) wrong. And if you wrote wrong code you already have an implicit dependency on your compiler version, as more recent versions with more advanced static analysis may turn a semi-harmless unrecognized UB as a very harmful recognized one (and thus warn). I had this case a few times already, see a simple repro : https://gcc.godbolt.org/z/jY69sas8K https://gcc.godbolt.org/z/jY69sas8K -Werror just makes this dependency explicit.

saagarjha 5y ago
> No, if you get warnings, it means that your code is (and was already) wrong. No, that's not right, otherwise they wouldn't be warnings. The whole point of them existing is the compiler trying to alert you of a suspicious construct, but it's not confident enough to be sure it's a bug. Sometimes I want to use assignment for its result–that's not a bug, it's just something that is often done accidentally.
- tialaramex 5y ago
  There's a big language problem here. It's not the whole of the issue, but it's certainly a big component. In C and C++ there are a huge number of things that are legal, and thus must not cause an error diagnostic (out of the box) - and yet are obviously not what anybody should do (and so a warning diagnostic is appropriate if the compiler can successfully diagnose them). You give the example of "sometimes I want to use assignment for its result". A better language outright forbids this. If you want the result, write that, if you want an assignment, write that, if you want both, write both so that it's clear what you intended, and let the optimiser turn your clear program into efficient machine code rather than trying to out-guess the machine. Ideally warnings would not be red flags. If warnings are stuff like "This variable's name start with a capital letter when that's contrary to standard style guides." or "You keep saying 15 in this program, have you considered just defining a constant instead for whatever 15 is?" then you're in a better place, but unfortunately in C the warning is more likely to be "99.99% of the time this code is a buffer overflow, so, you probably don't mean that" or "It looks like this does X doesn't it? But, it actually does nothing at all, and for complicated reasons we can't make it do X, so, I really hope you didn't mean X."
  - josefx 5y ago
    > but unfortunately in C the warning is more likely to be "99.99% of the time this code is a buffer overflow, so, you probably don't mean that" In my experience it is more likely that 0.01% of the time this signed to unsigned conversion (the one in the if block checking value ranges) can cause issues, please explicitly cast it to silence the warning.

SAI_Peregrinus 5y ago

The issue IMO isn't -Werror, it's -Wall, -Wextra, and -Weverything. If you individually specify the warnings you want, other compiler versions won't have a different list. So -Werror won't cause your build to fail because some other compiler (or version) had a different -Wall list.

steerablesafe 5y ago
That's not guaranteed. Compilers do fix existing warnings that missed some existing cases.

MichaelMoser123 5y ago

You can still have a separate makefile target for -Wall -Werror, even if you are releasing the source code; Alternatively you can add it this check conditioned on the presence of an optional environment variable, and set that environment variable in your CI pipeline. Also don't forget to add -Wextra, if you are at it! I don't quite understand the controversy, this is just another code checking tool, just like lint. Also: did anyone write an article titled "'consider harmful' is considered harmful" yet? I am considering writing something like this. I think this genre has a tendency to state an opinion as the final truth with a capital T, and because of that, it tends to provoke argument out of nothing.