3 ms·

It's good practice to turn on all warnings but then what? If the aim is no warnings, and that can even be a hard rule, then you also want to add -Werror to trea

It's good practice to turn on all warnings but then what? If the aim is no warnings, and that can even be a hard rule, then you also want to add -Werror to treat all warnings as errors. That might sound drastic but that in fact enforces discipline and the removal of warnings and is IMHO very good practice especially if you start doing that from day one. For legacy projects this can be a goal after an comprehensive phase of warnings removal. I've worked on projects with and without "all warnings as errors" and ultimately it is well worth it.

rwmj 4y ago
Yes, but don't turn it on except in developer builds. For downstream packagers it's a pain if a newer compiler adds a warning which then breaks the build.
- DyslexicAtheist 4y ago
  I see your point, and agree it might be painful. it depends on the promises the maintainer makes to themselves and to their users. I'd rather break the build and get immediately alerted by (hopefully moderately) upset downstream packagers than having this warning become visible in my users builds. It isn't just about getting rid of a warning, but the new compiler has changed some important logic so this warning might highlight potentially undefined behavior. Am I, the maintainer OK with that - that is the main question I believe. Might be useful to do regression with any new compiler being released especially when a product is used in safety or security critical systems.
  - rwmj 4y ago
    The trouble is a downstream packager might be compiling the whole distro -- 1000s of packages -- and they're not in a position to fix a new warning somewhere deep inside a program they don't know and probably don't use. They are more likely to hack the build system to add -Wno-error while cursing the upstream project.
    - DyslexicAtheist 4y ago
      fair point
- tmtvl 4y ago
  This may be a controversial statement, but I don't think projects should distribute any flags aside from the bare minimum to get everything working. E.g.: -std=c17 -lm And let downstream add their own flags (-Owhatever, -fsomething,...) on top of that.
  - rwmj 4y ago
    That's right. Fedora overrides a lot of flags globally, eg. adding -D_FORTIFY_SOURCE=3 and a bunch of other hardening stuff.
    - injinj 4y ago
      That's new. Wow, latest has -mno-omit-leaf-frame-pointer (rpm --eval '%{optflags}') Do you know of a way to get rpmbuild to keep the -ffat-lto-objects in the ar libs?
  - account42 4y ago
    There are many more flags such as -ffast-math where the project will be better positioned to decide if they make sense than some random packager juggling hundreds of packages. Even for optimzation flags that don't change behavior there is no clear best set of flags that you should blindly apply to all projects.
DyslexicAtheist 4y ago
not sure why your comment is downvoted. The only way to ensure discipline and rid a large code base of warnings is to use -Werror It may sound drastic until you're the poor intern that needs to refactor an active codebase with thousands of warning. Other than the technical debt it creates you're setting yourself up for having serious issue slipping through in all the noise. "But some warnings might be false positives" ? If one really must override the behavior it should be done explicitly and with a strong justification. This is possible either with a pragma[1] #pragma GCC diagnostic ignored "-Wfoo" or by globally[2] removing the entire warning category. -Wno-foo [1] http://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html http://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html [2] https://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Warning-Options.html https://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Warning-Options...
- hulitu 4y ago
  > The only way to ensure discipline and rid a large code base of warnings is to use -Werror And this works as long as everybody has the same setup as you. This is unfortunately never the case. Kernel changes, libc changes, the compiler changes and sooner or later you will break the build.
  - pjmlp 4y ago
    Allow developers to do whatever they feel like, impose -Werror on CI/CD and PR.
flohofwoe 4y ago
Exactly, turning on -Werror is good practice during development and when running CI, but not when building the source distribution of your code (because the user might have a slightly different version of the compiler which has different warnings).
- mytailorisrich 4y ago
  What source distribution? Many replies seem to imply open source by default. Of course projects may have specific requirements, and open source is indeed a specific use case, but the majority of software is commercial, closed source. Regarding compiler versions, many projects I worked on actually check that during the build because the code may only have been designed for or tested against a specific compiler/compiler version. In general upgrading the version of the compiler is an activity in itself because, indeed, this may require solving new warnings/compilation issues and a full test campaign.
  - flohofwoe 4y ago
    > but the majority of software is commercial, closed source That may be true for 'end user applications', but definitely not for libraries (thankfully). I use exactly zero closed source (precompiled) dependencies in my hobby and professional work because they are such pain to work with. Thankfully these days there's an open source alternative for pretty much everything, 15 years ago the situation was much more dire.
    - mytailorisrich 4y ago
      Oh you use plenty of closed source software in your daily life. The majority of software out there is closed source. Edit: the applications you use on your desktop represent a very small portion of the software running in the world, including software that crosses your path on a daily basis and so 'software devlopment' is not synonymous with open source.
      - flohofwoe 4y ago
JoshTriplett 4y ago
I personally like turning on `-Werror=specific-warning` `-Werror=another-specific-warning`, for a long list of warnings. Then, if the compiler adds a new warning, you have a chance to review it and eliminate all instances of it before making it an error. That doesn't prevent the compiler from expanding an existing warning, but that seems to break code less often than a blanket `-Werror`.
account42 4y ago
> If the aim is no warnings, and that can even be a hard rule, then you also want to add -Werror to treat all warnings as errors Hard disagree. No warnings means CI should fail (or otherwise scream loudly), not every build of your in-progress working tree. You can add a commit hook if you want but if it needs to be enforced then it isn't really discipline. Even for CI you probably don't want -Werror but instead run the build completely so that you can track all warnings. And for source released to others -Werror is downright rude as you have no idea what warnings their possible future compiler will have.

rwmj 4y ago

Yes, but don't turn it on except in developer builds. For downstream packagers it's a pain if a newer compiler adds a warning which then breaks the build.

DyslexicAtheist 4y ago
I see your point, and agree it might be painful. it depends on the promises the maintainer makes to themselves and to their users. I'd rather break the build and get immediately alerted by (hopefully moderately) upset downstream packagers than having this warning become visible in my users builds. It isn't just about getting rid of a warning, but the new compiler has changed some important logic so this warning might highlight potentially undefined behavior. Am I, the maintainer OK with that - that is the main question I believe. Might be useful to do regression with any new compiler being released especially when a product is used in safety or security critical systems.
- rwmj 4y ago
  The trouble is a downstream packager might be compiling the whole distro -- 1000s of packages -- and they're not in a position to fix a new warning somewhere deep inside a program they don't know and probably don't use. They are more likely to hack the build system to add -Wno-error while cursing the upstream project.
  - DyslexicAtheist 4y ago
    fair point
tmtvl 4y ago
This may be a controversial statement, but I don't think projects should distribute any flags aside from the bare minimum to get everything working. E.g.: -std=c17 -lm And let downstream add their own flags (-Owhatever, -fsomething,...) on top of that.
- rwmj 4y ago
  That's right. Fedora overrides a lot of flags globally, eg. adding -D_FORTIFY_SOURCE=3 and a bunch of other hardening stuff.
  - injinj 4y ago
    That's new. Wow, latest has -mno-omit-leaf-frame-pointer (rpm --eval '%{optflags}') Do you know of a way to get rpmbuild to keep the -ffat-lto-objects in the ar libs?
- account42 4y ago
  There are many more flags such as -ffast-math where the project will be better positioned to decide if they make sense than some random packager juggling hundreds of packages. Even for optimzation flags that don't change behavior there is no clear best set of flags that you should blindly apply to all projects.

DyslexicAtheist 4y ago

not sure why your comment is downvoted. The only way to ensure discipline and rid a large code base of warnings is to use -Werror It may sound drastic until you're the poor intern that needs to refactor an active codebase with thousands of warning. Other than the technical debt it creates you're setting yourself up for having serious issue slipping through in all the noise. "But some warnings might be false positives" ? If one really must override the behavior it should be done explicitly and with a strong justification. This is possible either with a pragma[1] #pragma GCC diagnostic ignored "-Wfoo" or by globally[2] removing the entire warning category. -Wno-foo [1] http://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html http://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html [2] https://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Warning-Options.html https://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Warning-Options...

hulitu 4y ago
> The only way to ensure discipline and rid a large code base of warnings is to use -Werror And this works as long as everybody has the same setup as you. This is unfortunately never the case. Kernel changes, libc changes, the compiler changes and sooner or later you will break the build.
- pjmlp 4y ago
  Allow developers to do whatever they feel like, impose -Werror on CI/CD and PR.

flohofwoe 4y ago

Exactly, turning on -Werror is good practice during development and when running CI, but not when building the source distribution of your code (because the user might have a slightly different version of the compiler which has different warnings).

mytailorisrich 4y ago
What source distribution? Many replies seem to imply open source by default. Of course projects may have specific requirements, and open source is indeed a specific use case, but the majority of software is commercial, closed source. Regarding compiler versions, many projects I worked on actually check that during the build because the code may only have been designed for or tested against a specific compiler/compiler version. In general upgrading the version of the compiler is an activity in itself because, indeed, this may require solving new warnings/compilation issues and a full test campaign.
- flohofwoe 4y ago
  > but the majority of software is commercial, closed source That may be true for 'end user applications', but definitely not for libraries (thankfully). I use exactly zero closed source (precompiled) dependencies in my hobby and professional work because they are such pain to work with. Thankfully these days there's an open source alternative for pretty much everything, 15 years ago the situation was much more dire.
  - mytailorisrich 4y ago
    Oh you use plenty of closed source software in your daily life. The majority of software out there is closed source. Edit: the applications you use on your desktop represent a very small portion of the software running in the world, including software that crosses your path on a daily basis and so 'software devlopment' is not synonymous with open source.
    - flohofwoe 4y ago

JoshTriplett 4y ago

I personally like turning on `-Werror=specific-warning` `-Werror=another-specific-warning`, for a long list of warnings. Then, if the compiler adds a new warning, you have a chance to review it and eliminate all instances of it before making it an error. That doesn't prevent the compiler from expanding an existing warning, but that seems to break code less often than a blanket `-Werror`.

account42 4y ago

> If the aim is no warnings, and that can even be a hard rule, then you also want to add -Werror to treat all warnings as errors Hard disagree. No warnings means CI should fail (or otherwise scream loudly), not every build of your in-progress working tree. You can add a commit hook if you want but if it needs to be enforced then it isn't really discipline. Even for CI you probably don't want -Werror but instead run the build completely so that you can track all warnings. And for source released to others -Werror is downright rude as you have no idea what warnings their possible future compiler will have.