5 ms·

Why, what’s the alternative - leave the warnings hanging around? I work on a multi million loc.C++ codebase that used -Werror. If we turned it off, the codebase

by olig15 3y ago

Why, what’s the alternative - leave the warnings hanging around? I work on a multi million loc.C++ codebase that used -Werror. If we turned it off, the codebase would be full of warnings in hours.

kevin_thibedeau 3y ago
Use -Werror for the build server and have a policy that you can't commit code that generates warnings. That lets you still build locally without having to immediately fix minor warnings that would be a distraction.
- yellow_lead 3y ago
  How can the build system differentiate between new and old errors? Maybe I misunderstand you, but it seems like that piece is missing.
  - kevin_thibedeau 3y ago
    If the policy is followed no errors will be generated on the build server. You have to have a codebase that builds cleanly for this to be possible so you're forced to resolve warnings before they become a cancer.
  - plorkyeran 3y ago
    There are no old warnings because the commits introducing them would have been rejected. Obviously a prerequisite of this is that you get your codebase to the point where it has no warnings. It is sort of like asking how the build server should differentiate between new test failures and existing test failures.
    - Am4TIfIsER0ppos 3y ago
      You update the compiler and suddenly the same code now generates warnings. Are these old warnings or new warnings? I have seen that with printing an integer in a loop 0..99. Old compilers didn't understand the limit so warned that my buffer was not big enough. Middle aged compilers were silent because I assume they understood the range. New compilers started warning again that the range was -2147483648..99 dumb shits. Or are you one of those to never update a working system, which I completely agree with.
      - plorkyeran 3y ago
        You should test new compiler versions before updating your CI setup. If you get new warnings, you fix them before deploying.
r2vcap 3y ago
If you're working in a controlled environment, such as proprietary software with a fixed number of toolchains or a few OSS projects like Chromium that use their own fixed toolchain, then it makes sense to turn on -Werror. However, most OSS projects deal with tons of different compilers, so it is not possible to enforce -Werror, as different compilers and versions handle warnings differently.
- soulbadguy 3y ago
  it takes a bit of discipline, but i think it is entirely possible. Even when some OSS project support a lot of different build environment, the support is usually tiered with some of the target having daily or per commit builds. Those should be setup with -Werror + exceptions where it makes sense. Once a project get to stable state (as in for a given target, all the warning are either turned off, or handled). Upgrading or adding new target get easier.
  - saagarjha 3y ago
    Even projects with a “stable” set of targets have too much churn for this to be practical. Just because you support “latest macOS” doesn’t mean my build should fail if I’m on a beta build ahead.
    - soulbadguy 3y ago
      > Even projects with a “stable” set of targets have too much churn for this to be practical. Are you speaking here from experience ? Because mine has been quite different.
      - saagarjha 3y ago
        Yes, I typically run toolchains that are slightly ahead or different than the one that is actually used in CI. I don't think it is reasonable to think that projects will be using it, though they often do appreciate me testing it early for them.
    - 3y ago
ploxiln 3y ago
The alternative is to fix warnings! You don't have to completely fail the build, to have the good engineering sense to fix warnings. Unless you never build locally? And never look at a build log? You really never improve code if Werror doesn't force you to? It's really obnoxious to also immediately and completely fail the build for someone who wants to use an updated compiler, or wants to compile for a different architecture, or wants to compare to an older compiler, or a different vendor's compiler, or test with and updated library with changed headers, or ...
- not2b 3y ago
  Turning on -Werror means that once the warnings are eliminated, they stay eliminated, and a developer who adds code that produces a warning has their checkin rejected. If it isn't used the number of warnings will just grow and grow. You're right, updated compilers that have more warnings are an issue, and that's why the document recommends that -Werror be used during development but not in the shipped code (for open source projects), so the recipient of the code isn't blocked by the problem you cite.
  - account42 3y ago
    > If it isn't used the number of warnings will just grow and grow. Only in shitty teams without discipline and only if warnings are not tracked in some other way.
    - not2b 3y ago
      The way to impose discipline effectively is to have automated checks. Like -Werror, and additional "lint"-style checks to make sure that coding guidelnes are followed. Non-shitty teams automate as much of the flow as possible, to catch mistakes early and help the human reviewers catch everything.
    - pjmlp 3y ago
      Most companies have such teams, fortunate of those that never experienced such employers.

by olig15 3y ago

Why, what’s the alternative - leave the warnings hanging around? I work on a multi million loc.C++ codebase that used -Werror. If we turned it off, the codebase would be full of warnings in hours.

kevin_thibedeau 3y ago

Use -Werror for the build server and have a policy that you can't commit code that generates warnings. That lets you still build locally without having to immediately fix minor warnings that would be a distraction.

yellow_lead 3y ago
How can the build system differentiate between new and old errors? Maybe I misunderstand you, but it seems like that piece is missing.
- kevin_thibedeau 3y ago
  If the policy is followed no errors will be generated on the build server. You have to have a codebase that builds cleanly for this to be possible so you're forced to resolve warnings before they become a cancer.
- plorkyeran 3y ago
  There are no old warnings because the commits introducing them would have been rejected. Obviously a prerequisite of this is that you get your codebase to the point where it has no warnings. It is sort of like asking how the build server should differentiate between new test failures and existing test failures.
  - Am4TIfIsER0ppos 3y ago
    You update the compiler and suddenly the same code now generates warnings. Are these old warnings or new warnings? I have seen that with printing an integer in a loop 0..99. Old compilers didn't understand the limit so warned that my buffer was not big enough. Middle aged compilers were silent because I assume they understood the range. New compilers started warning again that the range was -2147483648..99 dumb shits. Or are you one of those to never update a working system, which I completely agree with.
    - plorkyeran 3y ago
      You should test new compiler versions before updating your CI setup. If you get new warnings, you fix them before deploying.

r2vcap 3y ago

If you're working in a controlled environment, such as proprietary software with a fixed number of toolchains or a few OSS projects like Chromium that use their own fixed toolchain, then it makes sense to turn on -Werror. However, most OSS projects deal with tons of different compilers, so it is not possible to enforce -Werror, as different compilers and versions handle warnings differently.

soulbadguy 3y ago
it takes a bit of discipline, but i think it is entirely possible. Even when some OSS project support a lot of different build environment, the support is usually tiered with some of the target having daily or per commit builds. Those should be setup with -Werror + exceptions where it makes sense. Once a project get to stable state (as in for a given target, all the warning are either turned off, or handled). Upgrading or adding new target get easier.
- saagarjha 3y ago
  Even projects with a “stable” set of targets have too much churn for this to be practical. Just because you support “latest macOS” doesn’t mean my build should fail if I’m on a beta build ahead.
  - soulbadguy 3y ago
    > Even projects with a “stable” set of targets have too much churn for this to be practical. Are you speaking here from experience ? Because mine has been quite different.
    - saagarjha 3y ago
      Yes, I typically run toolchains that are slightly ahead or different than the one that is actually used in CI. I don't think it is reasonable to think that projects will be using it, though they often do appreciate me testing it early for them.
  - 3y ago

ploxiln 3y ago

The alternative is to fix warnings! You don't have to completely fail the build, to have the good engineering sense to fix warnings. Unless you never build locally? And never look at a build log? You really never improve code if Werror doesn't force you to? It's really obnoxious to also immediately and completely fail the build for someone who wants to use an updated compiler, or wants to compile for a different architecture, or wants to compare to an older compiler, or a different vendor's compiler, or test with and updated library with changed headers, or ...

not2b 3y ago
Turning on -Werror means that once the warnings are eliminated, they stay eliminated, and a developer who adds code that produces a warning has their checkin rejected. If it isn't used the number of warnings will just grow and grow. You're right, updated compilers that have more warnings are an issue, and that's why the document recommends that -Werror be used during development but not in the shipped code (for open source projects), so the recipient of the code isn't blocked by the problem you cite.
- account42 3y ago
  > If it isn't used the number of warnings will just grow and grow. Only in shitty teams without discipline and only if warnings are not tracked in some other way.
  - not2b 3y ago
    The way to impose discipline effectively is to have automated checks. Like -Werror, and additional "lint"-style checks to make sure that coding guidelnes are followed. Non-shitty teams automate as much of the flow as possible, to catch mistakes early and help the human reviewers catch everything.
  - pjmlp 3y ago
    Most companies have such teams, fortunate of those that never experienced such employers.