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
Depends on:
Blocks:
 
Reported: 2015-06-05 00:38 UTC by rusty
Modified: 2018-08-17 02:59 UTC (History)
17 users (show)

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


Attachments

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.