[HN Gopher] Not -Werror considered harmful
___________________________________________________________________
Not -Werror considered harmful
Author : ingve
Score : 59 points
Date : 2021-10-04 12:46 UTC (10 hours ago)
(HTM) web link (rsalsamendi.github.io)
(TXT) w3m dump (rsalsamendi.github.io)
| ulnarkressty wrote:
| Even the VC++ team at Microsoft only targets clean /W4
| compilation [0]. If one enables /Wall there's hundreds of
| warnings in the standard library files [1]. Not to mention any
| dependency one might have (looking at you, Boost)...
|
| [0]
| https://www.reddit.com/r/cpp/comments/bubg7b/msvc_compiler_w...
|
| [1] https://developercommunity.visualstudio.com/t/c-include-
| file...
| ludocode wrote:
| I agree with the spirit of this article: -Werror is a very good
| thing and should be on in any serious codebase. But the rebuttals
| for the cons are pretty weak.
|
| For Con #3, "make I=0" flag is cute, but the final "make" doesn't
| actually work. Any files that had warnings but still compiled
| successfully will not get recompiled. The simplest workaround is
| to "make clean" first but this can be time consuming (ccache
| helps.) Another workaround is a script that touches any unclean
| or just-committed git files but this is error-prone. It's
| possible to have "make" rebuild any files that were built with
| I=0 but it adds a lot more Makefile complexity. And in any case
| all of these are manual workarounds that are easy to forget.
|
| For Con #4, a docker image doesn't help you build for Windows or
| macOS. You need full VMs to test on them (and a macOS VM violates
| the license, so technically you need a full Mac somewhere you can
| SSH into.) This process can be optimized pretty well but it's
| still another manual step.
| daemin wrote:
| I'm sticking by the principle that warnings should be warnings
| and errors should be errors, and that you shouldn't "promote"
| warnings to be errors. I can't remember who said it but I think
| it was in a conference video.
|
| People often claim that without promoting warnings to be errors
| and thus stopping compilation that you'll just get a lot of
| warnings. However in reality most of these warnings just get
| disabled in the code, and often in a way that disables them for
| the entire project, meaning that you're missing out on good
| diagnostic warnings.
|
| The proper way to fix warnings is to keep them as warnings and
| have it on the team to make sure they don't get out of hand. By
| making sure when you implement a new feature or fix a bug that
| you don't introduce new warnings, and also have people
| investigate warnings and fix them in the correct and sustainable
| way. In some cases you would disable that warnings but in general
| they should be fixed by fixing the code. That way you get the
| benefit of having good diagnostics along with the flexibility of
| being able to investigate and fix it at a later date.
|
| One final note is that having warnings show up during the compile
| is a good indicator of how much technical debt there is in the
| project. If you just end up disabling warnings you're really
| hiding how bad the code state is.
| krzyk wrote:
| > People often claim that without promoting warnings to be
| errors and thus stopping compilation that you'll just get a lot
| of warnings.
|
| This, do you know people that look at warnings that don't break
| the code/compilation?
|
| Those are usually ignored, that's why one can promote them to
| errors.
| adrianN wrote:
| If you disable warnings such that they're disabled for the
| whole project you're just doing it wrong imho. Warnings need to
| be fixed by making the compiler happy and only (locally!)
| disabling them if there is a bug in the compiler.
| daemin wrote:
| Yes you should only disable them locally, in as small a
| region as possible. Though in a lot of cases in my experience
| this isn't so. They're generally bulk disabled before
| including external third party headers, they're disabled in
| low level projects, they're disabled in headers so that they
| can be submitted.
|
| The point is that people often disable them in ways which
| they shouldn't just so they can compile their code and get on
| with fixing their bug or implementing their feature. Mainly
| because warnings get treated like errors and prevent the
| build from succeeding.
|
| With warnings as warnings we could take a more caretaker
| approach to dealing with them and we shouldn't have these
| situations created.
| bjourne wrote:
| I think your comment illustrates well why treating
| "warnings as warnings" is counterproductive. People who
| treat warnings as warnings _haven 't_ taken a caretaker
| approach and so they have accumulated and are now so
| numerous that bulk disabling them is the only practical
| option!
| daemin wrote:
| I have worked at (and are currently working at) places
| that treat warnings as errors, and if the warning is not
| trivial to fix it just ends up disabled. People do notice
| it during code reviews but it usually goes through as
| someone "will fix it later".
|
| So out of sight and out of mind.
| lupire wrote:
| Why do you think that people who are so undisciplined that
| they disabled warnings, are so disciplined that they will
| fix warnings? Not disabling a warning is far easier than
| fixing it.
|
| A warning is only useful if it's _someone else 's place to
| fix_ and it's warning you that you might suffer or you they
| might suffer from misusing you code. Warnings are most
| useful at runtime where it's impossible to stop and fix.
| adrianN wrote:
| I don't know. If your process is not up to the task of
| preventing people from bulk-disabling warnings, I wouldn't
| trust the process to adequately inspect warnings that don't
| prevent shipping.
| daemin wrote:
| Well it will require a behaviour change either way. I
| think that keeping warnings as warnings will at least
| make it visible how much debt there is to fix in the
| project rather than hiding it behind warning disabling
| pragmas.
| outsomnia wrote:
| > The proper way to fix warnings is to keep them as warnings
| and have it on the team to make sure they don't get out of
| hand.
|
| "Proper"... no, warnings just scroll by and are forgotten.
| Error stop the build and make you keep the number of them at
| zero.
|
| If you allow any warnings, they will increase according to the
| tolerance of the most lax person on your team. And you will
| never spend the time on the technical debt to clean them. You
| are accepting an increasing number of things wrong with your
| codebase that you will never investigate nor regain control of.
| devwastaken wrote:
| Either you have warnings, or you have no warnings. The
| problem with no warnings is not that the warnings were
| "fixed", but that they get disabled. That is far more
| problematic.
| krzyk wrote:
| Disabled or just shown, doesn't matter they will
| proliferate unless turned into errors.
| daemin wrote:
| If they're disabled then they will proliferate even if
| they are turned into errors.
|
| Better to be visible and culled promptly than disabled in
| code and hidden away.
| nullc wrote:
| > no, warnings just scroll by and are forgotten
|
| Sounds like a bad team/management, not anything to do with
| werror.
|
| Plenty of groups have no problems maintaining warning free
| code on a target toolchain without using Werror.
|
| Disrupting what people are working one sometimes encourages
| "just make it go" fixes that introduce bugs to silence the
| warning.
|
| If the simplest change that silenced the warning was always
| the right one the compiler could just do it for you and not
| warn. :)
| daemin wrote:
| I never said it was easy, it does require the team to have
| some discipline about it, but it doesn't sweep the issue that
| the warning was about under the rug, hidden from view.
|
| If you have a lot of warnings then you have a lot of issues
| with the code. Would you rather have them visible or
| disabled?
| outsomnia wrote:
| They are not 'visible' if you just let them scroll by.
|
| If there are individual warnings that have no meaning, turn
| them off.
|
| Everything else, is help from the compiler, which is your
| only friend. Ignoring even one of those should not be
| tolerated at all. They must be analyzed and the code
| improved to remove them. And you enforce your team
| following that policy with -Werror.
| daemin wrote:
| Yeah, but push comes to shove during shipping, people
| leave, and the pragmas remain in the code in various
| places.
|
| At least if they are scrolling by then people can get fed
| up with that and take some initiative to fix those
| warnings. Or the new people joining your studio can come
| in and try to fix them.
|
| It's much more difficult to get people to fix pragmas
| scattered throughout the code. Which ones were legit,
| which ones were put in at the last moment out of
| frustration, which ones were put in temporarily years
| ago?
|
| I think a softer approach works better since people won't
| be so quick to work around the compiler by disabling the
| warnings.
| saagarjha wrote:
| > If you allow any warnings, they will increase according to
| the tolerance of the most lax person on your team. And you
| will never spend the time on the technical debt to clean
| them. You are accepting an increasing number of things wrong
| with your codebase that you will never investigate nor regain
| control of.
|
| This is making a lot of assumptions about team dynamics, and
| fails to account for the person who notices build warnings
| creep and aggressively sends in patches to fix them.
| bhaak wrote:
| You can promote only specific warnings, for example
| -Werror=format.
|
| This way you can still use the compiler to stop you reliably
| from committing unwanted code constructs.
| CodesInChaos wrote:
| I think gating pull-requests on -Werror makes sense, at least for
| your own code. That way you keep the development branch warning
| free on the current compiler version. (An override for backported
| fixes and other exception cases is still a good idea)
|
| But I would not use -Werror when building existing releases, so
| you can still build old versions of your code using newer
| compilers.
|
| This article seems to only consider the pull-request scenario
| when talking about -Werror being fine, while the open-source
| people opposing -Werror are mostly concerned with building
| existing releases.
|
| For example rust's stability guarantees do not cover new
| warnings, so your code can stop building when upgrading the
| compiler if you use its -Werror equivalent.
| pornel wrote:
| C doesn't have a reliable portable way to silence a known
| warning. Inevitably, some warnings will be
| irrelevant/imprecise/overzealous (after all, they're only
| warnings, not errors).
|
| To silence warnings in C without vendor-specific pragmas, people
| fiddle with the code until the warning goes away. This can be
| messy, cause bugs, and isn't even guaranteed to work on any other
| compiler, including a newer version of the same compiler.
|
| So lack of -Werror is harmful, and -Werror is harmful too. C
| needs to sort it out.
| jpegqs wrote:
| I'm porting Linux packages that compile for GCC/Clang to an EDG
| frontend based compiler and I need to remove forced -Werror from
| build scripts because different compilers have different warnings
| (and even false warnings due to compiler bugs). So what I want to
| say to people who force -Werror by default is that your software
| is no longer portable. Do not do this.
| flohofwoe wrote:
| Unfortunately it's not as simple as always enabling -Werror:
|
| - new compiler versions regularly introduce new warnings, not a
| bad thing in general of course, but:
|
| - even if your own code is warning-clean (which it really should
| be!), dependencies are usually full of warnings especially after
| upgrading compiler versions
|
| - some compilers issue different warnings based on compile
| settings (e.g. optimization level)
|
| - there's a threshold where picky warnings stop being all that
| useful. For instance hardly any C/C++ code I've encountered is
| -Wsign-compare clean, and some APIs are not designed with this
| sort of warning in mind, so you have the option of littering your
| code with explicit casts (which makes it less readable), or just
| ignore that warning and move on.
|
| My own strategy is: enable -Werror for development and keep your
| own code warning free, while locally supressing warnings in
| dependencies. For release builds, don't use -Werror (because you
| can't know what compiler version your users are using).
|
| There's also the theoretical feature that headers included with
| '#include <...>' don't generate warnings, but I haven't gotten
| this to work reliably across compilers.
| nullc wrote:
| > some compilers issue different warnings based on compile
| settings
|
| Also based on which platform they're run on.
|
| > new compiler versions regularly introduce new warnings,
|
| Much worse, IMO is _older_ compiler versions which emit
| incorrect warnings. At least when a new version emits a new,
| correct, warning it 's at least something you want to fix (or
| disabling a new unhelpful warning). The failure to compile may
| still be a practical problem but at least the direction it
| creates is a right one.
|
| For older compiler versions, there may be no fix-- breaking the
| code or turning off the warning may be the only options if
| Werror is used.
|
| Outside of heavily controlled environments, I think Werror is
| just plain toxic. Want to run it on your CI builds where you
| don't need to worry about the compiler being upgraded or
| downgraded without warning? Great! Everyone should do that.
| Being warning clean on your primary target toolchains(s) is an
| important move.
|
| For code distributed to users the use of Werror use creates a
| pressure against introducing new and helpful warnings for
| compiler authors. Fortunately, it's such a nuisance for code
| distributed to others that no one survives enabling it for long
| and the worse damage it likely does is that the few projects
| that use it for publicly distributed code refrain from enabling
| many otherwise useful non-default warnings.
| mbatza wrote:
| > dependencies are usually full of warnings > locally
| supressing warnings in dependencies
|
| I don't see any reason to enable warnings for dependencies in
| your project's build system. I suggest building dependencies
| with (as much as possible, considering ABI compatibility) their
| own preferred compiler flags (and ideally, with their own build
| systems), and use your compiler's equivalent of `-isystem` to
| suppress warnings in their headers.
| HelloNurse wrote:
| -Wsign-compare is a typical example of warning that implies a
| serious error unless you prove it couldn't happen; spending,
| typically, more effort than would be needed to actually fix the
| warning.
| flohofwoe wrote:
| Well yeah, but then there's stupid design warts like size_t
| being unsigned, and it's much too late to fix this now.
| microtherion wrote:
| How many objects of negative size are you typically dealing
| with?
| rkeene2 wrote:
| This is why I use `-Werror` in maintainer mode but not
| otherwise.
| [deleted]
| nuerow wrote:
| > Unfortunately it's not as simple as always enabling -Werror:
|
| I was already bitten in the backside by Werror. Allow me to
| tell you how.
|
| I was working on a legacy C++ project and my employer uses an
| in-house build system that automatically builds the whole
| dependency tree if any dependency changes. A previous team was
| a staunch supporter of the "no Werror considered harmful"
| philosophy, and made it their point to even pass it to unit
| tests.
|
| That's all fine and dandy, except compilers are upgraded and
| older compiler versions are deprecated out of our reach.
| Consequently, when I did a minor update to my project and
| launched a build job, the entire dependency tree started
| failing left and right, throwing a million error messages as a
| packed wall of text. The root cause? The current version of GCC
| was updated to throw warnings regarding some inane drivel
| involving white spaces, and with Werror passed everywhere those
| were upgraded to full blown errors.
|
| The end result was a couple of weeks wasted going through the
| dependency tree to tackle warnings, because otherwise the build
| pipeline failed spectacularly.
| danuker wrote:
| > inane drivel involving white spaces
|
| I suspect you wouldn't enjoy Python.
| throwaway889900 wrote:
| Treating a stylistic warning as an error is completely
| different than a semantic error being an actual error.
| dv_dt wrote:
| This is not a reason to avoid -Werror, but a reason to
| approach changes in nontrivial build environments like one
| would approach significant changes with the software itself.
| nuerow wrote:
| > This is not a reason to avoid -Werror
|
| Sorry but it really was. Passing Werror like confetti was
| the root cause of a problem that led to two weeks of
| downtime and added absolutely no value at all, and the
| solution consisted of addressing said warnings alone. And
| if there was any doubt, the upgrade was undoubtedly
| trivial, mainly due to the fact that the project still
| targeted C++11, and with exception of the Werror nonsense
| was absolutely flawless.
| MaulingMonkey wrote:
| > and the solution consisted of addressing said warnings
| alone.
|
| Alternative solutions which should _not_ have taken two
| weeks would 've included: -Wno-inane-
| drivel-involving-white-spaces -Wno-error=inane-
| drivel-involving-white-spaces
|
| Just because you're using -Werror doesn't mean literally
| every single warning in the compiler ever should be a
| hard error. It means errors should be the _default_ ,
| allowing you to opt-out of stupid warnings-as-errors,
| rather than needing to opt-in to critical warnings-as-
| errors.
| emidln wrote:
| This seems like your compiler toolchain definition should
| have been a part of your build system's graph. I do this
| exact thing, with -Werror on, and bumping to a new gcc
| version or clang version typically exposes new errors that
| will be blocking on gerrit until I fix the corresponding now
| warning code. Bazel makes this easy, but you can definitely
| do it with CMake or other systems too.
| GuB-42 wrote:
| -Werror is fine as something you enable in your personal
| development process to make sure you didn't miss a warning.
|
| But other than that, it is a bad idea.
|
| Compiler warnings are just another static analyzer, on equal
| footing with cppcheck, clang-tidy, etc... It is just convenient
| because it is built-in. So you can make a policy that in order to
| push something, it has to pass a variety of automatic checks
| including compiler warnings, static analysis, unit tests,
| documentation, coverage, etc... And -Werror can be used for that,
| though I think it would be better to let the compilation go
| though to make a complete report.
|
| The big problem with -Werror is that what a compiler considers a
| warning depends on the version of the compiler. It means you will
| get new warnings as you upgrade the compiler, so you will be
| tempted to stay with the old compiler to silence the new
| warnings, because otherwise, your code stops compiling. Terrible
| idea.
|
| Another con that is not mentioned is a typical problem when you
| have strict rules you can't ignore. People will do what it takes
| to make the warning disappear without actually solving the
| problem. For example, there is a warning if an enum value is not
| referenced in a switch/case with no default. It is a very useful
| warning, it tells you that some case is not implemented, and
| IMHO, you should leave the warning until it is properly done. But
| let's say you need to move forward and that annoying -Werror
| won't let you. You just need to add and empty default case,
| silencing the warning, not fixing the problem, forgetting it, and
| shipping buggy code. And maybe you won't do these ugly hacks
| (really?) but someone else will, I've seen such things done in
| certified code, the kind that flies airplanes...
| jwlake wrote:
| I think the problem with this article is it makes a ton of
| assumptions. For an open source dependency used in a ton of
| projects and platform, -Werror tends to be fairly infeasible. For
| a specific CI build on a specific platform, it tends to be a good
| idea. The confounder is always -Wall, which is overkill alot of
| the time.
| baybal2 wrote:
| One more thing:
|
| If you ever decide on to do formal verification of your code,
| getting it into shape will probably take as much as the
| verification itself.
|
| Being super-duper strict from day 1 of the project makes it so
| much easier.
| jmull wrote:
| The fundamental confusion of the article (and come commenters
| here) is whether you primarily release code or binaries.
|
| If your primary product is a binary (even if you release code too
| for reference or to be open), by all means, use -Werror (and
| strictly set the supported compiler).
|
| If your product is code (even if you release binaries too as a
| convenience), -Werror in the release is an impediment to your
| users because you've introduced a dependency on a specific
| compiler version. Think of what will happen when a user tries to
| use another source library using -Werror... but pinned to a
| different compiler or compiler version. You've created a problem
| with your project that your users will have to solve.
|
| You could dictate the the build environment of your users, but
| that's simply a roundabout way to actually be in the business of
| releasing binaries.
|
| For some projects, I think you _can_ decide to dictate the use of
| -Werror (and compiler version) for certain code changes
| processes, but that 's a judgement call, with pros and cons. A
| strength of open source is that the distinction between user and
| contributer can be blurry. E.g., imagine a user runs your library
| on a platform you have neither the time nor expertise to support,
| finds and fixes some issues and is willing to contribute those
| changes back... are you sure you want to put them in the position
| of addressing errors from a toolchain they don't know or
| understand? Maybe. But maybe not.
| jcelerier wrote:
| > because you've introduced a dependency on a specific compiler
| version
|
| No, if you get warnings, it means that your code is (and was
| already) wrong. And if you wrote wrong code you already have an
| implicit dependency on your compiler version, as more recent
| versions with more advanced static analysis may turn a semi-
| harmless unrecognized UB as a very harmful recognized one (and
| thus warn). I had this case a few times already, see a simple
| repro : https://gcc.godbolt.org/z/jY69sas8K
|
| -Werror just makes this dependency explicit.
| josefx wrote:
| > No, if you get warnings, it means that your code is (and
| was already) wrong.
|
| I get dozens of warnings for unused variables, signed
| unsigned conversions (where the value range is API enforced),
| etc. . Long ago we had thousands of warnings and went through
| our code base just to get rid of them, I would be surprised
| if we fixed even one bug while doing that, most was just
| adding some verbose construct to be explicit about what was
| already happening. Warnings can be helpful, which is why I
| still have them enabled, but not everyone is.
| asveikau wrote:
| Have you not seen a warning that appeared frivolous or you
| disagreed with? Or, as my sibling comment says, might be a
| harmful construct in some contexts but used correctly in your
| example?
|
| Further, the compilers are evolving entities written by mere
| mortals, with their own bugs, occasional poor decisions, and
| yes, chances to be corrected over time. If the compiler's
| authors decide a warning is too noisy and disable it in
| release X, but random linux distro still uses (X - 0.1),
| should you fix it for them...?
| saagarjha wrote:
| > No, if you get warnings, it means that your code is (and
| was already) wrong.
|
| No, that's not right, otherwise they wouldn't be _warnings_.
| The whole point of them existing is the compiler trying to
| alert you of a suspicious construct, but it 's not confident
| enough to be sure it's a bug. Sometimes I _want_ to use
| assignment for its result-that 's not a bug, it's just
| something that is often done accidentally.
| tialaramex wrote:
| There's a big _language_ problem here. It 's not the whole
| of the issue, but it's certainly a big component.
|
| In C and C++ there are a _huge_ number of things that are
| legal, and thus must not cause an error diagnostic (out of
| the box) - and yet are obviously not what anybody should do
| (and so a warning diagnostic is appropriate if the compiler
| can successfully diagnose them).
|
| You give the example of "sometimes I want to use assignment
| for its result". A better language outright forbids this.
| If you want the result, write that, if you want an
| assignment, write that, if you want both, _write both_ so
| that it 's clear what you intended, and let the optimiser
| turn your clear program into efficient machine code rather
| than trying to out-guess the machine.
|
| Ideally warnings would not be red flags. If warnings are
| stuff like "This variable's name start with a capital
| letter when that's contrary to standard style guides." or
| "You keep saying 15 in this program, have you considered
| just defining a constant instead for whatever 15 is?" then
| you're in a better place, but unfortunately in C the
| warning is more likely to be "99.99% of the time this code
| is a buffer overflow, so, you probably don't mean that" or
| "It looks like this does X doesn't it? But, it actually
| does nothing at all, and for complicated reasons we can't
| make it do X, so, I really hope you didn't mean X."
| pizlonator wrote:
| You hit the nail on the head!
|
| And the same exact critique applies to folks saying "-Werror is
| harmful". That statement is false unless you also add the
| limiting principle: "-Werror is harmful in software shipped as
| source that users build themselves".
|
| This is a classic case of folks talking past each other. Kinda
| fun to watch actually.
| SAI_Peregrinus wrote:
| The issue IMO isn't -Werror, it's -Wall, -Wextra, and
| -Weverything. If you individually specify the warnings you
| want, other compiler versions won't have a different list. So
| -Werror won't cause your build to fail because some other
| compiler (or version) had a different -Wall list.
| Macha wrote:
| 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.
| bjourne wrote:
| > 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.
| slavik81 wrote:
| > 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.
| loup-vaillant wrote:
| 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._
| UncleMeat wrote:
| 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.
| loup-vaillant wrote:
| 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.
| UncleMeat wrote:
| "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).
| MaxBarraclough wrote:
| > 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?
| UncleMeat wrote:
| 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.
| bjourne wrote:
| 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.
| flohofwoe wrote:
| 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.
| nullc wrote:
| 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.
| phkahler wrote:
| >> 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.
| detaro wrote:
| 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.
| phkahler wrote:
| >> 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.
| detaro wrote:
| 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.
| acdha wrote:
| > 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.
| flohofwoe wrote:
| 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).
| sjmulder wrote:
| 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.
| acdha wrote:
| 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.
| shadowgovt wrote:
| 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.
| account42 wrote:
| 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.
| mytailorisrich wrote:
| 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.
| rightbyte wrote:
| 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.
| ludocode wrote:
| 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.
| [deleted]
| saagarjha wrote:
| 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?
| jeltz wrote:
| 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.
| bhaak wrote:
| 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.
| 4ad wrote:
| 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.
| twic wrote:
| 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.
| 4ad wrote:
| Have you ever heard of libraries (as opposed to binaries)?
| Or open source software packaged by distributions?
| ludocode wrote:
| 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.
| 4ad wrote:
| 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.
| strenholme wrote:
| 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.
| ghostwriter wrote:
| 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.
| 4ad wrote:
| 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.
| strenholme wrote:
| 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.
| 4ad wrote:
| 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.
| strenholme wrote:
| 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.
| nullc wrote:
| > 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.
| outsomnia wrote:
| Exactly. -Wextra should also be in the same boat as -Werror and
| -Wall.
|
| How to deal with different toolchains having different approaches
| to what warnings are enabled? Assertively disable the handful of
| stupid warnings like -Wno-unused-parameter, and have your CI
| encompass most toolchains such that the union of their warnings
| is your target and break build if any blow warnings.
|
| Then you will have something robust and portable.
| saagarjha wrote:
| Robust and portable, until someone tries compiling it with LLVM
| 13 that you haven't tested on...
| steerablesafe wrote:
| You can enable -Werror with a known compiler all you want for
| development, but if you package and distribute your sources then
| turn that off by default.
| Cthulhu_ wrote:
| If you're still getting warnings that you think should not be
| ignored (which is -Werror iirc) in your build by the time
| you're ready to distribute, you need to take a hard look at
| your development and merge process, too. Ideally nothing goes
| into the main branch if it's not done and ready to be deployed.
|
| One of the things we finally managed to do at a previous job
| was "master=live", where a build was automatically triggered
| that put the product ('just' a website, but still) live in
| production. While I'm not advocating this per se, you SHOULD
| put everything in place so that you could do that. (see also:
| continuous integration vs continuous deployment).
| remram wrote:
| Compilers get new diagnostics all the time.
| hzhou321 wrote:
| For me, the biggest con is the bad code developers commit in
| order to suppress warnings, both true positives and false
| positives. The latter really pushes me in the camp of haters
| firmly. The more warnings you turn on, the more false positives
| you will encounter. Any opinions should acknowledge the balance
| first.
| IshKebab wrote:
| I feel like only the last point (your build will fail using
| different compilers / compiler versions) is the only serious
| issue.
|
| But "just build it using Docker" is a pretty awful solution.
| Doing stuff in Docker is just extra pain in general, and it
| doesn't even work on Mac or Windows.
|
| I think a better option I've seen is just to have a config flag
| to disable `-Werror`.
| strenholme wrote:
| Docker runs fine on a Mac. Docker kinda sorta runs in Windows,
| but doesn't play well with VMware player, to the point I gave
| up on asking my users to use Docker in Windows (I briefly
| flirted with the idea until I tried actually installing Docker
| in Windows).
|
| Docker doesn't work in any of the BSDs, it doesn't work in
| Solaris, it doesn't work in HaikuOS, and it doesn't work in
| anything resembling an embedded space. Saying "you must use
| Docker to compile my software" severely limits where it can
| actually be used.
|
| I say all this as someone who uses a Docker container for my CI
| testing step (once a day whenever my code is changed, I compile
| and test it in a Docker container).
| IshKebab wrote:
| Docker is always used for running _Linux_. Even on Mac or
| Windows Docker is running Linux.
|
| That's no help if you're building software for Mac or
| Windows.
| zxcvbn4038 wrote:
| "X considered harmful" should be a meme at this point. It is
| basically an opinion piece in someone's personal blog. I wouldn't
| be surprised if there is a "too much compiler output considered
| harmful" counter argument in a blog someplace.
| wffurr wrote:
| It obviously is a "meme" as in a bit of knowledge that gets
| passed around and mutates and spreads. Do you mean something
| else by that?
|
| Perhaps you mean we should disregard titles like that?
|
| I think it's a fine title for a polemic. You can interpret and
| grant as much weight as you like based on the presented
| arguments and the author's bona fides. Just like any other
| essay.
| yakubin wrote:
| It's boring. Every week there is a post with a title of this
| form. It's as if mindless bots were writing the titles, not
| creative humans.
| yakubin wrote:
| Addendum: just today there are two of those on the front
| page:
|
| [1]: <https://news.ycombinator.com/item?id=28746549>
|
| [2]: <https://news.ycombinator.com/item?id=28745949>
| st_goliath wrote:
| "X considered harmful"
|
| "Everything you always wanted to know about X, but were afraid
| to ask"
|
| "How I learnt to stop worrying and love X"
|
| For me personally, at this point, I have seen so many articles
| and conference talks with one of those titles, it's starting to
| genuinely annoy me. Thankfully, the 2016 trend of "make X great
| again" didn't last that long.
| [deleted]
| jerf wrote:
| It is a meme: https://meyerweb.com/eric/comment/chech.html
|
| That doesn't have a date on the page but there's an HN link
| back at least to 2015 and I think it's older than that.
| bayindirh wrote:
| It all started in 1968. From the page:
|
| Edsger W. Dijkstra's note in the March 1968 Communications of
| the ACM, "Go To Statement Considered Harmful,"
| bayindirh wrote:
| I think it's used a lot since it triggers a ton of heated
| engagement in a very short time.
|
| I personally tend to not read these pieces anymore. I'm over my
| "I have strong opinions, and that's my style. _Get used to it_
| " style of writing quota for quite some time.
| zxcvbn4038 wrote:
| I think you have captured it perfectly.
___________________________________________________________________
(page generated 2021-10-04 23:02 UTC)