Hacker News Reader

Comment by Macha | Hacker News Reader

It misses the big con for me:

It means your code may fail to compile on future compilers using your provided build scripts. Since C doesn't have a method of enforcing a specific compiler version like newer languages, that's a use case you need to prepare for.

The most recent example I was bitten for with this was with the vendored version of ring used in the redox project. My system had GCC 11, which added a new warning for mixing array and pointer syntax between a function declaration and function definition.

View on HN

Exactly right — and I think his intro explains the difference: if you’re used to working on closed source projects, you have dedicated staffing to deal with this ongoing work and it’s likely that all of the parties involved work for the same place. Open source maintainers rarely have anywhere near that much available time and will get random complaints from people around the world saying that their project sucks because some unreleased version of GCC on RISC-V gets a warning[1]. For cases like this, I generally go back to what the goal is: helping the developers be safer – turning it on in CI testing is nice because it gives feedback when you’re already working on it, but having it in build scripts run by strangers is inviting support requests.

1. This will take a couple weeks of back and forth before the person who filed it either updates to the latest version or admits that the error was in a modification they were working on.

And for uncommon platforms and compilers. Both C and C++ are very portable, and it's impossible for a project to account for every version of every compiler and every operating system and every architecture someone might end up compiling it for. It's super annoying when you have to fix a dependency failing to build for no real reason.

I'd suggest using -Werror for CI builds, passed into CFLAGS or CXXFLAGS by the builder, instead of hardcoding it into the project. Then you get most of the assurances from the article without breaking random stuff downstream.

And for packagers of distributions this would mean your program breaks way more often than it should.

I think it's a good idea to have -Werror on your internal CI build but not on by default for released tarballs. Especially not if you enable -Weverything as well. That WILL break with almost every compiler update.

Yeah, build with -Werror when you control the compiler, e.g. in your CI system, or whatever you use to produce build artifacts, but don't hardcode it in your build otherwise.

You should always control the compiler. The compiler is a dependency of your project, just like the libraries you use. The project should specify the version, just as it does for libraries. In Rust, you use the rust-toolchain file, in Gradle, set java.toolchain.languageVersion, etc. In my C++ projects, i use an ugly ball of shell script; the author of this article explains how to do it using Docker.

If someone wants to build the project with a different compiler, that's fine - they have to make the change and fix any resulting problems, just as if they were changing the version of a library.

Have you ever heard of libraries (as opposed to binaries)? Or open source software packaged by distributions?

It is difficult to assume good faith in your comment. Of course the parent has heard of libraries. It is sad to see they are being downvoted because they are quite correct.

In any professional project I've ever worked on, the compiler version has been strictly controlled. The entire team must use the same version of Visual Studio, or Xcode, or the JDK, or the Android build tools or whatever. You don't just let your developers upgrade whenever they want. Upgrading the build tools is an involved process that must be coordinated with the release cycle. All new warnings need to be fixed, all CI nodes need to be upgraded at once, scripts and metadata need fixes to work with the new tools, etc.

You are only approaching this issue from the perspective of open source software. That is not most software. The parent is correct: the compiler version is a dependency of your project and, in any serious development team, should be fixed for the whole team.

Ah, yes, so a "professional team" is one where 1. the deliverable are binaries (as opposed to source code), and which 2. is not open source. I get it.

The article we’re linking to very specifically asks open source software to use -Werror. They’re saying we should just have a Dockerfile, and force people to use Docker to compile an open source application, which is a rather disingenuous solution: Dockerfiles can only run Linux, and there’s a lot more than Linux out there (FreeBSD, etc.)

There was a time my open source project was running on Solaris, MacOS X, Linux (multiple distros), and Windows. Making something run only on Docker is a little restrictive by comparison.

Yeah, although in reality the OSS angle is kind of moot. There can only be one party in control of dependencies. If there is more than one party involved in the development of a product, only the downstream consumer can be in charge of dependencies. This happens all the time with closed source projects too, it's not specific to OSS.

There is a difference between being a leaf and a node in a tree of producers and consumers.

Thanks for your reply. With OSS, the exact compiler and build toolchain varies from user to user, so good OSS code tries to be as easy as possible to compile, i.e. avoid -Werror or anything else that deliberately stops compiling.

Of course, the GP tries to instill the idea that open source is not "professional" when in fact in OSS you will see far more variation in development practices and deployment practices than in closed source world. It's a strict superset.

use Nix instead of Docker, if platform support is your only concern. It also solves the issue with "good OSS code tries to be as easy as possible to compile" that you mention below.

From the article: “some (many?) folks in the open source and academic world hate -Werror with passion” (emphasis mine).

In the open source world, the developer can not control which compiler their users will use to try and compile their program. (OK, the article advocates forcing users to use Docker, but that significantly restricts who will be able to use their software.)

This leads to a lot of interesting coding style practices. For a while, I needed to always declare variables at the beginning of my functions because FreeBSD users at the time would make a stink because their compiler back then could not handle variables declared mid-function.

-Werror was unthinkable. More like, -Wall and people would make a stink on the mailing list when they saw compiler warnings. Of course, I would minimize them, but new compiler warnings come up with every GCC update (CLANG makes a lot fewer warnings), and I can just imagine what my GitHub issues page would look like if I were foolish enough to actually use -Werror

The linked article is wrong. Do not, repeat, do not use -Werror in environments where end users will compile the code themselves. Feel free, however, to use -Werror for CI builds on a Docker container with a known version of GCC.

This is hardly a significant issue. Many projects that have -Werror on by default support a --disable-werror configure flag. Your project can do the same.

As for compiler upgrades breaking things, everyone should be on the same compiler version to begin with. When a new Visual Studio version comes out you don't just let everyone upgrade right away, right? Someone installs it first and fixes up whatever issues arise with the new build tools. Then, at an appropriate point in the release cycle, they give the OK for everyone else to upgrade. Everyone upgrades at once including the CI nodes.

Why would GCC and Clang be any different? A serious project should have developers using a fixed version of GCC and other tools. Of course this doesn't help the random open source contributors who just use their system GCC but that's what --disable-werror is for.

This simply does not work for open source projects where you cannot control which compiler versions are used by people. Almost every time there is a new version of gcc or clang there will be a bunch of new warnings in any major code base. Additionally -Werror makes it harder to run git bisect.

Perhaps these projects should --enable-werror instead and use it internally, instead of pushing it to everyone else who is obviously not going to use the one blessed GCC that you have selected privately?

Ye, Werror can be really annoying for the user of the code. If your team are the only user it is fine, but otherwise it just breaks by compiler version bumps.

This has to be part of the understanding that any change or upgrade to the tool chain is a project in its own right that will require an amount of work in order to check that the code still compiles and that the resulting builds are satisfactory.

If you have a rule that there must be 0 warnings it has to be on the assumption that compiler warnings are red flags that need to be looked at. So you need to keep that discipline: if a new version of the compiler add warnings and your code trigger them then it means you need to spend the time to look at each occurrence and to 'fix' it.

> It means your code may fail to compile on future compilers using your provided build scripts.

Which is an advantage. If the compiler issues a warning that triggers your -Werror then your code may contain bugs. A new version of the compiler may introduce a novel optimization step that causes code that takes advantage of some undefined behavior to crash. The way to detect this is to read the compiler warnings.

Essentially, you the developer have only have two choices. 1) Users complaining about random mysterious crashes. 2) Users complaining about not being able to compile your software. 2 is always preferable to 1.

Especially in gcc and clang, many of the higher level warnings are more like style guide recommendations or "did you really mean this" warnings (like switch-case fallthrough, unused variables, or unused static functions).

Also the fix is simple: -Werror for development builds, but not release builds. This way users can still compile even if they upgraded to a new compiler versions but haven't yet or can't update dependencies.

I wrote a cryptographic library in C. The quality requirements for that kind of thing is pretty much "don't publish a single error, or Thomas Ptacek is going to haunt you for the rest of your life[1]". Or, you know, actual consequences that might arise from users using your broken crypto. Inevitably, I did publish some errors, one of which was actually critical. Here's how many would have been caught by a compiler warning not already present in GCC-8's -Wall -Wextra:

Zero. None. Nada. Zilch.

Mind you, I did have reports of some other compiler (mostly MSVC, but Clang with -Weverything as well) complaining about various weird things, including stuff like using bitwise operations on signed integers, or negation on an unsigned integer. They never lead to the discovery of an actual bug.

On the other hand, one very important thing I did was using sanitizers extensively. ASan, MSan, UBSan, Valgrind, even the TIS interpreter (and now TIS CI), so I could weed out as much undefined behaviour as I could. And boy did those tools find actual bugs. Those, coupled with a proper test suite, will find all the bugs. The bugs I failed to find in time can all be traced with either not using the strictest runtime sanitiser, or a hole in the test suite.

That being said, I still try very hard to be warning-free for most compilers out there. And I mostly am. But this is not about bugs. This is about sparing my users the headache that comes with investigating some trivial warning they should not care about.

Monocypher does not need -Werror.

[1]: I'm joking, he actually didn't.

All advice has exceptions. But I don't think it would be controversial to say that in most engineering organizations, -Werror will catch and prevent real bugs at a nontrivial rate.

I strongly suspect the reason is because they tolerate incomplete test suite, and don't use sanitisers enough, despite their use of an insanely unsafe language (one thing I absolutely hate about C is the sheer amount of undefined behaviour).

-Werror is just a stop gap desperate measure for when devs are pressured to write crap as quickly as possible. What's exceptional about Monocypher is not really that it's a cryptographic library (though that helps, modern cryptographic code tends to have pathologically simple control flow), it's that I took the time to polish it.

That being said, I am not against static analysis. On the contrary, I am huge fan of strong static type systems, and for languages like C a static analyser can be a huge help. I also trust John Carmack when he says running a static analyser has found non-trivial bugs in real code. But then I want real static analysis, that can assess actual risks of making mistakes. Compilers need to be fast, so their warnings naturally tend to be crude. And if they start complaining about stuff like whitespace in a language where whitespace has no semantics, that may hurt more than it helps.

"Just write good code" is not a workable plan for most organizations of any size. You often want workflows that are resistant to "we promoted somebody to TL when they shouldn't have been and they are not doing a very good job" situations.

Yes, it is outrageous that anybody still writes programs in C or C++, which basically guarantee that you'll write oodles of subtle bugs. Yes, static analysis that finds problems that you can't train humans to find is more interesting. Yes, sanitizers and fuzzing should be mandatory for any serious project in C or C++.

But "hey, you've got a vacuously true branch predicate" is valuable in a large number of cases even if humans can catch that with tests, since most organizations are not able to successfully hire only excellent engineers and give them sufficient time to never be rushed.

Just a few weeks ago Google shipped a branch condition that included & rather than && that broke all Chromebook logins. I submit this as evidence that it it not possible to construct a large organization that is resistant to bad code without automation. Yes, a lot of other things needed to go wrong to enable this code to reach production but so many of those processes are either done by humans (and therefore error prone) or produce the same complaints when mandated by automation (e.g., 100% line coverage).

> Google shipped a branch condition that included & rather than && that broke all Chromebook logins

To be fair to C++, that's not the sort of issue that can really be blamed on the language, its unsafety, or its many footguns.

Did they skimp on testing?

I don't know the testing situation.

But there is a lesson from other forms of engineering here. Almost no failure has a single cause. A variety of things need to go wrong. "Just test better" is not a strategy. Any honest view of large scale software engineering has to internalize that all of the human processes can fail and even the technical processes can fail and so you need layers of protection.

People should try to write correct code. Sometimes they will fail.

People should review code carefully. Sometimes they will fail.

People should test code thoroughly. Sometimes they will fail.

Tooling should automate issue detection with static analysis and fuzzing. Sometimes it will fail.

Tooling should automate issue detection with canaries and monitoring. Sometimes it will fail.

Maybe the team skimped on testing as a rule. Maybe the engineers who submitted and review the code were tired that day and didn't test it well. Maybe something in the the CI was flakey so they did a force push. I don't know.

But I do know that making it harder to do weird things like using bitwise-and in a branch predicate with something other than a constant mask adds an extra layer of defense.

> I do know that making it harder to do weird things like using bitwise-and in a branch predicate with something other than a constant mask adds an extra layer of defense.

Sure, and perhaps I was mistaken in saying this isn't due to C++'s footguns. The & / && operators do look similar. Ada uses and / and then, which is more distinctive.

No, it is rare for compiler warnings to be about important errors. But once in a blue moon they are, so why not be cautious? While your library is fairly small (only 3k lines), so I don't know what conclusions can be drawn from it, I checked out a version of it from 2017. Compiling it with gcc 11 generates only two warnings:

x25519.c:9:22: warning: '_0' defined but not used [-Wunused-const-variable=] 9 | static const uint8_t _0[16]; | ^~ ed25519.c:302:14: warning: argument 1 of type 'u8[64]' {aka 'unsigned char[64]'} with mismatched bound [-Warray-parameter=] 302 | void HASH(u8 hash[64], const u8 in, size_t inlen); | ~~~^~~~~~~~ In file included from ed25519.c:285: sha512.h:19:29: note: previously declared as 'uint8_t ' {aka 'unsigned char '} 19 | void crypto_sha512(uint8_t out,const uint8_t *input, size_t input_size);

Thus it appears to me that both the benefit and the cost of -Werror for your library is small.

> But once in a blue moon they are, so why not be cautious?

You have to balance the inconvenience of false positives against the consequences of false negatives. If the false negatives are rare enough, compared to the false positives, it might be best not to be cautious.

Still, in practice I am fairly cautious, and tolerate zero warning. When someone reports a warning to me, I always try to fix it. So in practice, what I do is fairly close to -Werror, with one big difference: if some compiler comes up with some new warning, or if a set of compilers are impossible to please at the same time, my users aren't blocked. Which is important, because new warnings are now overwhelmingly likely to be false positives.

That mismatch bound you just showed however does set off my alarm bells. I'll need to make sure I actually got rid of it.

A warning being thrown for a user which is caused by a bug that leads to mysterious crashes in a codebase which is warning free for the developer is something which is almost unobservably rare. So sure, a user getting a failed compile is superior to a mysterious crash, but that isn't the actual tradeoff in practice.

Instead, if you have a post compile test suite which covers extensively all branches in the code you will catch things like miscompilation that warnings have little chance of catching. And well constructed the tests can have a zero rate of false positives-- which isn't something you'll get from warnings.

So another choice: take the time you'd spend on the sisyphean task of dealing with random failures due to Werror new/old/different toolchains and invest it on making better post compile tests and save the Werror for your CI where its time wastage is more proportional to the benefit it provides.

> Which is an advantage. If the compiler issues a warning that triggers your -Werror then your code may contain bugs.

Even if the compiler doesn't issue a warning that triggers -Werror, your code may contain bugs. This is the root of the false dichotomy you present.

> Since C doesn't have a method of enforcing a specific compiler version

That's a question of the compiler you'd enforce rather than C itself. You could enforce compiling with a specific GCC version, for example-- but you don't want to do that. After all, eventually someone is going to want to run your code on RISC-V or something where the enforced compiler doesn't work.

Note that this con applies even if noone besides you builds the code as you might want to build older snapthots in the future, e.g. for git bisect.

Is this another pro in disguise?

Language compilation environments themselves are vulnerable to security vulnerabilities, like libraries... Practices previously considered acceptable may become harmful if they are revealed to create common correctness exploits in code. So if code that compiles under a previous version now fails to compile in -Werror on a subsequent version, that's a signal to the maintainer to either fix the code to be in compliance with best practices or manually disable that specific error with an explanation... A signal they'd miss if they didn't turn all the warnings on and stop compilation by default.

>> It means your code may fail to compile on future compilers using your provided build scripts.

Seems fishy. Changing compilers sounds like a hypothetical. If you know you're going to use say GCC on Linux, LLVM on maxOS, and MSVC on Windows then set that up and work through the problems. Otherwise, what future compiler change? Also, once all the warnings have been cleaned up there won't be that many when you change compiler.

Changing compilers to future versions is not "a hypothetical" for the vast majority of software.

> there won't be that many when you change compiler.

When people build your software as a dependency, as in the parents example, any error you didn't cover yet is a problem.

>> When people build your software as a dependency...

When your project is a dependency for others, I think you've got even more responsibility to use these flags and fix the issues. If not, you're denying your end users the ability to use those options for building their project.

I'm looking at you GTK.

Of course you should fix the issues. But you shouldn't have the build scripts etc you ship and that your users use trip them.

> Changing compilers sounds like a hypothetical.

How? It’s happened many times before – in the 90s, most people used the platform compilers; then most projects switched to GCC for {price,stability,features}; Mac developers flirted with things like xlc towards the end of the PowerPC era; lots of people switched to LLVM — many projects have scar tissue from those past transitions still scattered around their code and build scripts!

Moreover, you will certainly be switching to newer versions of the same compiler. Your code might fine now but after the compiler team helpfully identifies additional areas for concern the same code will raise new warnings.

If you're a library author you generally can't dictate what compiler to use (within reason of course). IME a surprisingly high number of people use MingW/GCC or Clang to build Windows projects for instance (and this works surprisingly well too).

Also: new versions of the same compiler add new warnings, so you need to account for that anyway, unless you want to stick forever on an old compiler version (I've worked on projects where upgrading to a new compiler version was often delayed for exactly that reason).

Reply on news.ycombinator.com