Why should I always enable compiler warnings? | Hacker News Reader

Why should I always enable compiler warnings?

stackoverflow.com

69 pointsazhenley78 comments

The trouble with warnings is they inherently balkanize the language into multiple incompatible and even contradictory languages.

Warnings arise from being unable to modify the language semantics. But even if you can modify the semantics, warnings are spawned as a compromise when people cannot agree on what the language semantics should be.

I've tried pretty hard with the D programming language to not have any warnings - that the semantics should be firmly decided upon. Some warnings still have crept into the compiler, but fortunately just a handful.

> Warnings arise from being unable to modify the language semantics

Not always. For instance, C has a lot of stuff around casting, memory management, etc. that is semantically legal but often good to warn about. Other times, it can warn about a possible stack overflow. C has tons of behavior which is not technically defined but has been agreed upon by most compiler implementers (and doesn't warn), though admittedly, there are also cases where you'll get different results.

> Not always. For instance, C has a lot of stuff around casting, memory management, etc. that is semantically legal but often good to warn about.

That's just what I meant. Can't change C, so issue a warning. For example,

    for (i = 0; i < 10; ++i);
        sum += i;

My C compiler warns about the ; and the D compiler gives an error for it. A number of common C warnings are just flat out errors in D. For another example,

    if (a < b < c)

likely will produce a warning in C, but it won't even parse in D (I fixed the grammar so it is not a valid construction).

The fact that these pointless and buggy constructs persist in C is evidence that the language cannot be changed, hence a warning is used.

An even better solution for the "for" example is enforcing the curly braces around every block. Simple rule, no heuristics to catch such bugs.

I find that many people really dislike to put what they feel are “extra” braces.

and then it bites them in the ass when they need to add another line later on.

I've been writing C and C++ for thirty years and I've made many mistakes but never that one. I suspect it's far more common if an always-uses-braces programmer edits an optional-braces code base. I'm not religious about it though. If the Romans are wearing braces I'll wear braces.

Getting worked up over a non-issue is counter-productive (to be clear: it is fussing over the 'extra' braces that is the non-issue; having an explicit syntax for the empty statement is an issue, and one that makes sense.)

They are extra braces. The C grammar allows any statement as the body of a control structure. Compound statements are only one of the possibilities. Just because other C-like langs have different grammar requiring the braces doesn't mean C has to look like them too.

Just to be clear, D doesn't require extra braces. It just doesn't allow an empty statement terminated with a ;.

    if (x) foo();  // accepted
    if (x) ;       // rejected
    if (x) { }     // accepted

It's not a heuristic in D. The grammar was adjusted so having a ; denote an empty statement is not part of the grammar. Hence,

    if (x);

etc. are also rejected. If you actually want an empty statement, { } does the job. Doing it with the grammar is not hackish and it works out quite nicely.

I'm guessing that was a design goal from day one? How hard has been to maintain that goal, as any slip up will become just as hard to fix as in C, right?

D has been more accepting of changes to fix bad design decisions than C has. The ; thing was there since day 1, the rejection of (a < b < c) came later - nobody objected to it.

Is that a property of the D community being younger (the community itself, rather than the people in the community) and smaller than C and C++ communities. Or is it something more culture based. Any ideas?

That's a great question. It seems to be a property of the D community wanting the language to get better and being willing to accept change.

In the olden days (1989) people were more willing to accept change in C, such as value-preserving rather than sign-preserving integer promotion rules, and some improvements to the C preprocessor.

I have Visual Studio configured to warn on blocks without braces. It finds a bug about once every two years.

No matter what your language semantics are, there will still be different styles of coding. It's extremely valuable to shepherd your codebase into a particular paradigm for readability reasons.

That said, style warnings are incredibly hard to detect and enforce (e.g., -Weffc++ is so noisy I've never been in any codebase where I could leave it on)

Style warnings are something else entirely. The warnings I'm talking about are for code that looks suspiciously like a bug.

Another argument (which I remember coming from Walter): The job of the compiler is to generate code. It's job is not too serve as a half-assed static analysis tool.

Today a compiler is not the only tool to parse a language anymore. It sure is convenient to add some print statements where the compilers sees the warnings anyways. A better approach is to provide the frontend as a library to allow plenty of other tools, e.g. a formatter.

I think the "the compiler is not a static analysis tool" is a viewpoint that is fighting against the in-practice user-experience that reporting of problems in the code is more effective the earlier in the feedback loop it happens and the more guaranteed it is that the issues are noticed [X]. So having an automated static analysis report delivered as comments into a code-review system is better than after-the-fact offline static analysis, but "the compiler tells you about the problem as soon as you try to build the code" is better still. Lots of people don't run static analyzers, but everybody runs the compiler. (This doesn't mean that you need to structure the compiler internally to do everything at the same time rather than as separate compile and analyse passes -- just that the command your users use to compile should include the production of static-analysis type issues at the same time and in the same error-message format as it would for stuff like syntax errors.)

[X] for a reference backing this up see the part of Facebook's ACM article on static analysis https://cacm.acm.org/magazines/2019/8/238344-scaling-static-... in the section titled "Human factors"; there they compare "after-the-fact offline analysis" with "in the code-review system", and I think the same reasoning applies to comparing "in the compiler immediately" against "later in the code-review system".

Who says that static analysis happens "later in the code-review system"? Static analysis does not equal (slow) data flow analysis.

Actually, most people have a (very weak) form a static analysis which happens before the compiler: syntax highlighting. This can be expanded such that type checking and similar fast checks are integrated into the typing feedback loop.

In my ideal world, the next (slightly slower) feedback loop should be the unit test which the developer is currently working on/against.

Only the third-fastest feedback loop is a full compilation where compiler warnings would matter.

When I was doing lots of cross-platform development in C and C++, using various vendor-specific compilers (which had a lot of variation in warnings), I had a practice of being warnings&lint-free on all of them. I used various portable warnings-silencing conventions for things like unreachable code, assignment in expression context, unused return values, etc. (Besides all the other conventions for potential pitfalls that static analysis can't detect.)

A colleague, who was new to C back then, asked about it, so I shared some of my masochistic practices, including the `lint`-necessitated mantra, "Always cast the return value of printf to void."

Years later, after moving to another US state, I'm cutting through the courtyard of a large block, where people are eating lunch outside, and someone shouts, "Oh my god, it's ____! Always cast the return value of printf to void!" She then introduced me to her coworkers at the table, "This is ___. ... He's the reason I code C like an [bad word]." Which I took as a compliment.

Programming in C reminds me of advice I've heard about helicopter piloting: from the moment you start the helicopter, that thing is trying to kill you.

Perhaps it's helpful to think about coding C like a cool-headed, meticulous pilot, who's constantly aware of the danger.

> "Always cast the return value of printf to void."

Why?

It was part of being "lint-free". The `lint` program of the time would otherwise complain that you're not doing anything with the return value of `printf`. Not doing anything with the return value of a function was considered possibly poor code. One of the likely problems was not checking an error return value, and handling the error cases. However, in the case of `printf`, normally you would not check and handle errors there, so you wrote the code as `(void)printf(...);`, to tell `lint` that you're ignoring the return value intentionally.

I'd think the linter would special-case the printf function since it's so rare to care about its return value.

But it didn't. And there were other bits of language like that, including ones for which it's less clear-cut (and even some code checked printf), and then the question becomes where do you draw the line in the defaults.

So the idea was to pass every compiler and linter. Whenever a warning appeared -- such as due to code changes, moving to a new platform, updating some library, etc., -- that was something to look at, not to assume you knew the warning was innocuous and get in the habit of disabling warnings.

The environment is a bit different now, with everyone using a small number of standards-compliant and featureful compilers. When you had situations like, e.g., your cpp on one new platform not even being quite K&R compliant, and potentially mangling your layers of macro expansion subtly, you really needed every hint you could get that something wasn't parsing or behaving like it looked like it would.

It was also perhaps faster just to do all your pedantic practices, than to try and reason about when you needed to do them.

Today, of course, especially if I'm using only one compiler, I might not bother with void on printf. That's where I might redraw the line, like you suggested. (Speaking of personal code for which I have stylistic discretion; for other code, I'd defer to the current conventions of the project and/or discuss with team.)

Though, a side benefit to being conspicuously pedantic is that, when you see a chunk of code that is less-pedantic in some way, it stands out. When we have so many critical defects due to insufficiently perfect C coding, spotting a chunk of code (especially one's own code) that's a little less-pedantic might mean that someone who worked on that was maybe being a little cavalier at the time, and maybe that chunk is a place to focus some attention.

Being warnings&lint-free isn't the most important thing in C, and my bigger concern is that, as a practice, overall, we don't agonize enough over C code correctness and manageability.

I have found that codebases that have not been built with warnings accrue bugs. Static analysis techniques from newer compilers etc reveal these bugs. Sometimes it's pretty astonishing -- you'll find bugs that make you wonder how anything works at all.

Even if you have some mild disagreement with the warning or some critical context that indicates "that's not a problem for us" -- fix it anyways. This goes for both static analysis and compiler warnings (ok, yes -- they're both static analysis). Keeping the codebase green means that you can build with `-Wall -Werror` and prevent new bugs from creeping in. If there's code that absolutely can't be rewritten to avoid the warning, you can mask the warning locally with things like pragmas.

That can work great if you only use one compiler, and can get messy if you use several.

You fix warnings on all of them.

All major compilers come with a way to suppress the particular warning in a particular file if all else fails.

While I completely agree in principle and do exactly that on my own personal codebases, that’s a hard sell at work at times

I think this is controversial only because C doesn't have a way to explicitly and reliably silence an instance of a warning. Without ability to negotiate with warnings they are effectively errors.

Rust has a ton of lints/warnings in the style of "this looks fishy, did you really mean that?", and users can answer "yes" by adding `#[allow(fishy_situation)]` to a particular scope.

Users also have an option of setting `#[deny(fishy_situation)]` on entire modules or programs to ensure the particular thing never happens.

Because it's per scope per warning, it doesn't have the dilemmas of all-or-nothing `-Werror` and other global warning flags.

Many answers suggest passing -Wall -Wextra to the compiler. In my experience, this misses many useful warnings. Here's a good list of additions warnings that usually produce useful results:

https://kristerw.blogspot.com/2017/09/useful-gcc-warning-opt...

I never really understood why "all" wasn't actually a catch-all. If it's not a catch-all, don't call it all. Or at least give us a -Wallyesreally, which doesn't currently exist afaik

GHC (Haskell) has -Weverything which is great for this. I’d much rather get all warnings and blacklist the few I don’t want

-Weverything is also part of clang. An example of a warning is "this struct has a padding byte between these two fields." A blacklist would be painful.

-Weverything should be thought of as a tool for discovering new warning flags relevant to your project. Add those flags, but don't enable -Weverything for routine builds, unless you're a masochist.

Barrow/compiler-warnings is a nice reference for all Chang and gcc warning flags, which flags implicitly enable other warnings, and in which compiler version they were added:

https://github.com/Barro/compiler-warnings/

Clang has -Weverything:

https://clang.llvm.org/docs/UsersManual.html#diagnostics-ena...

The problem with -Wall is that people use it with -Werror, thus compilers would have to be very conservative about adding new warnings to it. Hence the situation we are in with a few flags "above and beyond". Most of -Wpedantic for example is not something I'd want to break my build on.

The following stack overflow answer has some good example of other warnings you probably don't want, but would be included in -Weverything: https://stackoverflow.com/questions/11714827/how-to-turn-on-...

I'd argue that if you used -Wall and you got all the warnings you can't really complain too much because you got exactly what you asked for.

That's fine for an argument in a bar, and not so helpful for people who are doing software engineering with code that is developed under multiple compiler versions. Or who build old code versions with a new compiler version.

Why are "people who are doing software engineering" expecting to build code in a new compiler without migration effort, and ignoring new warnings that have been added potentially pointing to existing undetected issues just so the code continues to compile?

"People who want to get shit done", sure. Pragmatists, even.

It's only not so helpful because -Wall is badly named and now we're forced to deal with that legacy. Ideally, -Wall would be the same as clang's -Weverything, and there'd be a -Wstandard (or something) flag for what -Wall currently does.

How often do you get warnings that aren't either real bugs or trivial to fix anyway?

The problem with ignoring warnings is that they tend to accumulate and before long you can't see the ones you really do need to pay attention to. And yes that means you have to take the time to cleanup on new compilers with more warnings.

The issue really isn't with -Wall, it's with -Werror. If I release an open source package that compiles fine today with -Wall -Werror, and tomorrow there's a new compiler release that adds a new warning to -Wall, then that adds a new error to my package, and users trying to compile it with a new compiler will fail and complain. They didn't ask for -Wall -Werror, they only asked for some program that produces a working executable with ./configure ; make.

Like enabling assertions or not, this is (a matter of opinion, and) a difference between "development" and "release" compilations. Ideally we would consistently use something like ./configure --devel for one and ./configure --release for the other. Other languages' build systems have this built in, and building a tarball with autotools used to do something similar, but if you just pull something off GitHub it's not clear what mode you'll end up with.

> Ideally we would consistently use something like ./configure --devel for one and ./configure --release for the other. Other languages' build systems have this built in

Makefile has this built in too; you'd just be saying "make development" instead of "make --development".

If you need to write specific rules for it, I wouldn't call it "built in". But sure, it's possible. But there is no general consensus across projects to do it, and do it the same way.

wow, I'm astonished that -Wshadow is not enabled by either -Wall nor -Wextra

Lots of shadowing is present in existing codebases, sometimes by design.

LOL. I did not realize "all" was not all. Jesus Christ. Maybe I will finally be able to figure out what is causing my program to crash.

I’d recommend using a sanitizer rather than relying on compiler warnings for this.

I tried to use some sanitizers but they seemed unavailable for my system. I am on MSYS2 with GCC 9.

What about gdb?

Is it possible to use gdb as a sanitizer or something? gdb usually works great for me but the backtraces and random crashes make me think it's actually some kind of memory corruption.

If Valgrind supports your platform, or if you can also build for a supported platform, then that will likely help you find your crash.

Yeah it doesn't. I mean there is an old version that supposedly works with mingw but it seems outdated and I couldn't get it to build.

Windows has something resembling ASAN, you could try running your application with some/all checks enabled.

https://docs.microsoft.com/en-us/windows-hardware/drivers/de...

Yeah I tried that. Can't remember for sure if I was able to get it to work for my program or not but I think not.

I'm disappointed that the answer does not have any discussion about the problems with mixing `-Weverything` and `-Werror`.

In case you're not aware, the issue with this is upgrading your compiler almost certainly causes your project to stop compiling altogether, requiring immediate fixes to stuff that otherwise could have waited. What's worse, it also means you can't go back and compile older versions of your code with the newer compiler.

If you want your code to compile with the next versions of the compiler, you wouldn't wait the latest official stable release of the compiler to start looking at your future work. You would have a step in your CI that periodically checks out the latest compiler's revision/nightly and build your codebase with it. It wil surely fail, but you know what is coming months in advance before the stable release is out

At that point just flag the warnings instead of having it break the build...

This certainly changes for open-source vs closed-source code, but for the latter it's not much of a problem.

Either you use docker to have a stable environment, or you disable the Werror warning for this specific case.

There are more closed-source projects than fit in the Docker workflow.

That won't necessarily be enough to save you either because the existing warnings can still change. Here's some Rust code

    use foo;

    fn my_function() {
        use foo;
        foo::other_function()
    }

This used to not trigger a warning. In rustc 1.35, the existing warning unused_imports became smarter and could now detect the redundant import of foo. This resulted in several crates that had made unused item warnings into errors having the exact problem you said.

I think -Werror is a bad idea in general.

Indeed, I still don't see the point, the only non-superfluous thing -Werror does is to break builds for random people at random points in time.

Because C++ compilers have _major_ flaws in them that "can't be fixed" because they would break some people's builds from 1977 or something.

Such as giving "warnings" and happily compiling a program that is practically guaranteed to start doing random things at some point.

Yep. I wish backwards compatibility were less religious concept in C++. Just use old tooling if you need to compile your rats' nest of old code.

It's a nightmare to go between toolings and remember all their quirks.

The new language standards do forfeit elements of backwards compatibility. And newer compilers use newer language standards by default.

It's true that there is a great deal of hand-wringing over backwards compatibility when new C++ features are discussed, though.

GCC 9 defaults to a warning for no return which is undefined behavior.

I always enable all warnings globally for a very simple reason -- they can be useful (particularly the more obscure ones). Enabling them all globally means that I have to actually investigate the cause. If it turns out that the warning does not actually indicate a real problem, then I can disable that warning locally.

That way, they act as a kind of "checklist" for potential trouble that, once investigated, are removed from the checklist.

This practice has, on a number of occasions, allowed me to avoid some problems that would have been very hard, if not impossible, to nail down during testing or after release.

I think that code quality largely follows from discipline. We write unit tests even though it won't catch all of the potential defects, especially at service boundaries. We write integration tests even though they may not catch all of the defects of, for example, joe random giving garbage data to your public facing API. Tools like compiler warnings and static analysis alert us to the presence of potential issues. Heeding them at the outset is no more onerous in my ind than the previous two approaches. For me it's also part imitation. If you look at the high quality open source libraries available to you in languages that are statically typed, most of them have the compiler configured with warnings as errors. I don't think it's a coincidence. However, as stated previously, it also isn't enough on its own.

Is this a real question? Or someone just whoring for upvotes?

"Why should I eat my vegables? Innernet, tell me why!"

Note that the answer was written by the asker. Who then commented on the answer:

> I have posted this Q&A because I'm sick and tired of telling people to enable warnings. Now I can just point them here (or, if I'm in a particularly evil mood, close their question as a dupe). You are welcome to improve this answer or add your own.

Asking and answering a common question is standard stackoverflow practice.

I have yet to work on a code base where this is actually true. We have some 3.5Mio LOC and about 30k warnings... We have many bugs and tried to address this but the business does not warrant the resources as the product is "good enough"

What is your real world experience in this regard?

Yep, happens in many places I have worked in. When used with lint tools and metric measuring tools, the "problem" compounds exponentially because it trigger other warnings or risks that are not picked up the compiler.

I have never compiled a large application that did not have a ton of warnings. So I guess they are in good company at least.

I know post is about C/C++ compilers, but in general, I find it useful to treat compiler messages just as a normal log of a program – which, in this case happened to be a parser/compiler/linker.

Logging, in a nutshell, is a computer program seeking an attention of a human being. When everything goes smoothly and as expected, no log output needed.

We usually have, though, 4 most often used levels of logging:

- [DEBUG] – human explicitly asking program to tell a lot, for debugging

- [INFO] - program tells "hey, I'm fine here", mostly for assuring worrying human if program even works. As confidence grows, need in this log level falls. (Historically, this log level is often abused for metrics/tracing purposes, but that's another story)

- [WARNING] - program says "hey, human I don't know if this is good or bad, up to you to decide"

- [ERROR] - human attention required, program don't know how to handle situation automatically

Now, from this perspective, [WARNING] level of the compiler is basically compiler/spec giving up on telling human what's right and what's wrong. Given the compilers are written by people who know language spec better than anyone, it would safe to assume that compiler should know better than average developer what's good and what's not.

So when compiler tells develeoper "warning: I don't know if it's ok, deal it yourself", majority of the developers can only say "meh, I don't know either" and "doesn't crash, so we're good, ignore it".

That's why I strongly believe there should be no warnings in compilers at all. INFO and ERROR levels are sufficient for this kind of programs.

Any particular reason of downvoting without explaining what's wrong with my point of view?

This approach (no warning from compiler) is successfully used in Go, for example, and, I believe, the reasoning is similar.

Reply on news.ycombinator.com