[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #61 from Andrew Church --- For the record, I'll maintain a copy of my (unaccepted) patch to add -Wunused-result=strict at: https://achurch.org/patch-pile/#gcc (wur-strict.diff) This flag obviously shouldn't be relied on in released packages, but it may be helpful for individual users trying to work around this issue.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #53 from Andrew Church --- (In reply to Segher Boessenkool from comment #51) > And that is the core of why this issue reinflames once in a while: some > people > abuse the attribute, and the compiler cannot read minds. Ah, for a mindread() API... But ultimately, this is why I suggest that the compiler should leave as many decisions as possible to the end user (in this case, the API user as opposed to the API developer) - if the user has decided a particular result is safe to ignore, then anything the compiler tries to do to stop that is just an annoyance to be worked around or outright disabled. > Completely useless casts to void cluttered programs decades ago already, > we do not fear cargo cult, instead we observed it already existed. At the risk of starting a linguistic discussion, the phrase you're looking for is not "cargo cult", but "idiomatic". Using "(void)" to mean "I am explicitly discarding this value" is idiomatic, much the same way as "goodbye" is an idiomatic greeting and not a literal wish for God to be with the listener. I'm probably not as old as some developers here, but I'm certainly old enough to remember some compilers which gleefully spit out warnings on practically every non-void expression, and liberal use of (void) was needed if you wanted any chance at seeing the useful warnings. "Cargo cult" programming is when people start using (void) without even knowing what it does: "Hey, so what does this 'void' thing do?" "Uh, I dunno, it's all over the code so I just copied it." I wouldn't be surprised to find cases of that as well, but I don't think that's the specific thing we're worried about here. > Changing the behaviour of this attribute after all that time will not make > things better. But perhaps we can say a bit more in the documentation, > maybe at one of the three very concise quotes above? Say half a line worth? If glibc changes from _wur to [[nodiscard]], as Florian suggested may happen, that would resolve the immediate case that brought me here (a "best-effort" system() call, where the success or failure of the call or the program executed is irrelevant to the caller). I do fear leaving the current behavior as is could just be kicking the can down the road, so to speak, but perhaps a slight edit here might help: > 'warn_unused_result' > The 'warn_unused_result' attribute causes a warning to be emitted > if a caller of the function with this attribute does not use its > return value. This is useful for functions where not checking the > result is either a security problem or always a bug, such as > 'realloc'. I would suggest removing "either a security problem or", and adding something along the lines of: "For cases in which not checking the result carries risks but is not necessarily an error, use the [[nodiscard]] attribute, which allows the caller to suppress the warning by explicitly casting the result to void." Writing user documentation isn't (remotely!) my specialty, but I think something like this could both help clarify that the two behave differently and let people know that yes, the fact that you can't silence _wur calls is intentional.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #48 from Andrew Church --- (In reply to rusty from comment #47) > Civility please. I have no intention of trying to start a fight :) Like you, I'm just trying to improve the situation, and knowing that in my own open-source work I'm always happier when a user offers a patch instead of just a "please fix this", I've done the same here. That said, since I _am_ trying to improve the situation, I won't step back from debating the utility of the change if the developer disagrees. (And I freely admit that I can be tempted into a bit of snark from time to time...) > But accept that the > conversation on this issue is only a weak indication of consensus. I agree, which is why I gave other examples such as Clang's behavior and especially the C++/C2x standards. While I grant that even standards committees are small subsets of the total user community and are not immune to poor decision-making, I'd consider the facts that (1) the C++17 description of [[nodiscard]] called out void casts as a case which should not be diagnosed, (2) C++20 expanded on that with an explicit example of a cast-to-void call not being diagnosed, and (3) C2x, as of the current draft, also includes the call-out of void casts from C++17 (though not the explicit example from C++20), to collectively be a much stronger indicator of that consensus. > As Andrew Pinski says "people are mis-using this attribute", and Jakub > Jelinek makes a similar point. The use of _wur has changed from "ignoring > the result is criminally wrong" to "possibly wrong". I think "mis-using" is a bit harsh; the core concept (warning about a discarded return value) is a useful one, as evidenced by the feature's adoption into C++ and subsequently C2x. I grant that it's being used for a wider variety of purposes than originally intended, but since there was no other option until the relatively recent addition of [[nodiscard]], that's what people went with. I was thinking about adding a suggestion for multiple levels of warning, such as "ignoring this is almost certainly wrong" (e.g. realloc()) vs "ignoring this could be dangerous, you might want to doublecheck" (e.g. system()); in fact, [[nodiscard]] is effectively that since void casts do silence [[nodiscard]] warnings in GCC, though I don't know if the difference in behavior from _wur is intentional. But that ultimately wouldn't do anything about the present problem, which is API developers and users disagreeing on whether a return value is safe to discard - it might morph into something like "why is this function marked with the stricter _wur when it should just be [[nodiscard]]", and we're back to wanting to selectively silence _wur warnings again.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425
--- Comment #46 from Andrew Church ---
(In reply to Andrew Pinski from comment #45)
> But there is no general agreement at all. If clang behavior agreed with gcc,
> then there would be consensus here. In fact gcc behavior is older than clang
> behavior makes this even more difficult and even points out that if clang
> said it implemented a compatible extension, it did not.
First, a single disagreement does not invalidate a consensus, at least in the
general meaning which I quoted ("the judgment arrived at by most of those
concerned" - note "most" and not "all"). As I discussed earlier, I still see
significantly wider support for the view that void casts should silence
discarded-value warnings, to the extent that I consider it fair to call that
view a consensus.
Second, an equally valid view of the meaning of Clang's behavior differing from
GCC's - barring any deeper knowledge of the LLVM team's decision-making, which
I do not have - is that the LLVM team merged the observed intent behind the
original extension (warn about values which should not be discarded) with its
knowledge of existing practice (use of a void cast to indicate a deliberately
discarded value) to arrive at their choice of behavior. If anything, I'd
suggest that https://bugs.llvm.org/show_bug.cgi?id=51228 being marked WONTFIX
because the developers consider it to be "working as intended" argues for this
view.
> You are basically saying all warnings will be ignored which is true.
That's not what I said, and I disagree at least with that particular phrasing.
I would, however, accept "warnings will be ignored if the user feels the effort
to fix the warning is not worth the benefit". By corollary, users in general
will take the path of least resistance to resolving a warning, and if allowing
void casts to silence specific instances of the warning encourages them to
leave the warning enabled in general, it will result in better code than if the
user gets so frustrated with the warning that they just turn it off completely.
> Using a void cast makes life too easy to ignore this behavior.
It still requires action on the part of the programmer. A programmer who
writes
system("foo");
may want to be warned that the return code from system() is discarded, whereas
a programmer who writes
(void) system("foo");
has clearly stated an intent to discard that return value, and warning about it
anyway is unnecessary noise. (Though an argument could be certainly be made
for a better word than "void", perhaps a construction like "[[discard]]
foo()".)
Is there a risk of cargo-cult syndrome? (A: "Hey B, how do I call this
external foo program?" B: "Just write '(void) system("foo")'.") Certainly.
But the same argument can be made for _any_ workaround, and I suspect that more
complex workarounds would increase, rather than decrease, that risk - the more
complex the code, the less likely the user is to read it and understand what
it's doing.
> If the api user wants to ignore the return
> values while the api developer tells you should not, then really the api
> user will have bugs. This is why the warning is there.
Demonstrably false (see above, and even your own caveat below).
> Now api developers
> have known to place this attribute on things that should not be placed on
> but that is not a compiler issue and the api user should talk with the api
> developer rather than try to change the compiler for their workaround.
Suppose the API developer's response is, "I don't care about your use case, I
want you to do things my way." What is the (non-compiler developer) user to do
then? Look for an alternative library, which may not even exist? Spend the
time to write their own? Disable the warning and accept the risk of bugs in
unrelated code? Look up the workaround du jour and throw it all over their
code base, cargo-cult style?
If I were faced with that choice, I would probably just switch to Clang.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #44 from Andrew Church --- (In reply to Segher Boessenkool from comment #43) > That is not the consensus, no. "Consensus" does not mean doing what the > unthinking masses shout. Merriam-Webster disagrees: con.sen.sus 1 a: general agreement : UNANIMITY b: the judgment arrived at by most of those concerned I specifically clarified that with "wider" to indicate the larger group of "C compiler users", as opposed to what you seem to mean, "C compiler developers". And in whatever sense you choose to regard them, the judgment arrived at by most C compiler users does seem to be "cast to void should suppress a warning about a discarded value", as I described above. > So allowing casts to void > to suppress this warning means the warning becomes less useful, and people > will write worse code. That is not something GCC should encourage IMO. You seem to think that making the warning harder to work around will encourage programmers to change their code to fix the warning. In reality, they are more likely to either (1) disable the warning entirely or (2) disregard warnings in general, both of which result in considerably worse code - the situation you say you are trying to avoid. Thus my suggestion to follow the principle of least surprise and allow a void cast to disable the warning by default.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #42 from Andrew Church --- (In reply to Sam James from comment #41) > Could you send it to the gcc-patches mailing list please? (Even if it is a > PoC). Sent as requested.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 Andrew Church changed: What|Removed |Added CC||achurch+gcc at achurch dot org --- Comment #40 from Andrew Church --- Created attachment 54906 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54906=edit POC patch to add -Wunused-result=strict In hopes of moving this along (after having run into it for the third or fourth time myself), here's a proof-of-concept patch against GCC 12.2.0 which adds "-Wunused-result=strict" for the current behavior and changes "-Wunused-result" to ignore cases in which the result is discarded by casting to void. My rationale for changing the default behavior is that the wider community consensus, as evidenced by things like the C++ (and C2x) [[nodiscard]] specification, the behavior of Clang, and the balance of comments on this bug, seems to be that casting a discarded return value to void should suppress any warning about the discarded value; and under the principle of least surprise, GCC should follow that consensus by default even if it also provides alternative behaviors.
