Bug 66425 - (void) cast doesn't suppress __attribute__((warn_unused_result))
Summary: (void) cast doesn't suppress __attribute__((warn_unused_result))
Status: UNCONFIRMED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 4.9.2
: P3 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
: 89031 (view as bug list)
Depends on:
Blocks: Wunused 94389
  Show dependency treegraph
 
Reported: 2015-06-05 00:38 UTC by rusty
Modified: 2023-09-06 11:51 UTC (History)
18 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
POC patch to add -Wunused-result=strict (1.65 KB, patch)
2023-04-22 21:20 UTC, Andrew Church
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description rusty 2015-06-05 00:38:32 UTC
Yes, a repeat of 2005's https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25509

The world, especially glibc and the Linux kernel, have adopted __wur with a passion, taking it well outside the "must" case (basically, realloc and nothing else) into the "should" case.  For better or worse.

Thus I politely request that an explicit (void) cast work to disable this warning.  Having projects create their own custom macros/templates to do this is a worse outcome, and doesn't stop idiocy like ignoring realloc() anyway.

Please consider.
PS.  Happy to create a patch, if that would help.
Comment 1 Manuel López-Ibáñez 2015-06-05 07:21:06 UTC
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.
Comment 2 Filipe Brandenburger 2015-06-11 00:51:05 UTC
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
Comment 3 rusty 2015-06-11 05:26:25 UTC
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.
Comment 4 Andrew Pinski 2015-06-11 05:39:53 UTC
(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.
Comment 5 Konstantin Khlebnikov 2015-06-11 05:51:59 UTC
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.
Comment 6 Filipe Brandenburger 2015-06-11 05:55:38 UTC
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
Comment 7 Andrew Pinski 2015-06-11 06:06:27 UTC
(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
Comment 8 Filipe Brandenburger 2015-06-11 06:37:44 UTC
(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...
Comment 9 Filipe Brandenburger 2015-06-11 06:45:59 UTC
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.
Comment 10 Lucas De Marchi 2015-06-11 06:49:18 UTC
(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.
Comment 11 Manuel López-Ibáñez 2015-06-11 08:41:03 UTC
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.
Comment 12 Filipe Brandenburger 2015-06-11 13:57:03 UTC
(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
Comment 13 Andreas Schwab 2015-06-11 14:10:05 UTC
> 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.
Comment 14 Manuel López-Ibáñez 2015-06-11 14:25:30 UTC
(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!
Comment 15 Lucas De Marchi 2015-06-11 15:08:29 UTC
(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.
Comment 16 joseph@codesourcery.com 2015-06-11 15:48:57 UTC
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.
Comment 17 Filipe Brandenburger 2015-06-11 16:57:40 UTC
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
Comment 18 Segher Boessenkool 2015-06-14 17:29:19 UTC
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.  */
}
Comment 19 rusty 2015-06-14 20:28:06 UTC
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!
Comment 20 Jan Engelhardt 2015-08-18 20:53:57 UTC
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?
Comment 21 rusty 2015-08-20 05:35:37 UTC
jengelh at inai dot de <gcc-bugzilla@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.
Comment 22 Filipe Brandenburger 2015-08-20 06:05:21 UTC
(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
Comment 23 costinc 2016-04-07 17:29:02 UTC
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.
Comment 24 Jason Merrill 2016-04-28 16:29:26 UTC
I agree that (void) should suppress the warning.  Jakub, do you remember why you made a different choice?
Comment 25 Jakub Jelinek 2016-04-28 16:35:40 UTC
(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.
Comment 26 Steven Bosscher 2016-04-28 19:08:06 UTC
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)?
Comment 27 Manuel López-Ibáñez 2016-05-01 21:10:00 UTC
(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.
Comment 28 Adam Borowski 2017-08-28 10:02:30 UTC
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").
Comment 29 Andrzej Krzemienski 2017-10-11 07:22:36 UTC
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
}
```
Comment 30 Yongwei Wu 2017-12-15 09:41:56 UTC
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!
Comment 31 Hans Ecke 2017-12-15 16:31:02 UTC
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.
Comment 32 Segher Boessenkool 2017-12-15 18:25:34 UTC
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).
Comment 33 costinc 2017-12-15 19:29:51 UTC
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).
Comment 34 Ed Catmur 2018-05-16 16:39:51 UTC
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;
  }
Comment 35 Jason Merrill 2018-05-16 17:12:12 UTC
Is there a reason you can't use [[nodiscard]]?
Comment 36 Jason Merrill 2018-05-16 17:12:58 UTC
(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.
Comment 37 Andrew Pinski 2019-01-24 09:40:15 UTC
*** Bug 89031 has been marked as a duplicate of this bug. ***
Comment 38 Petr Skocik 2020-03-28 11:07:24 UTC
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)).
Comment 39 Nick Desaulniers 2021-07-27 19:33:23 UTC
Cross referencing an LLVM bug on a similar discussion:
https://bugs.llvm.org/show_bug.cgi?id=51228
Comment 40 Andrew Church 2023-04-22 21:20:10 UTC
Created attachment 54906 [details]
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.
Comment 41 Sam James 2023-04-23 07:07:17 UTC
(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).
Comment 42 Andrew Church 2023-04-23 07:53:45 UTC
(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.
Comment 43 Segher Boessenkool 2023-04-23 20:00:18 UTC
(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.
Comment 44 Andrew Church 2023-04-23 21:51:05 UTC
(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.
Comment 45 Andrew Pinski 2023-04-23 22:03:47 UTC
(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.
Comment 46 Andrew Church 2023-04-23 23:06:12 UTC
(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.
Comment 47 rusty 2023-04-24 06:45:54 UTC
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 :)
Comment 48 Andrew Church 2023-04-24 08:21:48 UTC
(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.
Comment 49 Jakub Jelinek 2023-04-24 08:26:32 UTC
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.
Comment 50 Florian Weimer 2023-04-24 08:35:32 UTC
(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)).
Comment 51 Segher Boessenkool 2023-04-24 09:06:04 UTC
(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?
Comment 52 Jan Engelhardt 2023-04-24 09:18:40 UTC
>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);
Comment 53 Andrew Church 2023-04-24 09:41:13 UTC
(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.
Comment 54 Roman Krotov 2023-08-09 22:33:21 UTC
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&type=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&type=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.
Comment 55 Ed Catmur 2023-08-09 23:09:14 UTC
(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.
Comment 56 Martin Uecker 2023-08-10 09:23:16 UTC
[[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.
Comment 57 Roman Krotov 2023-08-10 12:25:52 UTC
(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.
Comment 58 Ed Catmur 2023-08-10 18:12:53 UTC
(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]].
Comment 59 Roman Krotov 2023-08-17 13:09:11 UTC
(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.
Comment 60 Segher Boessenkool 2023-09-06 11:07:02 UTC
(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.
Comment 61 Andrew Church 2023-09-06 11:51:51 UTC
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.