Bug 25509 - can't disable __attribute__((warn_unused_result))
Summary: can't disable __attribute__((warn_unused_result))
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 4.1.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks:
 
Reported: 2005-12-20 23:09 UTC by Dirk Mueller
Modified: 2015-06-11 13:37 UTC (History)
12 users (show)

See Also:
Host: i686-suse-linux
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2005-12-21 09:59:43


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Mueller 2005-12-20 23:09:47 UTC
casting to (void) doesn't avoid the unused_result warning. testcase: 

=== Cut ===
extern int foo() __attribute__((warn_unused_result));
int main()
{
   (void) foo();

   return 0;
}
=== Cut ===
g++ -Wall -W -O2 -c unused.cc

unused.cc: In function 'int main()':
unused.cc:4: warning: ignoring return value of 'int foo()', declared with attribute warn_unused_result
Comment 1 joseph@codesourcery.com 2005-12-20 23:53:48 UTC
Subject: Re:   New: can't voidify __attribute__((warn_unused_result))

On Tue, 20 Dec 2005, mueller at kde dot org wrote:

> casting to (void) doesn't avoid the unused_result warning. testcase: 

Why do you think this is a bug?  warn_unused_result is for cases where 
"not checking the result is either a security problem or always a bug".

Comment 2 Dirk Mueller 2005-12-21 00:07:41 UTC
background: glibc 2.3 CVS attributes "fwrite" and "write" with it, and it causes a lot (in the hundreds/thousands) of false positives for bigger software projects, because while it is indeed the case that they ignore the return value, it simply doesn't matter for the application (if, for example, it is used as debug output). Yes, write(2) can fail, but there are cases where the application can't possibly do anything about it, or even cares. 

Comment 3 Andrew Pinski 2005-12-21 03:20:02 UTC
This is a glibc bug and not a GCC bug then.
Comment 4 Dirk Mueller 2005-12-21 03:35:02 UTC
Care to explain how it is a glibc bug? its not documented that there shouldn't be a way to suppress the warning. 

I agree glibc is overly paranoid and pedantic, but that doesn't make it less of a gcc issue. 

Comment 5 Andrew Pinski 2005-12-21 03:47:45 UTC
Actually it is documented that this is acting the way it is acting, just not with the docs of the attributes:
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 (see Function Attributes).

"should never" means it cannot be the result cannot be ignored at all (well assigning to a variable and ignoring that is a work around).


As shown this is not a GCC bug as GCC is acting as documented.

The reason why it is a glibc bug is that it is very over the top of adding the attribute here.
Comment 6 joseph@codesourcery.com 2005-12-21 03:55:29 UTC
Subject: Re:  can't voidify __attribute__((warn_unused_result))

On Wed, 21 Dec 2005, pinskia at gcc dot gnu dot org wrote:

> The reason why it is a glibc bug is that it is very over the top of adding the
> attribute here.

And indeed there is no logical difference between printf and fwrite here, 
but glibc is marking fwrite and not printf.

In both cases, a valid programming style is to use fflush and ferror at 
the end to check for errors, rather than checking on every write, or to 
check the return value of fclose.  A program that uses fwrite without 
checking the return value or such a subsequent error is buggy - so is one 
using printf and failing later to check for errors on stdout.  (GCC is 
among such buggy programs; "gcc --help >/dev/full" does not return error 
status as it should.)  But checking at the end suffices (albeit losing 
information about the value of errno for the original error), you don't 
need to check at every call.

Comment 7 Dirk Mueller 2005-12-21 04:02:46 UTC
ok, lets assume that you meant with "can not be ignored" actually "must not be ignored". now thats where the definitions in RFC2119 kick in: 

2. MUST NOT   This phrase, or the phrase "SHALL NOT", mean that the
   definition is an absolute prohibition of the specification.

4. SHOULD NOT   This phrase, or the phrase "NOT RECOMMENDED" mean that
   there may exist valid reasons in particular circumstances when the
   particular behavior is acceptable or even useful, but the full
   implications should be understood and the case carefully weighed
   before implementing any behavior described with this label.


The documentation correctly states SHOULD NOT, and thats distinctively different from MUST NOT. 

I already agreed that glibc is over the top, nevertheless the (void)ify trick doesn't suppress the warning, and either that behaviour is a bug or the behaviour that assigning to a dummy variable (which is never read and therefore dead storage) doesn't warn is a bug. you choose. 

Comment 8 Dirk Mueller 2005-12-21 04:17:05 UTC
 
> ok, lets assume that you meant with "can not be ignored" actually "must not > be ignored". now thats where the definitions in RFC2119 kick in: 

Hmm, that wasn't meant so harsh than it sounds after rereading. sorry about that. 
Comment 9 Richard Biener 2005-12-21 09:59:42 UTC
The (technical) problem that the void cast does not work is that the warning
is applied after gimplification, which strips the cast to void.  So, this is
another case where warnings from the middle-end show their bad side - not that
it would be easy to move to the frontend(s).

This is at least a bug because the warning cannot be disabled:

      if (lookup_attribute ("warn_unused_result", TYPE_ATTRIBUTES (ftype)))
        {
          if (fdecl)
            warning (0, "%Hignoring return value of %qD, "
                     "declared with attribute warn_unused_result",
                     EXPR_LOCUS (t), fdecl);
          else
            warning (0, "%Hignoring return value of function "
                     "declared with attribute warn_unused_result",
                     EXPR_LOCUS (t));
        }

so, confirmed.  Suggestions for a proper -Wno-XXX identifier? -Wno-unused-result?
Comment 10 Richard Biener 2005-12-21 10:16:11 UTC
This "fixes" it:

*** gimplify.c  (revision 108853)
--- gimplify.c  (working copy)
*************** gimplify_expr (tree *expr_p, tree *pre_p
*** 4203,4210 ****
              break;
            }
  
!         if (VOID_TYPE_P (TREE_TYPE (*expr_p))
!             || fallback == fb_none)
            {
              /* Just strip a conversion to void (or in void context) and
                 try again.  */
--- 4203,4211 ----
              break;
            }
  
!         if ((VOID_TYPE_P (TREE_TYPE (*expr_p))
!              || fallback == fb_none)
!             && ! TREE_CODE (TREE_OPERAND (*expr_p, 0)) == CALL_EXPR)
            {
              /* Just strip a conversion to void (or in void context) and
                 try again.  */

it makes the gimplifier output

main ()
{
  int D.1519;
  int D.1520;

  D.1519 = foo ();
  D.1520 = 0;
  return D.1520;
}

instead of (with the (void) cast removed):

main ()
{
  int D.1519;

  foo ();
  D.1519 = 0;
  return D.1519;
}

The question whether this is an appropriate fix or just my astonishing ability 
to find ugly workarounds remains to be answered ;)
Comment 11 Richard Biener 2005-12-21 10:59:18 UTC
Hm, we even check in the testsuite that we still warn for (void) foo():

  check1 ();            /* { dg-warning "ignoring return value of" } */
  (void) check1 ();     /* { dg-warning "ignoring return value of" } */
  check1 (), bar ();    /* { dg-warning "ignoring return value of" } */
Comment 12 joseph@codesourcery.com 2005-12-21 13:04:32 UTC
Subject: Re:  can't voidify __attribute__((warn_unused_result))

On Wed, 21 Dec 2005, mueller at kde dot org wrote:

> ok, lets assume that you meant with "can not be ignored" actually "must not be
> ignored". now thats where the definitions in RFC2119 kick in: 

The documentation isn't written in terms of RFC2119.

> The documentation correctly states SHOULD NOT, and thats distinctively
> different from MUST NOT. 

It says "should never", not "should not".

For the sort of functions this is intended for, if you really want to 
ignore the return value then you should probably have a conditional and a 
??? comment in every place you do so.  Not simply a cast to void which as 
the manual notes is visual noise.

  if (error_return()) {
    /* ??? For reason X we can't handle this error sensibly.  */
    abort();
  }

(I wouldn't recommend omitting the abort there; the comment would need a 
more detailed justification of why in the particular case it's safe to 
carry on if the abort is omitted.)

Comment 13 Dirk Mueller 2005-12-21 14:07:08 UTC
ok, then, lets see if we can get this fixed in glibc. 

Comment 14 Gabriel Dos Reis 2005-12-21 17:05:11 UTC
Subject: Re:  can't voidify __attribute__((warn_unused_result))

"mueller at kde dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| ok, then, lets see if we can get this fixed in glibc. 

good luck.
Comment 15 Frank Ch. Eigler 2007-03-15 21:41:40 UTC
This still seems fishy to me FWIW: both gcc's implementation and documentation appear to be needlessly aggressive.
Comment 16 Paolo Bonzini 2008-10-17 11:40:15 UTC
It does not matter if it is a "security" issue; if void-ifying is not an acceptable workaround, there must be at the very least a Wno-* option to disable it.
Comment 17 joseph@codesourcery.com 2008-10-17 15:31:09 UTC
Subject: Re:  can't disable __attribute__((warn_unused_result))

On Fri, 17 Oct 2008, bonzini at gnu dot org wrote:

> It does not matter if it is a "security" issue; if void-ifying is not an
> acceptable workaround, there must be at the very least a Wno-* option to
> disable it.

The workaround is to change the header declaring the function with the 
attribute.  There isn't an option to disable the error for calling a 
prototyped function with the wrong number of arguments either; if you feel 
you know better than the library author how many arguments the function 
should take for a particular use case in the program, you'll need to 
change the library or conform to the API it specifies.  This attribute is 
giving further information about the API for a function.

In the case of fwrite, for example, the only obvious case where checking 
would be useless is if you already are writing an error message before 
exiting with error status and so an error writing the error message could 
not usefully be reported anywhere and wouldn't lead to a change of exit 
status.  This suggests you might have an xfwrite function that looks at 
the return value and acts on it unless a static flag is set to say the 
program is in the process of exiting with an error.  Coding in the style 
suggested by the library API should be easier than trying to work around 
the API to code in another style.

Comment 18 Paolo Bonzini 2008-10-17 15:48:09 UTC
> In the case of fwrite, for example, the only obvious case where checking 
> would be useless is if you already are writing an error message before 
> exiting with error status and so an error writing the error message could 
> not usefully be reported anywhere and wouldn't lead to a change of exit 
> status.

Not really.  The return code of fwrite is not only useless: worse, it gives a *false* sense of security.  Stuff can stay in the buffers, only to give errors when you do an fflush or an fclose, which do not have the attribute in glibc (as of July 2007).

It is much better to do

  fwrite (buf, m, n, f);
  if (fflush (f) != EOF)
    perror ("write");
  if (fclose (f) != EOF)
    perror ("close");

than checking the return code of fwrite, and that's more or less what coreutils does.  Anyway this is OT, because this would be a glibc bug.

Back to the GCC point-of-view, the situation is similar to setting a format(printf) attribute on a printf-like function that also has some extension.  It would work for some calls, maybe most, but not for all of them.  So the solution would be to use -Wno-format, either directly or via #pragma GCC diagnostic.  This warning is not mandated by any standard, after all.
Comment 19 joseph@codesourcery.com 2008-10-17 16:55:24 UTC
Subject: Re:  can't disable __attribute__((warn_unused_result))

On Fri, 17 Oct 2008, bonzini at gnu dot org wrote:

> > In the case of fwrite, for example, the only obvious case where checking 
> > would be useless is if you already are writing an error message before 
> > exiting with error status and so an error writing the error message could 
> > not usefully be reported anywhere and wouldn't lead to a change of exit 
> > status.
> 
> Not really.  The return code of fwrite is not only useless: worse, it gives a
> *false* sense of security.  Stuff can stay in the buffers, only to give errors
> when you do an fflush or an fclose, which do not have the attribute in glibc
> (as of July 2007).
> 
> It is much better to do
> 
>   fwrite (buf, m, n, f);
>   if (fflush (f) != EOF)
>     perror ("write");
>   if (fclose (f) != EOF)
>     perror ("close");
> 
> than checking the return code of fwrite, and that's more or less what coreutils
> does.  Anyway this is OT, because this would be a glibc bug.

Yes, I previously noted this as an alternative valid style in comment#6. 
glibc has chosen to make one style much easier than the other and that's a 
matter for the glibc maintainers, not for GCC to work around glibc.

> Back to the GCC point-of-view, the situation is similar to setting a
> format(printf) attribute on a printf-like function that also has some
> extension.  It would work for some calls, maybe most, but not for all of them. 
> So the solution would be to use -Wno-format, either directly or via #pragma GCC
> diagnostic.  This warning is not mandated by any standard, after all.

Yes, all warnings in GCC should have options to control them as a general 
principle of warning control, but some (such as in this case) would be 
there more for a general principle than because they should actually be 
used.  GNU software should not be working around other GNU software; if 
some GNU software has a problem with attributes used in glibc then in the 
first instance the maintainers of both packages should try to ensure that 
glibc's headers and the other software's coding style work well together.

Comment 20 tz 2008-11-22 15:42:56 UTC
There minimally needs to be a way of turning this warning off in GCC.

GCC should not be trying to micromanage coding styles - either of the rest of gnu software or anywhere else, but at least until you clean up every bit of your own code, there should be a way of disabling the warning clutter.

HUNDREDS OF WARNINGS ARE NO BETTER THAN NO WARNINGS AT ALL.

I can't even find errors in the pages bilge that now spews out from a normal compile.  It might be and probably is appropriate with -Wall turned on.

And I really would like to be able to treat warnings as errors when they are legitimate warnings.

For now, I've hexedited cc1 to change the string so it won't be found and have to add -Wno-attributes so I don't get errors from things I might need.

I'm getting it even with -Wall turned off (version 4.3.2).  And I still should be able to disable it.

Somehow GCC and gnu thinks

    int dummy93857 = fwrite( buf, 1, 1, fp );

is so far superior code to just

    fwrite( buf, 1, 1, fp );

that it now must enforce it on every possible line.

Sometimes ignoring returns is the right (or better) thing to do instead of cluttering up the code.  Not every line of code is critical kernel or system code that can introduce security holes.  Not every call needs to have its exact behavior on the particular instance carefully monitored.

The author of the libraries can often make a bad choice.  And there are hundreds of instances - maybe 99% of them are good, but the bad ones on common functions are causing a great deal of noise.  And there is not a pedantic_warn_unused_result (with a -Wunused-result which would promote it), which would be perfect for the instances noted here and more easily made.  And perhaps even an error_unused_result.

I think it would be easy to argue for the large bodies of code that certain functions have return values that are conventionally ignored so should only warn at a higher level of checking than ordinary warnings.  Right now I have to argue each individual case with the only options to keep it (and the pages of new warnings) or remove it (and in the few cases where it might be critical be silent).

gcc currently has no middle option.

Sometimes return values are at a point where you can't do anything anyway like the exit example.  Somehow, if a printf, or an equivalent fwrite of a formatted string to stdout or stderr fails, what do you do?  Errors have both probability and criticality.  And there are a lot of highly improbable cases, and lots of non-critical sections.  If my CPU is melting down or my memory giving errors, I have worse problems.  If the number of parameters doesn't match a function declaration, it is likely an error that will cause things to fail 90% of the time.   99.99% of the time, f//read/write will return the expected value.  If fclose fails, what do you do?  And fwrite won't return the error, fflush might (but if it doesn't do a sync(), and writes are cached to a failing disk...).

Perhaps it is because we don't have a finer gradation (an INFO or MAY equivalent to the SHOULD/WARNING, MUST/ERROR).  The lack of checking a return, at least in the cases where the functions are mainly the side-effect (and if fwrite fails, perhaps there should be a signal or exception, and not depend on the return code if it is so critical) doesn't reach the threshold of a PERMANENTLY ENABLED warning.  It does reach the threshold of the things I usually check with -pedantic.  Like signed-unsigned mismatches.  Subtle printf format errors.  In my later QC checks I do turn everything on and verify every line of code.

I would work on adding a pedantic_* (and maybe error_*) set of attributes, but until then, leave the choice to the author of the program.  THIS WARNING IS A *GOOD* THING, but it doesn't apply to every program or every function, or every use of that function.  Many functions are used both in critical and noncritical forms, and there are a lot of existing programs that instead of being clear are now cluttered.

One of the reasons I don't normally use C++ is the stupidity where I am forced to lower the quality of my code because of what it enforces or doesn't enforce so instead of a concise function, it will only compile a bloated blob.  This warning is something like that.

I write code in C.  I know better what I'm writing that you or the compiler does.  I know when errors are critical and or likely at a specific point in my code.  And all I want is the choice to either have this (or any other common but not critical warning) enabled or disabled.
Comment 21 pinskia@gmail.com 2008-11-22 17:17:06 UTC
Subject: Re:  can't disable __attribute__((warn_unused_result))



Sent from my iPhone

On Nov 22, 2008, at 7:42 AM, "thomas at mich dot com" <gcc-bugzilla@gcc.gnu.org 
 > wrote:

>
>
> ------- Comment #20 from thomas at mich dot com  2008-11-22 15:42  
> -------
> There minimally needs to be a way of turning this warning off in GCC.
>
> GCC should not be trying to micromanage coding styles - either of  
> the rest of
> gnu software or anywhere else, but at least until you clean up every  
> bit of
> your own code, there should be a way of disabling the warning clutter.

Why GCC is not micromanaging at all, it just allows the developer of  
the API to have the warning.  So your complaints here are useless.

>
>
> HUNDREDS OF WARNINGS ARE NO BETTER THAN NO WARNINGS AT ALL.
>
> I can't even find errors in the pages bilge that now spews out from  
> a normal
> compile.  It might be and probably is appropriate with -Wall turned  
> on.
>
> And I really would like to be able to treat warnings as errors when  
> they are
> legitimate warnings.
>
> For now, I've hexedited cc1 to change the string so it won't be  
> found and have
> to add -Wno-attributes so I don't get errors from things I might need.
>
> I'm getting it even with -Wall turned off (version 4.3.2).  And I  
> still should
> be able to disable it.
>
> Somehow GCC and gnu thinks
>
>    int dummy93857 = fwrite( buf, 1, 1, fp );
>
> is so far superior code to just
>
>    fwrite( buf, 1, 1, fp );
>
> that it now must enforce it on every possible line.

It is not GCC which thinks that, it is the providers of your headers  
for fwriye that thinks that.

>
>
> Sometimes ignoring returns is the right (or better) thing to do  
> instead of
> cluttering up the code.  Not every line of code is critical kernel  
> or system
> code that can introduce security holes.  Not every call needs to  
> have its exact
> behavior on the particular instance carefully monitored.

Again we just provide the author of the Api to say that.

>
>
> The author of the libraries can often make a bad choice.

Yes and you should complain to them instead of us then.

> And there are
> hundreds of instances - maybe 99% of them are good, but the bad ones  
> on common
> functions are causing a great deal of noise.  And there is not a
> pedantic_warn_unused_result (with a -Wunused-result which would  
> promote it),
> which would be perfect for the instances noted here and more easily  
> made.  And
> perhaps even an error_unused_result.
>
> I think it would be easy to argue for the large bodies of code that  
> certain
> functions have return values that are conventionally ignored so  
> should only
> warn at a higher level of checking than ordinary warnings.  Right  
> now I have to
> argue each individual case with the only options to keep it (and the  
> pages of
> new warnings) or remove it (and in the few cases where it might be  
> critical be
> silent).
>
> gcc currently has no middle option.

Also this attribute is not on by default in glibc so you are asking to  
turn on the style based warnings.

>
>
> Sometimes return values are at a point where you can't do anything  
> anyway like
> the exit example.  Somehow, if a printf, or an equivalent fwrite of  
> a formatted
> string to stdout or stderr fails, what do you do?  Errors have both  
> probability
> and criticality.  And there are a lot of highly improbable cases,  
> and lots of
> non-critical sections.  If my CPU is melting down or my memory  
> giving errors, I
> have worse problems.  If the number of parameters doesn't match a  
> function
> declaration, it is likely an error that will cause things to fail  
> 90% of the
> time.   99.99% of the time, f//read/write will return the expected  
> value.  If
> fclose fails, what do you do?  And fwrite won't return the error,  
> fflush might
> (but if it doesn't do a sync(), and writes are cached to a failing  
> disk...).
>
> Perhaps it is because we don't have a finer gradation (an INFO or MAY
> equivalent to the SHOULD/WARNING, MUST/ERROR).  The lack of checking  
> a return,
> at least in the cases where the functions are mainly the side-effect  
> (and if
> fwrite fails, perhaps there should be a signal or exception, and not  
> depend on
> the return code if it is so critical) doesn't reach the threshold of a
> PERMANENTLY ENABLED warning.  It does reach the threshold of the  
> things I
> usually check with -pedantic.  Like signed-unsigned mismatches.   
> Subtle printf
> format errors.  In my later QC checks I do turn everything on and  
> verify every
> line of code.
>
> I would work on adding a pedantic_* (and maybe error_*) set of  
> attributes, but
> until then, leave the choice to the author of the program.  THIS  
> WARNING IS A
> *GOOD* THING, but it doesn't apply to every program or every  
> function, or every
> use of that function.  Many functions are used both in critical and  
> noncritical
> forms, and there are a lot of existing programs that instead of  
> being clear are
> now cluttered.
>
> One of the reasons I don't normally use C++ is the stupidity where I  
> am forced
> to lower the quality of my code because of what it enforces or  
> doesn't enforce
> so instead of a concise function, it will only compile a bloated  
> blob.  This
> warning is something like that.
>
> I write code in C.  I know better what I'm writing that you or the  
> compiler
> does.  I know when errors are critical and or likely at a specific  
> point in my
> code.  And all I want is the choice to either have this (or any  
> other common
> but not critical warning) enabled or disabled.

Someone turned these attributes in your glibc to be on by default so  
again it is not our fault.

>
>
>
> -- 
>
> thomas at mich dot com changed:
>
>           What    |Removed                     |Added
> --- 
> --- 
> ----------------------------------------------------------------------
>                 CC|                            |thomas at mich dot com
>
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25509
>
Comment 22 Frank Ch. Eigler 2008-11-22 18:35:04 UTC
(In reply to comment #21)
> Sent from my iPhone

Good to know.
 
> > GCC should not be trying to micromanage coding styles - either of  
> > the rest of gnu software or anywhere else, but at least until you
> > clean up every bit of your own code, there should be a way of disabling
> > the warning clutter.
> 
> Why GCC is not micromanaging at all, it just allows the developer of  
> the API to have the warning.  So your complaints here are useless.

What the poster seems to be requesting is another -Wno-unused-FOO flag
to override this warning.
Comment 23 Manuel López-Ibáñez 2009-07-10 07:27:49 UTC
Subject: Bug 25509

Author: manu
Date: Fri Jul 10 07:27:32 2009
New Revision: 149458

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=149458
Log:
2009-07-10  Manuel López-Ibáñez  <manu@gcc.gnu.org>

	PR 25509
	PR 40614
	* c.opt (Wunused-result): New.
	* doc/invoke.texi: Document it.
	* c-common.c (c_warn_unused_result): Use it.
testsuite/
	* g++.dg/warn/unused-result1-Werror.c: New.

Added:
    trunk/gcc/testsuite/g++.dg/warn/unused-result1-Werror.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/c-common.c
    trunk/gcc/c.opt
    trunk/gcc/doc/invoke.texi
    trunk/gcc/testsuite/ChangeLog

Comment 24 Manuel López-Ibáñez 2009-07-10 07:29:27 UTC
FIXED in GCC 4.5
Comment 25 Bruce Korb 2010-08-30 14:42:52 UTC
> > gcc currently has no middle option.
> 
> Also this attribute is not on by default in glibc so you are asking to  
> turn on the style based warnings.

(In reply to comment #24)
> FIXED in GCC 4.5

After having waded through this long series of comments, I am left
wondering just how this got addressed.  Does  --no-warn-unused-result
mean that fwrite() usage may be cast to void, or that it may be
treated as if it were a void procedure?  I think it is very reasonable
to warn if a returned result is not handled.  Casting to void is
a valid way to handle the result.  I would like warnings when returned
results are not handled.  What does the fix do?
Comment 26 Richard Biener 2010-08-30 15:00:49 UTC
(In reply to comment #25)
> > > gcc currently has no middle option.
> > 
> > Also this attribute is not on by default in glibc so you are asking to  
> > turn on the style based warnings.
> 
> (In reply to comment #24)
> > FIXED in GCC 4.5
> 
> After having waded through this long series of comments, I am left
> wondering just how this got addressed.  Does  --no-warn-unused-result
> mean that fwrite() usage may be cast to void, or that it may be
> treated as if it were a void procedure?  I think it is very reasonable
> to warn if a returned result is not handled.  Casting to void is
> a valid way to handle the result.  I would like warnings when returned
> results are not handled.  What does the fix do?

It simply adds -W[no-]unused-result and completely enables/disables all
unused result warnings.
Comment 27 Eric Blake 2010-08-30 21:09:28 UTC
See:
http://sourceware.org/bugzilla/show_bug.cgi?id=11959
for the glibc side of this bug (namely, fwrite() shouldn't be tagged wur).
Comment 28 Paul Pluzhnikov 2014-08-13 22:13:21 UTC
Google ref: b/16983603.

I wouldn't call this bug fixed.

I have just found ~30 bugs in our code, where someone wrote:

  vector<int> v;
  ...
  v.empty();   // v.clear() was intended!

No problem, I'll just add warn_unused_result to vector::empty(), right?

Well, that did expose the 30 bugs above, but unfortunately I can't do that permanently, because it also exposed this false positive:

   assert(v.empty());

where assert in NDEBUG mode expanded into

  static_cast<void>(v.empty());

and triggered the warning :-(

P.S. Some of the bugs I found were in parts of the code imported from open-source projects, so it's not a problem that is specific to just Google. If the assert problem could be addressed, adding warn_unused_result to trunk libstdc++ would benefit everyone.
Comment 29 Manuel López-Ibáñez 2014-08-13 23:46:58 UTC
(In reply to Paul Pluzhnikov from comment #28)
> Well, that did expose the 30 bugs above, but unfortunately I can't do that
> permanently, because it also exposed this false positive:
> 
>    assert(v.empty());
> 
> where assert in NDEBUG mode expanded into
> 
>   static_cast<void>(v.empty());

Isn't assert in NDEBUG mode guaranteed to not evaluate its argument? The above seems to violate that assumption.

In C++ you could do this:

template<typename T>
inline T ignore_result(T x __attribute__((unused)))
{
    return x;
}
extern int foo() __attribute__((warn_unused_result));

int main()
{
   ignore_result(foo());
   return 0;
}

Another alternative is to use #pragma GCC diagnostics push/ignored/pop. Ideally you could encapsulate that into a macro "ignore_result", but #pragma diagnostics does not work well in a macro definition yet (I cannot remember the PR number for this).
Comment 30 Manuel López-Ibáñez 2014-08-13 23:48:38 UTC
(In reply to Paul Pluzhnikov from comment #28)
> P.S. Some of the bugs I found were in parts of the code imported from
> open-source projects, so it's not a problem that is specific to just Google.
> If the assert problem could be addressed, adding warn_unused_result to trunk
> libstdc++ would benefit everyone.

That seems a different issue (and it will require convincing different people) than this one. Can you open a new PR if you really think it is a good idea?
Comment 31 Filipe Brandenburger 2015-06-11 00:52:54 UTC
See bug 66425 for the cast to (void) to ignore warn_unused_result for a single case:

It looks like clang is already doing the right thing here...
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425#c2

gcc should catch up.

Cheers,
Filipe
Comment 32 Manuel López-Ibáñez 2015-06-11 08:08:15 UTC
(In reply to Filipe Brandenburger from comment #31)
> gcc should catch up.

I thought Google employed some capable C/C++ engineers...
Comment 33 Manuel López-Ibáñez 2015-06-11 08:54:34 UTC
(In reply to Manuel López-Ibáñez from comment #32)
> (In reply to Filipe Brandenburger from comment #31)
> > gcc should catch up.
> 
> I thought Google employed some capable C/C++ engineers...

What I meant is that those engineers, if they exist, could help GCC "catch up" (whatever that means)... gcc does not develop itself or by magic gnomes, you know.
Comment 34 Manuel López-Ibáñez 2015-06-11 09:01:38 UTC
(In reply to Manuel López-Ibáñez from comment #33)
> (In reply to Manuel López-Ibáñez from comment #32)
> > (In reply to Filipe Brandenburger from comment #31)
> > > gcc should catch up.
> > 
> > I thought Google employed some capable C/C++ engineers...
> 
> What I meant is that those engineers, if they exist, could help GCC "catch
> up" (whatever that means)... gcc does not develop itself or by magic gnomes,
> you know.

Hum, that still sounds worse than it sounded in my mind. Sorry, I'm dense this morning :) 

Just: help us, we love you, let's make the world a better place together :)

(deleting comments in bugzilla would be helpful sometimes).
Comment 35 Filipe Brandenburger 2015-06-11 13:37:47 UTC
(In reply to Manuel López-Ibáñez from comments)

Don't worry, I got what you mean...

Though I don't think coming up with code to fix it is the issue here, in comment #10 a patch was provided (which admittedly I haven't tested personally) to turn a void cast into a temporary assignment (which I believe would have been optimized out later in the pipeline) so I wonder why that hasn't really gone forward...

Cheers,
Filipe