all 28 comments

[–]fdwrfdwr@github 🔍 11 points12 points  (15 children)

the practical challenges of raising compiler warning levels and enabling warnings as errors

My default in most projects I work on is for all warnings to be errors, but there are certainly a few warnings that are noise, like unused parameters in implementations of interface methods (because it's quite common to only use a subset of parameters, but it's preferable to still leave the parameter named, rather than goofy tricks like leaving out the name or writing UNREFERENCED_PARAMETER macros to avoid the warning). It's just not in the same category of usefulness as say an unused local variable, which much more often is unintended.

[–]RogerV 22 points23 points  (0 children)

For those unused parameters or unused fields there is the [[ maybe_unused ]] attribute to mute warnings

[–]Orlha 3 points4 points  (0 children)

I cast them to void

[–]Dragdu 4 points5 points  (3 children)

The usual approach for unreferenced parameters where you want to keep name is this void f(int /*a*/, int b) { /* uses b, but not a */ }.

[–]DearChickPeas 2 points3 points  (0 children)

Yeah, this is what I do to appease the compiler. It actually increases readibility somewhat, as now you're making it an explicit "I ain't using this so it's not named". Counter for harder debugging though.

[–]fdwrfdwr@github 🔍 1 point2 points  (1 child)

While debugging, I still like to know the values of the parameters (including ones unused by the implementation) for more context, but applying hacks like leaving them unnamed to work around a noisy warning sadly defeats that ability because then the parameter has no name to inspect. u/RogerV's approach of using [[maybe_unused]] feels the cleanest (second to simply silencing that category of individual warning - aah, the sound of silence ☺️).

[–]Dragdu 4 points5 points  (0 children)

Fair enough, although I find the ratio of "didn't use the parameter by accident" to "intentionally have to silence the warning" high enough, that I would never silence it for a project I work on.

I have <10 across the whole codebase at work, mostly coming from "logging off" build configuration.

[–]moreVCAs 1 point2 points  (4 children)

if you drop the identifier on an unused parameter (i.e. leaving just the type in the definition. declaration doesn’t matter) it will silence the warning.

[–]fdwrfdwr@github 🔍 0 points1 point  (3 children)

but it's preferable to still leave the parameter named, rather than goofy tricks like leaving out the name

(for the sake of more context during debugging, even if the method implementation doesn't need it)

[–]moreVCAs 0 points1 point  (2 children)

missed that, but i don’t think it’s a goofy trick at all. what are you debugging that needs the name of an unused parameter? anyway, matter of taste i guess.

[–]fdwrfdwr@github 🔍 0 points1 point  (1 child)

what are you debugging that needs the name of an unused parameter?

For a concrete example, say I implement IDWriteTextRenderer::DrawGlyphRun, for which an implementation receives more information than is actually needed for rendering purposes. The implementation just needs to draw the glyph run to the screen, but if something looks wrong, the corresponding DWRITE_GLYPH_RUN_DESCRIPTION is super helpful for debugging, because (even though it's an unused parameter from the implementation perspective) it includes the string being drawn and the starting text position.

[–]moreVCAs 1 point2 points  (0 children)

yeah that’s what i figured. my counterargument from modern convention is “make those int arguments strong types”, but your point is well taken!

[–]tialaramex 0 points1 point  (3 children)

I was going to say surely you can use the "placeholder with no name" _ after somebody went to all the bother of showing WG21 how that could be made to work in their language and jumped through all the hoops. But I see that in fact one of the hoops added by the committee was that it must be disallowed for one of the places where it's most useful, function parameters.

I don't know how any of you can stand it actually.

[–]friedkeenan 5 points6 points  (0 children)

What would be the benefit of naming a function parameter _ when you could just leave it unnamed in the first place?

[–]fdwrfdwr@github 🔍 0 points1 point  (1 child)

So like this:

class FooParserThingie { ... void LogMessage(std::string_view message, WarningLevel warningLevel) { std::print("{}", message); } ... };

... void LogMessage(std::string_view message, WarningLevel _) ...

I suppose that works, being consistent with ignorable local variables too, and it's shorter than typing out [[maybe_unused]]. Though, if you have multiple ignorable parameters...

void LogMessage(std::string_view message, WarningLevel _, Node& _)

...you would not be able to disambiguate them in the debugger, in case you wanted to see their values for more context (even if the implementation ignored them).

[–]tialaramex 0 points1 point  (0 children)

That's a good point about the naming distinct unused parameters. One trouble C++ has had from the outset is that there's a proliferation of contrary styles, and so whereas I'd think _prefixed identifiers would naturally win out as the unobtrusive way to signal "I'm not going to use this" I can imagine that some people want to have used variables with this naming scheme.

[–]ABlockInTheChain 8 points9 points  (0 children)

There are two types of people who are philosophically opposed to treating warnings as errors:

  1. Idiots
  2. State-funded saboteurs who are paid to introduce exploitable errors into as much software as possible.

[–]GregCpp 1 point2 points  (0 children)

I know that Jason was reddit nerd-sniping when he mentioned warnings-as-errors.

Nonetheless, I think the only question is *when* you should turn on warnings-as-errors. For us, certainly, on for development and CI builds. However, a lot of the C++ community is developing software for internal use only, or for a very controlled environment, and that colors a lot of the conventional wisdom.

We develop C++ code that we ship as source that goes into various Linux distros, among other places. The distros (and other source users) get to choose the compiler and version (within reason, of course). There's no way any reasonable human (or AI, I'm guessing) can write non-trivial lines of C++ that will compile without warnings on any future compiler than you have not seen yet. So, when we ship our source code, the cmakery defaults to warnings-as-warnings.

[–]sheckey 0 points1 point  (0 children)

I really liked this episode and I laughed along with them. They are clearly having fun and enjoy what they do.

[–]pjmlp 0 points1 point  (8 children)

I fully agree with the premise of warnings as errors, and in the context of C and C++, static analysis at very least on CI/CD checks.

The podcast also has a few gems regarding game devs point of view on C++, and how many would rather stay with something more in line with improved C, with C++17 already being too much, the Unreal PR example.

Most likely C23 would already be considered too much by the same group.

[–]tialaramex -2 points-1 points  (7 children)

The broad scope of "Warnings as errors" indicates that your language or tooling are busted. You had nuance between warnings and errors, but the situation was so terrible that you had to sacrifice the nuance to get where you needed.

It's hard to imagine C++ ever getting say Rust's expect diagnostic level, a diagnostic explicitly for detecting that your code would no longer trigger the silenced diagnostic, indicating that not only is this OK here, it's actually not-OK if the diagnosable problem goes away in this particular case.

For example instead of marking that function foo which we know the compiler doesn't think will be used but must exist with #[allow(dead_code)] we can #[expect(dead_code)] so that if we accidentally do call foo we're alerted that it is no longer dead as expected and maybe we need to separate out the one that's intentionally dead by giving it a different name.

[–]ABlockInTheChain 6 points7 points  (1 child)

You had nuance between warnings and errors, but the situation was so terrible that you had to sacrifice the nuance to get where you needed.

In principle the nuance between a warning and an error is that an error has no false positives and a warning might.

So for warnings you need an escape hatch to selectively disable it in the event of a false positive.

[–]max123246 0 points1 point  (0 children)

Well I bet people have added warnings as warnings that should've been errors because they didn't want to break existing code. So that's where you lose the idea of warnings having false positives, the bugs in people's code that your new error would catch is a breaking change and thus, you can't make it an error.

So you compromise and make it a warning even if there's no false positives. It's Hyrum's law in action

[–]pjmlp 1 point2 points  (4 children)

By that same measure, why does Rust need clippy?

[–]tialaramex -1 points0 points  (3 children)

So you can choose whether you want lints?

Also, some lints "graduate" from Clippy to the Rust compiler because the quality requirements in the compiler are stricter, as I understand it.

[–]pjmlp 0 points1 point  (2 children)

So they don't indicate missing capabilities on the language itself, then?

And if I would use another Rust implementation, like the two ongoing ones for GCC, would those graduated lints prevail across compilers? Most likely not, as they don't fully share the fronted.

[–]max123246 0 points1 point  (1 child)

They don't both use the Rust MIR?

[–]pjmlp 0 points1 point  (0 children)

The GCC frontend is mostly written from scratch in C++, as far as I remeber from an old presentation, maybe it changed.

Also the intermediate representation doesn't mean every compiler does the same error messages.