As a third party to this debacle, all I have to go on is the comments people have made, but a few of them have asserted that that language was added later on and some initial versions of this change either outright lacked that warning, or else used styling tricks to make it difficult to notice. I lack the time and/or motivation to go digging through the commit history for that plugin, but maybe someone more enterprising could do so and verify if that warning was always present and presented in such a visible fashion.
There is a caution box in the description, which contains a different warning, and then the warning you have cited is in a second caution box in the "Notes" section, after the changelog and the examples.
It doesn't surprise me that people might not notice the existence of that second warning. I believe that most developers wouldn't scroll down to read the changelog and the example if they think they understood what the function does from its description.
It looks like you had to run the command explicitly to do that though, and that it wasn't done by default, so it seems pretty kosher to me if they had appropriate warnings (and I think the issue says they did...)
And even if it were, it's a matter of adding -Wno-misleading-indentation to make builds green again. Not a big deal. And besides, it's likely that fixing those warnings could be done mechanically. (and it's hardly true that every project that ships with -Werror has such misleading indentation somewhere in its code)
It's gotten better the last few years, but I still see that exact warning all the time in open-source projects. Nobody cares about warnings until someone mandates -Werror.
> Also, before upgrading the compilers on CI you need to fix any new warnings. But this isn’t a bad thing.
It's, in fact, downright stupid and means you're writing in a different programming language with each compiler upgrade and at the mercy of some poorly-considered third-party opinions.
Many compiler warnings are just stylistic. They can even contradict each other. Compiler A says, "suggest parentheses here". Oops, compiler B doesn't like it: "superfluous parentheses in line 42".
As a rule of thumb, warnings that aren't finding bugs aren't worth a damn.
The proper engineering approach is to evaluate newly available warnings and try them. Keep any that are uncovering real problems, or could prevent future ones; enable those and not any others.
Warnings are just tools. You pick the tools and control them, not the other way around.
Isn't it useful for the tool to warn about something that is often incorrect but sometimes intentional? The parent's example of implicit constructors is a good one, and a sibling comment mentioned fallthrough as another example. None of your suggestions help with that:
- It's not a bug in the tool (because the warning is often useful).
- A mention in a commit message doesn't help when you run the tool in future and get false alarms (oh I'll just read all the historic commit messages to see if any say these should be ignored).
- If you find such a comment at peer review then that doesn't give you anything actionable to do if the comment is being used correctly in one of these casees.
The solution in these cases is not "a little discipline" but simply to use the disabling comment and accept that this is reasonable.
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.
A long time ago, countless projects started using X in a way that wasn't documented (specifically, using X to define multiple modules in one file, instead of one module per file), and it has worked forever just fine. I consider it an unintended (and nice) feature.
X no longer wants to support this use-case, because they discovered that by not supporting it, they can clean up X's code a bit, and it makes X a little bit more efficient.
So X turned this into a warning, and is waiting for downstream projects to fix their (new) warnings, before axing this feature. But one popular project, Y, refused to make the chance to clear the warning (possibly hoping that X will be more careful to support, not break, idiomatic uses of X).
In the end, project Y voluntarily and without friction added a maintainer who would be willing to keep up with these changes (which were considered by Y to be frivolous). Eventually the system was renamed.
It's not a summary by number of words, but it if someone doesn't want to read the article, it's what I want them to be aware of.
It's frustrating the number of silly things you can do at the command line that persist for backwards compatibility reasons. But we can at least add warnings for them. It would have been much better for the user to have been told at the time they failed to add a netmask that they made a mistake. It would have removed the need for the support ticket entirely (at least, one hopes so.)
I've yet to be convinced that this makes sense for every warning. It really depends on the warning IMO.
Edit to elaborate:
I was mostly referring to non-committed/non-release code not deserving -Werror [1]. However, even for release builds, the story kind of depends on whether your builds are hermetic or not. If your commit also includes a snapshot of all your dependencies including your compiler toolchain executables, then off the top of my head, I can't think of any cases where you want warnings without errors (though perhaps there might be some). But if you're ever going to compile the same commit with a different toolchain (like say your system toolchain that you updated), then I don't think you want -Werror, otherwise every time the warning catches a new case, you'll fail to build something that previously compiled fine.
> Warnings are ignored because it's too hard or not possible to configure the compiler to only issue the warnings a particular developer finds useful and actionable.
That's why you don't do that based on the whims of an individual developer. Simply have a "no warnings" policy. Then the project decides what is enabled and what isn't.
A commit must not introduce warnings.
If a code change triggers a warning, it must be accompanied with a disabling of that diagnostic, which must then pass review so that it is peer approved.
If the warning cannot be disabled, the change must be reworked.
This "silly corner case" is not silly at all; it reveals a dangerous translation strategy in the compiler. The appropriate treatment isn't the issuance of a warning; rather, this translation strategy should be turned off unless explicitly requested by an exotic code generation option. (And then it can be applied without any warning.)
Indeed. I was hoping that the blog post would conclude with a section describing how they had learned an important lesson and would now be much more strict about dealing with this compiler warning in particular, and indeed compiler warnings in general, but there was nothing like that. Which implies it will quite likely happen again.
reply