5 ms·
Within a company (at least in my experience) compilers/toolchains are changed infrequently, usually less than once a year. In this case, keeping the software bu
by sgerenser 3y ago
Within a company (at least in my experience) compilers/toolchains are changed infrequently, usually less than once a year. In this case, keeping the software building with -Werror is a heck of a lot easier than allowing warnings to creep in, then trying to fix it later. It ends up that “later” never happens. Using -Werror from the very beginning forces you to address warnings: either by silencing obviously false positives, or fixing legitimate problems.
A previous company I worked at had large C++ projects that built with 5000+ warnings. Nobody looked at them, obviously. Buried within them I recall one about printf-style formatting string mismatching arguments, that said something like “mismatching arguments will cause a crash if this code is executed.” I only noticed it buried in the list of warnings after debugging a segfault caused by that line of code.
- ninkendo 3y ago> either by silencing obviously false positives (Off-topic) Oh how I wish this were possible in the language I use at $dayjob. Swift does not let you silence warnings on a line by line basis. It’s easily one of the top 5 worst things about the language.
- saagarjha 3y agoThis would allow you to make a dialect where you call a function that Apple in their infinite wisdom has deprecated. Consider the tax this would impose on the ecosystem!
- saagarjha 3y agoI feel like every person who advocates for -Werror shows up to a codebase with a billion warnings once and then assumes that nobody can ever voluntarily keep their code compiling without warnings if they don’t have the build stop immediately so they can fix it. I’ve worked on dozens of non-legacy codebases and in every single one we don’t have warnings last for long, because going from zero to a few warnings is annoying. I don’t want that spam in my logs and I definitely don’t want suspicious constructs in my codebase. For the ones that enable -Werror, by the way, I always disable it so I can work and clean things up before committing. It’s just basic hygiene and discipline, the same as formatting your code before sending a PR. If your team doesn’t have that then it has bigger problems.
- vlovich123 3y agoHow many people were involved in this codebase at a given time and how many years was the project ongoing?
- saagarjha 3y agoProfessionally the ones that did this had between 2-200 engineers working on them at any given time. Most code was a decade or so old, with some legacy components that were older or vendored (but in many cases were not touched much and may have certain warnings disabled). iOS-focused, so Swift, Objective-C, C++ in Xcode. In open source projects most of the ones I work on have a handful of regular contributors and a couple dozen one-off or other irregular contributions. In many cases we don't get all the warnings on every system that anyone uses but if someone goes "fix warnings for GCC 13 on Alpha" or whatever environment they're using we are more than happy to take their changes, and if it warns in CI or on any of our machines we'll either bring it up during PR review or submit a change soon afterwards to fix it. I'm too young to have projects more than a few years old but there's a decent number of other open source projects with a long history and many contributors that compile with no warnings on say their Linux CI and maybe a handful if you pass it through Apple Clang (since they don't test that much).
- derefr 3y agoIt sounds like you don't want your compiler to stop on errors, but rather, you want your CI to recognize errors, and to prevent the merging of feature branches that have any warnings.
- code_biologist 3y agoMaybe this is what you're saying, but to have CI prevent the merging of feature branches that introduce new warnings? That's my favorite setup for introducing lint or lint rules to a large codebase.
- derefr 3y agoYes, but no need for the added complexity of a delta of warnings against the base branch. The presumption here (given the GP comment) is that main has no warnings, since warnings are only allowed "during development", and should be cleaned up before production. So the CI only has to be intelligent enough to prevent merging (to main) of a branch with any warnings.
- jcparkyn 3y agoThat's exactly why the author says to check for warnings in CI. Most warnings will be noticed and fixed by the dev before pushing, and the rare ones that get missed will be caught in CI before merging.
- crustaceansoup 3y agoAre there companies which would allow or suggest employees or contractors just use whatever random toolchain they have installed? I can understand the argument for open-source projects that aren't attached to a company, but otherwise? Maybe this is my bias being in a subindustry centered on distributed binaries and so have set standard toolchains (i.e. what the build machines use), but I'm still not seeing it. This seems like the more harmful practice than using -Werror
- saagarjha 3y agoMost companies I have worked at have a specific compile script that everyone uses and they don't mind if you use something else as long as you aren't causing problems for everyone else. I typically run pre-release toolchains to spot errors early for example, or sometimes I'll swap out the compiler to test things. Obviously, what gets built and deployed to production is not built off my computer; it goes through the standard CI they have.
- maccard 3y agoMost places I've worked won't actively stop you from using a different toolchain if you don't cause any issues
- lolinder 3y agoI feel like you're replying to the title and not the article. The article starts with a huge disclaimer saying that the author has no tolerance for warnings getting merged in to their projects, but they have the warnings check in the CI pipeline. Doing that would totally prevent the kind of lax never-fixed errors you're talking about, while allowing more flexibility while the PR is still in progress.
- sgerenser 3y agoThat’s fair, I’m responding more to the general aversion to using -Werror at all than the specific case of the OP. If I were writing open source software expected to be used by a wide variety of compilers and platforms I’d probably also not want to make -Werror the default.
- TheGoddessInari 3y agoAs an anecdote, there may be one or two packages in openwrt that won't build by default because of a strange interaction between certain glibc headers and the behavior of _FORTIFY_SOURCE on some embedded platforms, but only on some versions of gcc. Werror tends to be placed eagerly pass the buck along to anyone building it, not just CI or developers. I used to feel that Werror was a good thing, but nowadays anything with CI on merge requests will tell me about any warnings, and someone wanting to build the software shouldn't get penalized for having a different build environment: there's a reason they aren't thrown as errors to begin with. Does this make sense?
- paulddraper 3y agoYeah the author doesn't agree with his own title :)
- mirekrusin 3y agoHe says -Werror is CI's friend, not dev's.
- 3y ago
- bluedino 3y agoInterview question (from the job seeker): can I see your application logs? I once worked at a place that could flood the error/warning logs in two seconds (Ruby). Unless it was causing an issue, all warnings etc were ignored. Is that the kind of codebase you want to work in?
- jahnu 3y agoDepends. Am I being hired with the power to change things?
- extraduder_ire 3y agoAny time I have to compile ffmpeg, the quantity of warnings flying up my screen fills me with a desire to "fix" a bunch of them and send a pull request. Never enough to actually get started on it though. Most of the c++ I've written used the arduino build system, so an absurd amount of things that'd normally be errors or warnings are just permitted and expected.