Two musings on the design of compiler warnings | Hacker News Reader

Two musings on the design of compiler warnings

quuxplusone.github.io

35 pointsingve26 comments

There is a set of concerns in this article that are universal, and then there are the specifics of C, which generally leave me horrified that because some rando is free to write

  if((this=that)) {
    ...
  }

some hackers can crash the power grid, blow up a chemical plant, etc. Using the double paren to suppress warnings is frankly terrifying to me because the double paren is the kind of thing that comes out of the ten thousand monkeys that almost understand Noam Chomsky that are trying to 'help' me in my IDE or when I type with a touchscreen -- the same accidents that might result in "this=that" could just as well leave double parens...

OTOH in more civilized languages the question of error messages is fascinating to me but insufficiently documented, in fact it is part of the "parser generators haven't improved since yacc" syndrome. "Little languages" like the ones yacc was invented to make easy haven't taken off in the way that yacc's siblings did because of a handful of problems that haven't been addressed in one tool -- and the error handling is the worst of them.

I told the Jena people that I thought error handling with their RETE engine was great -- I could look at the error and look at the Jena source code and understand what was wrong with my source code. That was just not the case with Drools.

See, I think your problem is specifically with the fact that C provides the same operator for "assignment" and "assignment and I get a value back". The issue is that the latter is useful but generally not what you want, so this is why we warn about it but I don't think it's justified to be horrified about it.

unused variable is the warning that causes me the most pain

it happens so often that I want to test or debug something, disable some code, or replace some value by "5" or what not as a placeholder, and then get the unused variable warning

sure, it can be fixed with (void) casts, but sometimes there are multiple unused variables due to commenting out part of code, then you have to hunt all of them

and sure it can be fixed with a flag to disable this warning, but that may require a recompile of everything if the build system considers this a change of compiler flags

and sure you could disable it always, or only for certain modes, but the warning also has its merits

Or just ignore warnings if you know they're from a temporary change that wont ever be committed?

Some teams turn on the switch in their compiler or linter that turns warnings into errors. Not many people invoke their compilers directly anymore, so disabling this requires digging into the build system configuration, which is not always simple (and as the OP mentioned, can require a full rebuild).

Sometimes this is even baked into the language itself. For example Go can be strict about unused imports and variables. Python is strict about having correct indentation.

Right - I understand all that, I just think it's not a good idea. The compiler makers made some things warnings and others errors for a reason, to disregard their input because you have some idea that "warnings are just as bad as errors and should never be permitted in even dev builds of my very highly precious code" is to me a sign of an over-controlling "principle engineer" somewhere behind the scenes that doesn't trust their team, which is not a work environment I'd want to be in. (In the go case, the over-controlling principle engineers creating a work environment I'm not interested in participating in are the language designers themselves!)

I introduce unused variables all the time when I'm debugging/exploring, which always get cleaned up before commit (this is programmatically enforced, which I think is reasonable), to need to go and comment out the declarations for each variable that goes unused if I comment out a section, just because some "principle engineer" somewhere on my team (or the language design team) thinks unused variables are the devil reincarnate and can't be allowed in any build whatsoever is frankly insane. (Don't even get me started on cases where commenting out the declarations changes behavior...)

> Some teams turn on the switch in their compiler or linter that turns warnings into errors.

That's completely self-inflicted. I really don't get why people do that instead of just fixing the warnings... except if one is in some position of power over environment configuration, but not over work results. Even then, why does that person care about work results?

One problem is the updates in toolchains that come up with new warnings, a lot of which are nuisances. Developers get tired of the make-work which that generates, either to fix the warnings or else identify where they are coming from and put in options to stop them.

The real problem are warnings that are super-useful (find a bug) about one out of twenty times (or less). The other nineteen times, they are false positives, such that refactoring the program to eliminate them is a fool's errand.

You don't want to turn off these warnings, and you don't want to fix all instances. So then you need per-file warning disabling, or start using compiler-specific #pragma-s:

   #pragma GCC diagnostic warning "-Wno-whatever"

The issue is incremental compilation. It is common (although not universal) that the way the build system (interpreted broadly) remembers that there are errors in unit X is that it has failed to produce a (new enough) unit X. In that setting, -Werror turns warnings from something that might not be surfaced in a (re)build into something that will be.

In my experience the warnings tend to pile up. Once you have a few warnings in the codebase, what's a few more?

I think a good compromise is warnings aren't allowed in the final committed code (this can be enforced either by convention or with git hooks), but whatever you do locally is between you and your compiler. "What happens in the devbox stays in the devbox"

I'd tend to agree, though this can be harder to set up than just having a consistent build config for all platforms.

Generally I would hope that your CI build allows you to set this kind of configuration. (At the very least, you must have this as an option, because someone will run into it at some point and will have to turn your -Werror off.)

In large bodies of code, you might not notice the warnings if they weren't errors.

Code size can have something to do with it, but less than you think.

Basically, you don't notice warnings because you choose not to look.

Firstly, no matter how large the program is, if it builds without warnings and then a change is introduced which triggers a warning, it will be noticed. You only don't notice a new, potentially important warning when it is drowned out by "nuisance" warnings that you stopped caring about months or years ago. This tends to be a problem in larger programs worked on by many people, but it could easily affect a small, solo-effort program as well.

You perhaps don't notice warnings because the build log is too long to read, and you haven't put in CI process which scans logs for warnings and produces feedback. When you build locally, you don't bother using an IDE which picks up all the errors and warnings out of the build output an lets you navigate through them. This can be a problem when people don't build locally; they just throw the code into some remote build system and if it goes "green", they "ship it".

This is generally why such a policy is often accompanied with "we don't want you introducing warnings; the policy is just relaxed while you're developing the code".

> Some teams turn on the switch in their compiler or linter that turns warnings into errors.

Don't bend over backwards. Just laugh at them. Losers always whine about how dare you trigger muh unused parameter.

While perhaps satisfying, the issue is that this usually does little good in that the person you are directing this at will rarely change their ways in response.

So you change your ways I bet, to accommodate the stubborn cargo cultists who cause toil for you. Here's an alternative for you to consider. Say yes, I can absolutely support you, but if you believe that cc is an ada linter, then you can't use the project in 'preferred form' you have to link against the finished .so / .a binaries, and include the ANSI C89 preprocessed header file amalgamation, which will enable you to use whatever compiler flags you want, but you won't get the benefit of all the fancy macro optimizations and such.

Question: Does the compiler still throw the warning about "x and y unused" when, instead of replacing

  z = x + 2*y + 1;

  z = 5; // x + 2*y + 1;

you do something like

  z = (x + 2*y + 1) * 0 + 5;

Are you compiling with -Werror or something like that?

Because most people have no problem with warnings appearing while they modify their code.

for (auto foo : list) assert(foo);

The problem is that the assert() macro disappears in release mode, causing foo to trigger an unused variable warning. You need to (void)foo; as well.

On a related note, since assert() is removed in release mode, assert(fn_with_side_effects()) will fail to run the function in release mode. You need to separate it into two lines.

Should probably #define assert(x) to be ((void)x) for release builds instead.

Make sure to #undef assert before this, and then set NDEBUG and include assert.h again.

Go is awful about this, because it will refuse to compile your code if you have an unused import.

I remember very old versions of lint had hints you could add, like:

   /*NOTREACHED*/

that would suppress warnings. I kind of liked that method.

Reply on news.ycombinator.com