[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 Christian Groessler changed: What|Removed |Added CC||groessler_christian at yahoo dot d ||e --- Comment #62 from Christian Groessler --- (In reply to Segher Boessenkool from comment #60) > So you want to not warn for some (just *some*) explicitly unused cases, and > do > warn for other explicitly unused cases, and all implicitly unused cases? > While > the author of the code explicitly asked for a warning message to be emitted > in > all such cases: "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." Yes! I'm write()ing to a pipe a small amount of data (< page size), and anyway don't know how to continue if the write() fails. It would be noticed at the other end. (void)write() doesn't suppress the warning. Annoying...
[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 #60 from Segher Boessenkool --- (In reply to Roman Krotov from comment #59) > All, what I'm asking for, is to make something like -Wno-void-unused, which > would suppress the warnings only for the (void) casted calls. So you want to not warn for some (just *some*) explicitly unused cases, and do warn for other explicitly unused cases, and all implicitly unused cases? While the author of the code explicitly asked for a warning message to be emitted in all such cases: "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 desperately needed by the projects like systemd (see the first link > in my first comment) as a less severe variant than -Wno-unused-result, so > that they won't get punished with less diagnostics. They (like EVERYONE ELSE IN THE WORLD) should not use -Werror, if they do not like punishment. Warnings are warnings. The author of your code (the header files for the library code) wanted everyone to be warned about not using the return value from a certain function. He/she was almost certainly right about that. And it is easy to suppress the warning in the few cases where you really want to. > I don't see any reason not to implement -Wno-void-unused with the similar > description (stating that it's not recommended, if you want) to help the > projects like systemd. Define what it would do *exactly*, make a patch for it (including for the documentation, amending all existing documentation as well), and do that in such a way that it a) is correct, and b) makes any sense. Then send the patch to gcc-patches@. If you do not want to do all that work (including the very much non-trivial amount of follow-up work that will cause), then please go away? Don't tell us to do insane things that are an incredible amount of work just because you had a bad idea and now want it to become reality. > It won't change the meaning of the wur attribute, bacause it will be a > non-default switch. This makes no sense at all.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #59 from Roman Krotov --- (In reply to Ed Catmur from comment #58) > (In reply to Roman Krotov from comment #57) I already addressed all of it in my previous 2 comments... I'll write more clearly then. > > But I don't see any reasons not to implement the switch right now... > Making [[gnu::warn_unused_result]] mean the same as [[nodiscard]] would be a > reduction in expressivity. All, what I'm asking for, is to make something like -Wno-void-unused, which would suppress the warnings only for the (void) casted calls. This is desperately needed by the projects like systemd (see the first link in my first comment) as a less severe variant than -Wno-unused-result, so that they won't get punished with less diagnostics. In fact, gcc already has a "not recommended non-default" switch model: -Wno-coverage-mismatch <...> -Wno-coverage-mismatch can be used to disable the warning <...> disabling the warning is not recommended. I don't see any reason not to implement -Wno-void-unused with the similar description (stating that it's not recommended, if you want) to help the projects like systemd. It won't change the meaning of the wur attribute, bacause it will be a non-default switch. > > what about the projects > > what don't agree with the distinction of these 2 attributes? Or just want to > > be more compatible with clang? > They can use [[nodiscard]]. > > > Or they just may not agree with the choice of a library, that decided to put > > wur instead of nodiscard for a specific function. > Then they can write their own wrappers using [[nodiscard]]. What about libraries and programs that will prefer to stay on C99/C11 (without C2x extensions), especially if they use -pedantic? Even if there will be something like __attribute__(nodiscard), What about programs and libraries that will not prefer to update? That's why we shouldn't rely on the future and should implement the solution now.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #58 from Ed Catmur --- (In reply to Roman Krotov from comment #57) > But I don't see any reasons not to implement the switch right now... Making [[gnu::warn_unused_result]] mean the same as [[nodiscard]] would be a reduction in expressivity. > what about the projects > what don't agree with the distinction of these 2 attributes? Or just want to > be more compatible with clang? They can use [[nodiscard]]. > Or they just may not agree with the choice of a library, that decided to put > wur instead of nodiscard for a specific function. Then they can write their own wrappers using [[nodiscard]].
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #57 from Roman Krotov --- (In reply to Ed Catmur from comment #55) > So this problem will resolve itself, over time. But I don't see any reasons not to implement the switch right now... Late is better than never. > we can expect that attribute to be adopted where people intend that behavior > as opposed to the > warning not suppressible by cast to void I see your point and I partially agree with it, but what about the projects what don't agree with the distinction of these 2 attributes? Or just want to be more compatible with clang? Or they just may not agree with the choice of a library, that decided to put wur instead of nodiscard for a specific function. Something like -Wno-void-unused would still be better for them than -Wno-unused-result, for the reasons in my previous comment. (In reply to Martin Uecker from comment #56) > works as expected and is supported for older language modes Even if one day projects, that will prefer to stay on C99 or C11 and use -Wpedantic (which warns about [[]] right now), will be able to use [[syntax]] without any warning, they may still not like it. Even if gcc will include nodiscard into the __attribute__ syntax, there, for sure, will be the libraries that won't prefer to update or use the new attribute. For these situations, or even if they **will** use the new attribute, see my previous paragraph.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 Martin Uecker changed: What|Removed |Added CC||muecker at gwdg dot de --- Comment #56 from Martin Uecker --- [[nodiscard]] works as expected and is supported for older language modes since GCC 11 (and the syntax is ignored with a warning in GCC 10). Clang supports it with -std=c2x since version 9 and it seems Clang will also support it in older language modes in future releases.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #55 from Ed Catmur --- (In reply to Roman Krotov from comment #54) [[nodiscard]] is in C23, so we can expect that attribute to be adopted where people intend that behavior (warning suppressible by cast to void) as opposed to the nonportable [[gnu::warn_unused_result]] (warning not suppressible by cast to void). So this problem will resolve itself, over time.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 Roman Krotov changed: What|Removed |Added CC||romato.san1337 at gmail dot com --- Comment #54 from Roman Krotov --- I'm also very frustrated about this behavior, but I'm not asking to **change** it. I see your point and, although I think it's severely outdated now, I respect your decision. Instead I'm offering a compromise, or to better describe it, a win-win solution. What about introducing something like `-Wno-void-unused`, which would only disable the warnings for the (void) casted calls? And I'm not asking to do it for me, or some other random dudes; do it for systemd, they are also very frustrated about this: https://github.com/systemd/systemd/blob/5a96b32dead5132ba37a8b968c59101c2aac8d23/meson.build#L400 That's because they use (void) casts very frequently: https://github.com/search?q=repo%3Asystemd%2Fsystemd+language%3AC+%22%28void%29%22=code You can probably say they are in the wrong by doing the bad practice and should do a macro instead. And you may be right. I really appreciate your commitment to teach people good practices, but **suggesting** them (by setting the default behavior which **recommends** the programmers to use these practices) should be enough. I don't think it's a good idea to **force** people to use them, by either spamming them with warnings (useless to them, because they already made a decision) or making them disable such warnings altogether, which could lead them to miss a bug where they forgot to use a value of a function (which they didn't cast with void). You may say it's their fault, but let me ask you a question, who is winning from the current situation? Certainly, not you, because you didn't convince them to change, instead you just made them not listen to you, see also julia-language and mpv: https://github.com/search?q=%22-Wno-unused-result%22=code And certainly, not them, because of the bug-risk I typed above, which is really unfortunate for such a big and widely-used project as systemd. Also, making them switch to clang (or drop gcc support) would be really bad, because nothing hurts FOSS and frustrates me in a project more than a permissive license. Eww.. Sorry, if I was a little bit harsh there, I really appreciate your contribution to FOSS and Linux development, and I would never switch to clang. Just, please note, that, again, I'm begging you to just implement a non-default switch that would make everyone's lives easier. Why "everyone's"? You see, If a new programmer encounters this "problem", they would think it's just a "gcc bug", because "clang works as expected" (and also the mentioned projects wrote like this in a comment), so they just disable the warnings altogether. Nobody won, especially the programmer. Now consider the situation where you implemented the switch, but wrote, for example, in the manpage just that "It's not recommended". Now they will be curious why, so they will try to find an answer, and the search will probably lead them to the gnu.org page which has the answer. See, how even though you might think you gave the ability to write a bad code, in reality you would give the exact point why not to use (void)? (Which they really need because other compilers/linters are trying to convince them otherwise.) Come on, at least please admit that the situation described above would still be better than today's. If you will find at least some of my points somewhat reasonable, please share my comment with the people that can help bring this improvement to life.
[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 #52 from Jan Engelhardt --- >This is useful for functions where not checking the >result is either a security problem or always a bug, such as >'realloc'. always? reall..y..oc? void *x = malloc(1); realloc(x, 0);
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425
--- Comment #51 from Segher Boessenkool ---
(In reply to rusty from comment #47)
> Civility please.
Thank you.
> 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".
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.
The documentation of this attribute states
'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'.
The "non-bugs" section of the manual ("Certain Changes We Don't Want to Make"
says
* Warning when a non-void function value is ignored.
C contains many standard functions that return a value that most
programs choose to ignore. One obvious example is 'printf'.
Warning about this practice only leads the defensive programmer to
clutter programs with dozens of casts to 'void'. Such casts are
required so frequently that they become visual noise. Writing
those casts becomes so automatic that they no longer convey useful
information about the intentions of the programmer. For functions
where the return value should never be ignored, use the
'warn_unused_result' function attribute (*note Function
Attributes::).
Completely useless casts to void cluttered programs decades ago already,
we do not fear cargo cult, instead we observed it already existed.
And finally there is
'-Wno-unused-result'
Do not warn if a caller of a function marked with attribute
'warn_unused_result' (*note Function Attributes::) does not use its
return value. The default is '-Wunused-result'.
A caller that casts a return value to void *explicitly* does not use that
return value.
> I still put a comment complaining about this every time I hit it, which is
> about once or twice a year. But I have little more to say; it's been almost
> 20 year after all :)
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?
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #50 from Florian Weimer --- (In reply to Jakub Jelinek from comment #49) > All that means is for APIs for which cast to void as silencing is meant to > be ok should be using [[nodiscard]] rather than > __attribute__((warn_unused_result)). APIs which do not want that should > keep using warn_unused_result. When that attribute was being added, the > glibc folks that requested it specially asked for casts to void not being a > way to silence the attribute because the attributes were added to functions > where users really should use the return value rather than silently ignore > it. I don't think these folks work on glibc anymore. 8-) For glibc, I think we will use nodiscard by default (outside non-fortify mode) in most cases where we use __wur now (and some), and restrict __attribute__ ((warn_unused_result)) to old compilers in fortify mode. Other libraries should probably do the same. The __attribute__ ((warn_unused_result)) remaisn too problematic because it encourages non-portable code to suppress it, such as __attribute__ ((unused)).
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #49 from Jakub Jelinek --- All that means is for APIs for which cast to void as silencing is meant to be ok should be using [[nodiscard]] rather than __attribute__((warn_unused_result)). APIs which do not want that should keep using warn_unused_result. When that attribute was being added, the glibc folks that requested it specially asked for casts to void not being a way to silence the attribute because the attributes were added to functions where users really should use the return value rather than silently ignore it.
[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 #47 from rusty at rustcorp dot com.au --- Civility please. We're all trying to find a path to improve things here. But accept that the conversation on this issue is only a weak indication of 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 still put a comment complaining about this every time I hit it, which is about once or twice a year. But I have little more to say; it's been almost 20 year after all :)
[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 #45 from Andrew Pinski --- (In reply to Andrew Church from comment #44) > (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 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. > > 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. > There is still no general consensus really. Just that the folks who want behavior difference is loud while the ones who are happy with the current behavior are quiet because they don't need to tell their thoughts on it. > > 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. You are basically saying all warnings will be ignored which is true. Even ones with no false positives are ignored. Using a void cast makes life too easy to ignore this behavior. 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. 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.
[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 #43 from Segher Boessenkool --- (In reply to Andrew Church from comment #40) > 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. That is not the consensus, no. "Consensus" does not mean doing what the unthinking masses shout. There already are easy ways to deal suppress the error, very direct, and very descriptive ways. A cast to void is round-about, cryptic, and already is cargo-cult, before this attribute existed even! 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.
[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 --- Comment #41 from Sam James --- (In reply to Andrew Church from comment #40) > Created attachment 54906 [details] > POC patch to add -Wunused-result=strict > Could you send it to the gcc-patches mailing list please? (Even if it is a PoC).
[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.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 Nick Desaulniers changed: What|Removed |Added CC||ndesaulniers at google dot com --- Comment #39 from Nick Desaulniers --- Cross referencing an LLVM bug on a similar discussion: https://bugs.llvm.org/show_bug.cgi?id=51228
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425
pskocik at gmail dot com changed:
What|Removed |Added
CC||pskocik at gmail dot com
--- Comment #38 from pskocik at gmail dot com ---
I like this behavior. I use (void) casts to suppress warnings about unused
parameters and variables, but I'd rather suppressing WUR weren't that simple
because of functions whose return result represents an allocated resource
(allocated memory, FILE, filedescriptor, etc.), in which case the suppression
is in 99% cases erroneous.
Of course, WUR is also useful as an aid in enforcing consistent error checking
but a codebase using WUR like that might as well define an custom IGNORE macro
(which assigns the result to a properly typed temporary and then voids it) and
make sure such a macro only works on return values which are truly safe to
ignore (e.g., rather than returning plain int, long, etc., you might return
struct ignorable_int { int ignorable_retval; };, struct ignorable_long { long
ignorable_retval; }, etc. and have your ignore macro try and access the
specifically named member).
(An ability to directly attach WUR to such types, which clang has gcc currently
doesn't (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94379), would also go
nicely with this un-void-able WURs feature (although WURs are void-able on
clang)).
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 Andrew Pinski changed: What|Removed |Added CC||porton at narod dot ru --- Comment #37 from Andrew Pinski --- *** Bug 89031 has been marked as a duplicate of this bug. ***
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #36 from Jason Merrill --- (In reply to Jason Merrill from comment #35) > Is there a reason you can't use [[nodiscard]]? ...ah, because this is a bug report against the C compiler.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #35 from Jason Merrill --- Is there a reason you can't use [[nodiscard]]?
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425
Ed Catmur changed:
What|Removed |Added
CC||ed at catmur dot uk
--- Comment #34 from Ed Catmur ---
Note that a logical negation prior to cast to void is sufficient to suppress
the warning:
int void_cast_should_not_warn() {
(void) !foo();
// ^-- here
return 0;
}
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425
--- Comment #33 from costinc at gmail dot com ---
There are legitimate reasons to ignore results, even without additional
comments.
One use case I ran into is:
// ok() checks the same condition as the one returned by f().
while (ok()) {
switch (...) {
case 1:
(void)f(1);
case 2:
(void)f(2);
...
}
}
I don't think it's necessary to comment on every single call to f().
At some point people might start using if (f()){} on all function calls where
they don't use the result, because that works and casting to void doesn't
anymore because of this issue. The way to prevent that might be to start
warning on that too.
What if a static analysis tool decides to warn on if (f()){}? How do you please
both gcc and the tool? if ((void)f()){}? Thankfully this is a highly unlikely
scenario as it doesn't seem like other compilers/tools have their own unique
ideas here.
It seems the reason to warn even when using (void) is to implement a ticket
system. First, use (void). This will grant you a ticket to ignore results. At
some point we'll get angry and decide not to let you do this anymore because
you've abused it. Then, use if(f()){}. This will grant you a different ticket
so you can have a way to ignore results for a while longer. Then use the next
thing.
At least now there is [[nodiscard]] and casting to void is quite clearly
defined in the standard. But thanks to the present issue it's not quite the
portable way to ignore results (some libraries will use nodiscard, others will
use WUR).
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #32 from Segher Boessenkool --- Yes, it does hurt. Quite many people use casts to void automatically on all function calls where they do not use the result. They of course need to be re-educated on that. Casts to void do not portably suppress warnings, either. Finally, you can suppress the warning in much better ways (namely, by actually using the result! What a novel concept).
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #31 from Hans Ecke --- I would like to point out that what everybody here proposes - make (void) work properly with WUR - hurts no one. The other viewpoint has only given vague theoretical reasons. Let me give you something concrete: Sometimes my junior devs ask about wur-like functionality in C. My answer is "would be nice to have, right?". That is because this attribute as gcc implements it right now is useless. I'm not going to clutter my codebase with a bunch of custom macros. Please do one of the following: * my preference: make the attribute work properly, in line with C++ and clang * add a new attribute that has the behavior that 99.9% of the people actually want We've already lost 2 years. With the speed that new gcc releases make it to production systems we could use proper wur in gcc in 2023.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 Yongwei Wu changed: What|Removed |Added CC||wuyongwei at gmail dot com --- Comment #30 from Yongwei Wu --- I encountered this problem too. I'd say I would prefer (void)some_system_function_that_warns(...); to int ignored __attribute__((unused)); ignored = some_system_function_that_warns(...); The latter is not only verbose, but also creating portability issues. Should I also define macros for compilers that do not support __attribute__? It is extremely messy!
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425
Andrzej Krzemienski changed:
What|Removed |Added
CC||akrzemi1 at gmail dot com
--- Comment #29 from Andrzej Krzemienski ---
Note that the discussed problem only applies to
`__attribute__((warn_unused_result))` and `[[gnu::warn_unused_result]]`. Since
version 7, GCC also implements `[[nodiscard]]` which works as recommended by
the C++ Standard:
```
[[nodiscard]] int f () { return 1; }
int main()
{
f(); // warns
(void)f(); // doesn't warn
}
```
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 Adam Borowski changed: What|Removed |Added CC||kilobyte at angband dot pl --- Comment #28 from Adam Borowski --- Please tell me: what would be other likely uses for such an explicit (void) cast _other_ than trying to silence this warning? I can't think of any. Thus, I'd recommend to keep it simple and instead of adding complex really_strict_unavoidable_warn_unused_result options, just make (void) shut it down. It did the thing for many years before. There's too many system headers which abuse this attribute. On one hand you have realloc(), on the other write(STDERR, "error message").
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 Manuel López-Ibáñez changed: What|Removed |Added CC||manu at gcc dot gnu.org --- Comment #27 from Manuel López-Ibáñez --- (In reply to Steven Bosscher from comment #26) > Maybe make something like "-Wno-unused-result=[pedantic|nodiscard]", make > strict the current semantics of the flag and nodiscard the C++17 semantics > (and make that the default)? I see two alternatives: 1) Keep nosdiscard and __wur semantics separated and use different flags for them such as a new -Wdiscard which is enabled by default so that users can use -Wno-discard to disable "nodiscard" attrib warnings. (Or should it be -Wnodiscard and -Wno-nodiscard?). This has the downside that users of existing libraries using __wur do not see any benefit and this PR goes on. 2) Make __wur more strict than nodiscard only if some new flag -Wstrict-unused-result is enabled (stricter warnings would print this flag and users can use -Wno- version to disable it completely without disabling nodiscard warnings). This has the downside that users and library devs happy with the existing behavior will have their default silently changed. The upside is that users can decide on their own if they want the looser semantics by flipping a single switch. Option (2) is equivalent to what is proposed above. In that case -Wstrict-unused-result will imply -Wunused-result (but not the other way around) and the only question is whether -Wstrict-unused-result or -Wunused-result is enabled by default. "pedantic" has a very precise meaning within GCC that is already quite difficult to explain, let's not make it even more complex by overloading it.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 Steven Bosscher changed: What|Removed |Added CC||steven at gcc dot gnu.org --- Comment #26 from Steven Bosscher --- Maybe make something like "-Wno-unused-result=[pedantic|nodiscard]", make strict the current semantics of the flag and nodiscard the C++17 semantics (and make that the default)?
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #25 from Jakub Jelinek --- (In reply to Jason Merrill from comment #24) > I agree that (void) should suppress the warning. Jakub, do you remember why > you made a different choice? I think the request came from Uli that it isn't that easy to suppress it, at least initially the warning was meant for cases where it would be a major security problem to ignore the return value.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 Jason Merrill changed: What|Removed |Added CC||jakub at gcc dot gnu.org, ||jason at gcc dot gnu.org --- Comment #24 from Jason Merrill --- I agree that (void) should suppress the warning. Jakub, do you remember why you made a different choice?
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 costinc at gmail dot com changed: What|Removed |Added CC||costinc at gmail dot com --- Comment #23 from costinc at gmail dot com --- I've just been bitten by this. The problem isn't the attribute, it's the on-by-default warning. Why force people to move from one convention of ignoring expression results to another? I've just wasted one hour of my life. On the opposite side of the scale there are people using prettier ways to ignore expression results. I really hope someone, somewhere, was deterred from ignoring a result using a much uglier method than (void) and, because of it, fixed a real bug (this must be the only scenario that favors disallowing (void), deterrence through repugnance, am I wrong?). Note that casting to void seems to be mentioned in the standard precisely for discarding expression results: "Any expression can be explicitly converted to type “cv void.” The expression value is discarded." At best, this warning should be restricted to gnu99 or similar. And if the attribute and it's associated warning are supposed to apply only to important security related code then where is the warning for non-security related code? The warning name pretty clearly relates to the general problem. Isn't that confusing? My 2c: Clang did the right thing. Please, at least make the warning off-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 #22 from Filipe Brandenburger filbranden at google dot com --- (In reply to Jan Engelhardt from comment #20) Seems like the short route is to add a new attribute ((warn_unused_result_with_void_cancelling)) that exhibits the desired behavior of (void) cancelling the warning, and then make glibc use that. Simple, no? I'd rather see ((warn_unused_result_without_void_cancelling)). Or, better yet, add both in and add a command-line flag to allow ((warn_unused_result)) to use one or the other. I still don't see the point of preventing (void) from cancelling it since you can just store the result on an otherwise unused discard variable, I mean, if the programmer *really* wants to ignore the result, they can do it in many ways, ((warn_unused_result)) is to prevent programming mistakes where the programmer *meant* to look at the results but mistakenly forgot it... I also fail to see what else a void cast can mean other than explicitly indicating that the programmer *knows* the function returns a non-empty value but decided to ignore it... Anyways, let's not go on that rant again... If you get to implement the new attribute that would be awesome! Cheers, Filipe
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #21 from rusty at rustcorp dot com.au --- jengelh at inai dot de gcc-bugzi...@gcc.gnu.org writes: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #20 from Jan Engelhardt jengelh at inai dot de --- Seems like the short route is to add a new attribute ((warn_unused_result_with_void_cancelling)) that exhibits the desired behavior of (void) cancelling the warning, and then make glibc use that. Simple, no? Indeed! I'll produce the patch if anyone thinks it's worthwhile? It'll only take another 10 years to switch everyone :) Thanks, Rusty.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #20 from Jan Engelhardt jengelh at inai dot de --- Seems like the short route is to add a new attribute ((warn_unused_result_with_void_cancelling)) that exhibits the desired behavior of (void) cancelling the warning, and then make glibc use that. Simple, no?
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425
Segher Boessenkool segher at gcc dot gnu.org changed:
What|Removed |Added
CC||segher at gcc dot gnu.org
--- Comment #18 from Segher Boessenkool segher at gcc dot gnu.org ---
Another way of ignoring the unused result, friendlier on the eyes than
ugly macros or horrible casts, and even *inviting* the programmer to
write an explanatory comment, goes like
if (foo()) {
/* The return value of foo can be ignored here because X and Y. */
}
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #19 from rusty at rustcorp dot com.au --- I like WUR as a sanity-check, and it is useful that more and more library authors are using it (generally quite well). As Andrew points out, this has taken 10 years! The downside is that false positives are becoming more common. For example, gnulib also includes a routine to handle this, since 2008: http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/ignore-value.h;h=68521edb38f50cb2230c8560a125864be212db6a;hb=HEAD I believe that changing conditions call for a re-evaluation. And yes, I'd like to see a comment on any cast-to-void, not just for WUR. Thanks for your patience!
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #11 from Manuel López-Ibáñez manu at gcc dot gnu.org --- Some remarks before the discussion gets out of hand. Neither Andrew nor me nor other people that may comment here have the power to approve or reject this change. The people you need to convince are the C/C++ FE maintainers listed in the MAINTAINERS file. Some of them are also GNU libc maintainers, thus actual users of the _wur attribute. We are just trying to make you understand the context in which the decision *made by others* has been made. Casting to void is nowadays the de facto standard to say I really know what I'm doing and I don't care about the return value *here*. This is way better then letting each project invent their all workaround macros and even risk doing the wrong thing like the assert you pointed out as wrong. This cannot be true, because the most heavy users of _wur are the kernel and glibc, and the de-facto standard to compile those is GCC. How can (void) foo(x) be clearer than ignore_unused_result(foo(x)) ? Perhaps you could ask GNU libc to provide such non-standard macro ignore_unused_result to avoid every project inventing their own. Instead of '(void)', a patch that provides a __no_warn__ keyword as a short-hand might be more acceptable (again, you don't have to convince us, just the FE maintainers). So maybe after 10 years it's time to revisit that decision? It's not that the behavior of when warnings are emitted are really set in stone and never changed. It pretty much did all this time and we kept adapting to it. That is not so true, believe me. As someone that has been contributing to GCC diagnostics for the last 10 years, changing GCC diagnostics ALWAYS breaks someone's setup and they will be unhappy about it (https://xkcd.com/1172/). That said, the best strategy to change the defaults is not to argue here in this PR. The best strategy would be to collect the opinion from the most heavy users of _wur (that is, GNU libc and the kernel maintainers, not their users nor random people in their mailing lists), present a patch and argue convincingly to the FE maintainers that this should change (not because Clang is using it, but because the people using GCC want it). The gcc vs clang vs linters debate and who invented the attribute don't belong to this discussion. It's better to discuss why we can't let the attribute be actually useful by letting the users to turn it off when they know what they are doing. That way of arguing is self-defeating, I'm sorry to say. The attribute is actually useful because people are using it, otherwise, you would not even have noticed that it exists.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #13 from Andreas Schwab sch...@linux-m68k.org --- I don't really see why the trouble making the mental connection from the void cast with the coder's intent do discard that result. Could it mean anything else really? The C language doesn't define any such meaning. The cast is redundant as far as the standard is concerned.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #15 from Lucas De Marchi lucas.de.marchi at gmail dot com --- (In reply to Manuel López-Ibáñez from comment #14) (In reply to Filipe Brandenburger from comment #12) Can I have this issue reopened please? If that makes you happy... Ok, how can we assign this bug to the appropriate team then? It doesn't work like that. Individual contributors will work on things that they find interesting or get paid to fix. Rusty, who opened this issue, seemed inclined to prepare such a patch. I can't really think of a situation in the Linux kernel source code where I'd find a __must_check function with a void cast, exactly for that reason, right now it would generate a warning. The users of _wur added _wur to a function expecting that any use of this function that does not assign the result will get a warning, even if there is a cast to void. Not warning for a cast to void breaks that expectation. As I said, the people who add the attribute may not be the same people who use the interface with that attribute. I myself when writing libraries use the _wur to give my users a warning if they are ignoring the return value. However they have all the rights to ignore it. Be it simply by not using -Wunused-result, hacking a macro to ignore the result or use the de facto standard to cast to void. IMO the latter is the best one since there might be 1 particular case in the entire codebase in which ignoring the return makes sense.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 Manuel López-Ibáñez manu at gcc dot gnu.org changed: What|Removed |Added Status|RESOLVED|UNCONFIRMED CC|manu at gcc dot gnu.org| Resolution|INVALID |--- --- Comment #14 from Manuel López-Ibáñez manu at gcc dot gnu.org --- (In reply to Filipe Brandenburger from comment #12) Can I have this issue reopened please? If that makes you happy... Ok, how can we assign this bug to the appropriate team then? It doesn't work like that. Individual contributors will work on things that they find interesting or get paid to fix. I can't really think of a situation in the Linux kernel source code where I'd find a __must_check function with a void cast, exactly for that reason, right now it would generate a warning. The users of _wur added _wur to a function expecting that any use of this function that does not assign the result will get a warning, even if there is a cast to void. Not warning for a cast to void breaks that expectation. In any case, I simply tried to bring some clarity of why some people are contesting this and how you may get what you desire (and how you may not). I personally do not mind whether 'void' silences _wur or not. Good luck!
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #12 from Filipe Brandenburger filbranden at google dot com --- (In reply to Manuel López-Ibáñez from comment #11) Neither Andrew nor me nor other people that may comment here have the power to approve or reject this change. Great, so please don't preemptively dismiss it as invalid. Can I have this issue reopened please? The people you need to convince are the C/C++ FE maintainers listed in the MAINTAINERS file. Some of them are also GNU libc maintainers, thus actual users of the _wur attribute. We are just trying to make you understand the context in which the decision *made by others* has been made. Ok, how can we assign this bug to the appropriate team then? This cannot be true, because the most heavy users of _wur are the kernel and glibc, and the de-facto standard to compile those is GCC. How can (void) foo(x) be clearer than ignore_unused_result(foo(x)) ? Maybe it's not *clearer* but the void cast is already a valid construct which is part of the language and will work regardless of which compiler and libc you use. I don't really see why the trouble making the mental connection from the void cast with the coder's intent do discard that result. Could it mean anything else really? Perhaps you could ask GNU libc to provide such non-standard macro ignore_unused_result to avoid every project inventing their own. The world doesn't really revolve around glibc you know? There are other libc's around as well... Instead of '(void)', a patch that provides a __no_warn__ keyword as a short-hand might be more acceptable (again, you don't have to convince us, just the FE maintainers). Yeah maybe if we could go back in time 10 years that would have been sweet... But it seems the world has moved on and decided that the void cast is good enough, as code that uses it will work with any C compiler and any libc and the coder's intent is clear enough. So maybe after 10 years it's time to revisit that decision? It's not that the behavior of when warnings are emitted are really set in stone and never changed. It pretty much did all this time and we kept adapting to it. That is not so true, believe me. As someone that has been contributing to GCC diagnostics for the last 10 years, changing GCC diagnostics ALWAYS breaks someone's setup and they will be unhappy about it (https://xkcd.com/1172/). That said, the best strategy to change the defaults is not to argue here in this PR. The best strategy would be to collect the opinion from the most heavy users of _wur (that is, GNU libc and the kernel maintainers, not their users nor random people in their mailing lists), present a patch and argue convincingly to the FE maintainers that this should change (not because Clang is using it, but because the people using GCC want it). Please explain to me how changing this behavior could possibly break someone's code. If someone is already using -Wunused-result and using a void cast to try to silent the warning, they're failing to silent it on gcc, so if they're keeping the void cast around they're basically living with the warning and ignoring it... I can't really think of a situation in the Linux kernel source code where I'd find a __must_check function with a void cast, exactly for that reason, right now it would generate a warning. So I don't understand what is the big deal about this one... Cheers, Filipe
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #7 from Andrew Pinski pinskia at gcc dot gnu.org --- (In reply to Filipe Brandenburger from comment #6) Then please explain to me how this: (void) foo(); is any worse than this: int ignored __attribute__((unused)); ignored = foo(); /* do nothing with ignored here */ You can force me to assign the return value to something, but you can't really force me to do something useful with it... I am not forcing you to do anything. You (or others) are forcing yourself by using this attribute. GCC is not forcing you to use this attribute at all. You just don't the deinfition of the attribute which was decided on over 10 years ago. Yes 10 years. clang was not around back then. So one could say clang's attribute is not a copy of GCC's but rather a different one all together. In the cases above, I think the former is a much more clear way to express my intent to ignore that value, I'm telling the compiler yeah, I know, I'm supposed to use this value, but in this particular case I really know what I'm doing so please bear with me here. The -Wunused-result warnings would be a lot more useful (or, actually, just useful) if I was able to whitelist the places where I want to ignore the values, since right now my only practical choice is to disable it completely with -Wno-unused-result. This is an example of a *real* problem that would have been caught by this warning: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25509#c28 Except that assert definition is broken, the standard assert macro in C/C++ does not have any side effects for NDEBUG. And actually that does not show a real problem with the attribute but rather they have asserts which have side effects so they worked out that problem by defining their own assert :). Still not a GCC bug :). And it's not just LLVM... Other linters and static analysis tools seem to agree that a (void) cast is an explicit indication that the coder meant to ignore those results, so why would gcc want to be the one exception here? Again this has nothing to do with other lints, this attribute was designed so you can't ignore the return value. And most lints/static analysis tools will also look to see if you not really ignoring the return value in that you use it in an error case fashion. casting to void does not always cause (and really should not cause) static analysis to ignore what you did was broken for the API. It is not GCC's fault people are starting to misuse this attribute; you should write into those projects instead saying they are misunderstanding the attribute is being used incorrectly. I bet some of them, really don't want a warning even with a cast to void. Thanks! Filipe
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #8 from Filipe Brandenburger filbranden at google dot com --- (In reply to Andrew Pinski from comment #7) Again this has nothing to do with other lints, this attribute was designed so you can't ignore the return value. I obviously *can* ignore the return value, just assign it to an unused variable that I ignore... By pretending that warn_unused_result can only be used in cases where the return value can never be ignored with no exceptions (which, as mentioned above, is not really impossible to enforce) you just end up making -Wunused-result totally unusable. This is an example of a *real* problem that would have been caught by this warning: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25509#c28 Except that assert definition is broken, the standard assert macro in C/C++ does not have any side effects for NDEBUG. My point was actually about: v.empty(); // v.clear() was intended! Even without that assert() definition (which I imagine was done that way to prevent breaking code that relied on the side effects of the expressions inside it), it's basically impossible to enable it since then there's no good way (except for the horrible bogus hacky ignored variable) to create exceptions to the rule. Just because this option was introduced 10 years ago, because you invented it first, because you wrote unit tests to ensure this behavior, doesn't mean that it's correct or that it's useful the way it is... Please reconsider. I think it would be fine fine if you wanted to introduce an even stronger __attribute__((error_unused_result)) or similar that allows no exceptions, which could then be used for say malloc() or realloc() where there's really *really* no excuse to not save the return value, but I really don't see the point since there are always ways around it...
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #9 from Filipe Brandenburger filbranden at google dot com --- Or, conversely, please explain to me how changing the behavior (to allow a void-cast to silent the warning on a call to a warn_unused_result function) would actually affect anyone today... If it's a project that's actually using -Wunused-result (and they actually look at those warnings and care about them), then they're not really using a void-cast to ignore the return value, otherwise they would get a warning. Would the change of behavior really affect them? Not really. If they're using -Wunused-result but do not care about the warnings, well, do we really care? If anything (perhaps if they were adding void-casts for linters and other static analysis tools) it's possible that a lot of suprious warnings will be suppressed and they'll be left with a more manageable list and possibly start to actually look at those and fix them (since there's an easy way to explicitly say they should be ignored.) If they're using -Wno-unused-result, then the change of behavior doesn't affect them. If anything, if this *BUG* is fixed, they might actually reconsider and re-enable it. So please explain to me why sticking to the current unhelpful behavior is really that important.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 Lucas De Marchi lucas.de.marchi at gmail dot com changed: What|Removed |Added CC||lucas.de.marchi at gmail dot com --- Comment #10 from Lucas De Marchi lucas.de.marchi at gmail dot com --- (In reply to Andrew Pinski from comment #7) Again this has nothing to do with other lints, this attribute was designed so you can't ignore the return value. And most lints/static analysis tools will also look to see if you not really ignoring the return value in that you use it in an error case fashion. casting to void does not always cause (and really should not cause) static analysis to ignore what you did was broken for the API. It is not GCC's fault people are starting to misuse this attribute; you should write into those projects instead saying they are misunderstanding the attribute is being used incorrectly. I bet some of them, really don't want a warning even with a cast to void. You are ignoring the fact that the places in which the attribute is put is often out of control for those using that interface. This means they can't really remove the attribute or also that the attribute may make sense in MOST of the cases but not in that particular one. Casting to void is nowadays the de facto standard to say I really know what I'm doing and I don't care about the return value *here*. This is way better then letting each project invent their all workaround macros and even risk doing the wrong thing like the assert you pointed out as wrong. So maybe after 10 years it's time to revisit that decision? It's not that the behavior of when warnings are emitted are really set in stone and never changed. It pretty much did all this time and we kept adapting to it. If just changing the behavior is not acceptable then maybe splitting the -Wunused-result to let people ignore this particular case of cast to void as suggested would be good. The gcc vs clang vs linters debate and who invented the attribute don't belong to this discussion. It's better to discuss why we can't let the attribute be actually useful by letting the users to turn it off when they know what they are doing.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #16 from joseph at codesourcery dot com joseph at codesourcery dot com --- I'd say that for any function for which use of this attribute is appropriate, suppression of the warning should involve a detailed comment explaining why the particular use of the function is so exceptional that, very unusually, it is not possible to (for example) do anything useful with the information that the function call failed or it is otherwise exceptionally safe to ignore the result - and that any suppression of the warning should involve extra-careful code review when added. See, for example, how glibc handles use of diagnostic suppression pragmas, via macros DIAG_PUSH_NEEDS_COMMENT, DIAG_IGNORE_NEEDS_COMMENT and DIAG_POP_NEEDS_COMMENT, to make it extra-obvious if a patch is adding such uses and to make it obvious at the use site that such comments are needed. Now, the compiler cannot check whether an explanatory comment justifying diagnostic suppression is present, and cannot know how a particular project wishes to handle justifying exceptions to its normal coding style rules. But it can make it as visible as possible that something unusual and dubious is being done by ignoring the return value, to reduce the chance of it slipping by reviewers. I'd say that a warning suppressed by a (void) cast should be a separate attribute, e.g. warn_unused_result_weak.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425
--- Comment #17 from Filipe Brandenburger filbranden at google dot com ---
To make matters even worse, gcc doesn't even seem to be consistent with itself,
because in other situations it *does* accept a cast to void to silent warnings.
For example, -Wunused-but-set-variable with this test program (on gcc 4.8.2 on
Ubuntu):
void will_not_warn() {
int i __attribute__((unused));
i = 1;
}
void should_warn() {
int i;
i = 1;
}
void should_not_warn() {
int i;
i = 1;
(void) i;
}
See how there's no warning for should_not_warn which uses the (void) cast:
$ gcc -c -Wunused-but-set-variable test2.c
test2.c: In function ‘should_warn’:
test2.c:8:7: warning: variable ‘i’ set but not used
[-Wunused-but-set-variable]
int i;
^
And the same with -Wunused-parameter, see this test program:
int should_warn(int argument) {
return 0;
}
int should_not_warn(int argument) {
(void) argument;
return 0;
}
And the warning, again no warning in should_not_warn which uses the cast to
void:
$ gcc -c -Wunused-parameter test3.c
test3.c: In function ‘should_warn’:
test3.c:2:21: warning: unused parameter ‘argument’ [-Wunused-parameter]
int should_warn(int argument) {
^
And, to reiterate, you already *can* ignore the value of a warn_unused_result
function, all you have to do is store it in an otherwise unused variable. You
could even use a (void) ignored; on that variable on the following line to
silent warnings related to it being set and not used!
So I really don't see the big deal about saying that a cast to void in a
function result should not count as the coder saying that they explicitly
intend to disregard the return value of that function call.
I do agree with you that that code should have comments to indicate why it's
being disregarded and so on and that projects might want to create specific
macros to do it but I think that's the prerrogative of projects using these
features to decide how they'd like to handle it.
Cheers,
Filipe
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425
Filipe Brandenburger filbranden at google dot com changed:
What|Removed |Added
CC||filbranden at google dot com
--- Comment #2 from Filipe Brandenburger filbranden at google dot com ---
It turns out clang from LLVM seems to do the right thing here...
Using this as a test case:
__attribute__((warn_unused_result))
int foo() {
return -1;
}
int should_not_warn() {
return foo();
}
int should_warn() {
foo();
return 0;
}
int void_cast_should_not_warn() {
(void) foo();
return 0;
}
With gcc 5.1:
$ gcc -c test.c
test.c: In function ‘should_warn’:
test.c:12:4: warning: ignoring return value of ‘foo’, declared with attribute
warn_unused_result [-Wunused-result]
foo();
^
test.c: In function ‘void_cast_should_not_warn’:
test.c:17:4: warning: ignoring return value of ‘foo’, declared with attribute
warn_unused_result [-Wunused-result]
(void) foo();
^
With clang 3.5:
$ clang-3.5 -c test.c
test.c:12:4: warning: ignoring return value of function declared with
warn_unused_result attribute [-Wunused-result]
foo();
^~~
1 warning generated.
Other static analysis tools seem to also take a hint out of the (void) cast.
I mean, why make the programmer jump through hoops and store the return value
in an unused variable? Isn't the (void) cast more explicit than that anyways?
Right now the only choice I can see is to use -Wno-unused-result which
basically makes the feature useless when building using gcc...
Richard Blener proposed a patch here:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25509#c10
Would it make sense to adopt that patch?
Cheers,
Filipe
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #3 from rusty at rustcorp dot com.au --- Indeed, cast to void has been a standard I really want to ignore this notation. It's intuitive and obvious, and ISTR seeing it in the early 90s for lint warning suppression, for example.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 Konstantin Khlebnikov koct9i at gmail dot com changed: What|Removed |Added CC||koct9i at gmail dot com --- Comment #5 from Konstantin Khlebnikov koct9i at gmail dot com --- Is it possible to split this case into separate (sub)option -Wcast-unused-to-void to make this thing backward compatible? Because this is really annoying.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 Andrew Pinski pinskia at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |INVALID --- Comment #4 from Andrew Pinski pinskia at gcc dot gnu.org --- (In reply to Filipe Brandenburger from comment #2) It turns out clang from LLVM seems to do the right thing here... There is no right thing here. In fact some could argue that clang is doing the wrong thing here. GCC originally added this attribute and made the decision then that void would not suppress the attribute. There are even tests in the GCC testsuite testing that. https://gcc.gnu.org/ml/gcc-patches/2003-09/msg00851.html Also unlike most other extensions this one is well documented and even documents that it warns for this case. I am going to say people are mis-using this attribute and clang's attribute does not match up with GCC's attribute. Just the way life is.
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 --- Comment #6 from Filipe Brandenburger filbranden at google dot com --- Then please explain to me how this: (void) foo(); is any worse than this: int ignored __attribute__((unused)); ignored = foo(); /* do nothing with ignored here */ You can force me to assign the return value to something, but you can't really force me to do something useful with it... In the cases above, I think the former is a much more clear way to express my intent to ignore that value, I'm telling the compiler yeah, I know, I'm supposed to use this value, but in this particular case I really know what I'm doing so please bear with me here. The -Wunused-result warnings would be a lot more useful (or, actually, just useful) if I was able to whitelist the places where I want to ignore the values, since right now my only practical choice is to disable it completely with -Wno-unused-result. This is an example of a *real* problem that would have been caught by this warning: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25509#c28 And it's not just LLVM... Other linters and static analysis tools seem to agree that a (void) cast is an explicit indication that the coder meant to ignore those results, so why would gcc want to be the one exception here? Thanks! Filipe
[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 Manuel López-Ibáñez manu at gcc dot gnu.org changed: What|Removed |Added CC||manu at gcc dot gnu.org --- Comment #1 from Manuel López-Ibáñez manu at gcc dot gnu.org --- I am not the one who decides, but the conclusion of the previous discussion was that users of _wur (which are the API maintainers adding it) do want to get a warning even with (void) and GCC's testsuite explicitly tests for it. It is fairly easy to use -Wno-unused-result or #pragma GCC diagnostics if you want more fine-tuning. The location info in GCC has improved to the point that it is possible to ignore individual warnings using #pragmas. That seems far clearer to a reader and makes obvious which warning is ignored on purpose. Of course, we are happy to get as much help as we can, but I would not want you to waste your time on something that is likely not going to get approved.
