[Bug c/94389] New: __attribute__((warn_unused_result)) will warn if the result is discarded as an optimisation

[Bug c/94389] New: __attribute__((warn_unused_result)) will warn if the result is discarded as an optimisation

[felix.von.s at posteo dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94389

            Bug ID: 94389
           Summary: __attribute__((warn_unused_result)) will warn if the
                    result is discarded as an optimisation
           Product: gcc
           Version: 9.3.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: felix.von.s at posteo dot de
  Target Milestone: ---

With the code below, the compiler warns about an unused result, even though the
result is used:

    #define O_TEXT 0  /* defined on Windows and DOS, but not needed on Unix */

    __attribute__((warn_unused_result))
    extern int text_mode(void);

    int get_flags(void) {
        return text_mode() ? O_TEXT : 0;
    }

Similarly, there is a warning if the result of a function marked with
__attribute__((warn_unused_result)) is multiplied by 0, and probably with other
expressions that are easily constant-folded. If the function also has
__attribute__((const)), there is no warning.

I think this issue could be resolved as a by-product of fixing bug 66425.

[Bug c/94389] __attribute__((warn_unused_result)) will warn if the result is discarded as an optimisation

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94389

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marxin at gcc dot gnu.org

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
We end up during gimplification with:

get_flags ()
{
  int D.1939;

  text_mode ();
  D.1939 = 0;
  return D.1939;
}

which then leads to unused value. While:

int get_flags2(void) {
  return text_mode() ? O_TEXT : foo ();
}

is fine and no warning is reported.

[Bug c/94389] __attribute__((warn_unused_result)) will warn if the result is discarded as an optimisation

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94389

Segher Boessenkool <segher at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
                 CC|                            |segher at gcc dot gnu.org
   Last reconfirmed|                            |2020-03-30

--- Comment #2 from Segher Boessenkool <segher at gcc dot gnu.org> ---
Earlier than that, we have


;; Function get_flags (null)
;; enabled by -tree-original


{
  return text_mode ();, 0;
}


which already has lost (and looks funny btw).

Confirmed btw.

[Bug c/94389] __attribute__((warn_unused_result)) will warn if the result is discarded as an optimisation

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94389

--- Comment #3 from Segher Boessenkool <segher at gcc dot gnu.org> ---
The C frontend dumps nothing for -fdump-lang-all, but the C++ frontend
shows (in .002l.raw) that the ?: is optimised to just a constant zero
there, already.

[Bug c/94389] __attribute__((warn_unused_result)) will warn if the result is discarded as an optimisation

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94389

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Compared to [[nodiscard]], warn_unused_result attempts to force users harder
not to ignore the return value.  So, it doesn't allow just (void) cast to shut
up the warning, and this foo () ? 0 : 0 or 0 * foo () is similar obfuscation to
ignore the result when whomever that added the warn_unused_result attribute
wanted users not to do that.
So, use [[nodiscard]] instead of you want less aggressive checking, or change
your code, like O_TEXT != 0 ? text_mode () ? O_TEXT : 0 : 0 where you wouldn't
even call the function if you want to ignore the return value.

[Bug c/94389] __attribute__((warn_unused_result)) will warn if the result is discarded as an optimisation

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94389

--- Comment #5 from Segher Boessenkool <segher at gcc dot gnu.org> ---
The language frontend shouldn't do this kind of code transformations, whether
you think the warning should warn or not here, imo.

[Bug c/94389] __attribute__((warn_unused_result)) will warn if the result is discarded as an optimisation

[felix.von.s at posteo dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94389

--- Comment #6 from felix <felix.von.s at posteo dot de> ---
I don’t mind the transformation being applied. I think it is entirely sound and
may be beneficial; I imagine it may be harder to allocate registers for the
naïve translation of (foo() ? X : X) than it is for (foo(), X). It’s just the
warning that is wrong.

> So, use [[nodiscard]] instead

This is filed against the C frontend, where [[nodiscard]] has not been
implemented. Not even on trunk (as provided today by godbolt.org, at least).
You are asking people to use a feature that does not exist.

Even if [[nodiscard]] is going to be added eventually, most code in the wild
uses __attribute__((warn_unused_result)) and will do so for a while. Which
means it will still be subject to false positives like this one.

Moreover, the attribute often appears in library headers, which the library
user is not really at liberty to change. Are you suggesting that everyone
compile their code with -Dwarn_unused_result=nodiscard
-D__warn_unused_result__=nodiscard passed on the command line? Will that even
work?

Maybe you should reflect for a while on why the C++ and C committees considered
it more useful to standardise [[nodiscard]] with the semantics they did,
instead of the semantics of GCC’s __attribute__((warn_unused_result)) as of
today.

> O_TEXT != 0 ? text_mode () ? O_TEXT : 0 : 0

First of all, that is not equivalent:

    int text_mode(void) {
        if (mode == MODE_TEXT) {
            fprintf(log_file, "text mode selected\n");
            return 1;
        } else {
            fprintf(log_file, "binary mode selected\n");
            return 0;
        }
    }

Maybe I care about that log message, even in the case where the return value
itself happens to be ultimately irrelevant.

Second of all, I should not be faulted for not adding redundant special cases
to my already correct code. I am not going to add convoluted hacks just to
avoid an asinine compiler warning. I do not want to hand-optimise
platform-specific cases like this either; taking care of platform-specific
serendipities that can be exploited for optimisation purposes is the compiler’s
job.

And finally, just because a faulty workaround exists doesn’t make the warning
not wrong. If GCC cannot tell a legitimate use from ‘obfuscation’, it should
err on the side of the former. (I believe a case could be made for multiplying
by zero as well.)

Though I think it would be simpler to just make the semantics of
__attribute__((warn_unused_result)) be exactly like [[nodiscard]].

[Bug c/94389] __attribute__((warn_unused_result)) will warn if the result is discarded as an optimisation

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94389

--- Comment #7 from Segher Boessenkool <segher at gcc dot gnu.org> ---
(In reply to felix from comment #6)
> I don’t mind the transformation being applied.

That is not what I said.  I said the **language frontend** should not
do this.  A language frontend should give an as faithful as possible
description of the source code to the rest of the compiler.