[Bug c/105369] New: Improved diagnostics for code from statement expressions documentation [C component]

[Bug c/105369] New: Improved diagnostics for code from statement expressions documentation [C component]

[egallager at gcc dot gnu.org]

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

            Bug ID: 105369
           Summary: Improved diagnostics for code from statement
                    expressions documentation [C component]
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Keywords: diagnostic, documentation
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: egallager at gcc dot gnu.org
  Target Milestone: ---

So, I'm going back to reading GCC documentation again; I last left off with the
statement expressions documentation:
https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html#Statement-Exprs

I made this testcase from the plain-C code on that page:

$ cat stmt_exprs.c
int foo(void);
extern int global_var;

int compound_stmt(void)
{
    global_var++;
    return ({ int y = foo(); int z;
        if (y > 0) z = y;
        else z = -y;
        z; });
}

#define max_bad(a,b) ((a) > (b) ? (a) : (b))
#define maxint(a,b) \
  ({int _a = (a), _b = (b); (_a > _b) ? _a : _b; })

int unsafe(void)
{
    /* side effects: */
    return max_bad(foo(), compound_stmt());
}

int shadowing1(void)
{
    int _a = 1, _b = 2, c;
    c = max_bad(_a, _b);
    return c;
}

int shadowing2(void)
{
    int _a = 1, _b = 2, c;
    c = maxint(_a, _b);
    return c;
}

#define maxint3(a, b, c) \
  ({int _a = (a), _b = (b), _c = (c); maxint(maxint(_a, _b), _c); })

int shadowing3(void)
{
    int _a = 1, _b = 2, _c = 3, d;
    d = maxint3(_a, _b, _c);
    return d;
}

void bar1(void);
int bar2(void);
int baz(void);

int jumping(void)
{
a:
    return foo(), (({ bar1(); goto a; 0; }) + bar2()), baz();
}
$

The documentation says that the statement expression in compound_stmt() is a
"valid (though slightly more complex than necessary) expression" (for getting
an absolute value); maybe there could be a warning to just use abs() instead?
It could go under the existing -Wabsolute-value flag. Next, in unsafe(), the
documentation says that that usage of max_bad() "computes either a or b twice,
with bad results if the operand has side effects", which deserves a warning,
IMO; Karen Pease took advantage of this property of C to win the 2014
"Underhanded C Contest": http://www.underhanded-c.org/_page_id_26.html

As for the 3 shadowing functions, there are plenty of warnings from -Wshadow
and -Wunused-variable there already, so they're probably fine, although it
might help to clean up the display of them a bit. In the final function,
jumping(), the documentation says it "calls foo and bar1 and does not call baz
but may or may not call bar2. If bar2 is called, it is called after foo and
before bar1." This behavior may be surprising. However, there is no warning for
code like that with -Wall -Wextra -Wjump-misses-init. Perhaps there should be
one? Anyways, that's it for the plain-C code from that page; I'll open a
separate bug for the C++ if it turns out that that needs one, too.

[Bug c/105369] Improved diagnostics for code from statement expressions documentation [C component]

[egallager at gcc dot gnu.org]

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

--- Comment #1 from Eric Gallager <egallager at gcc dot gnu.org> ---
Oh, I should probably add something to test the sentence that says, "If you
don’t know the type of the operand, you can still do this, but you must use
typeof or __auto_type (see Typeof)," too...

[Bug c/105369] Improved diagnostics for code from statement expressions documentation [C component]

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I don't think there is a way to get a warning for the abs really and maybe not
even wanted.
There should definitely be a cross reference to where the documentation says
inline functions are fast as macros if there is not already one.

[Bug c/105369] Improved diagnostics for code from statement expressions documentation [C component]

[egallager at gcc dot gnu.org]

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

--- Comment #3 from Eric Gallager <egallager at gcc dot gnu.org> ---
(In reply to Eric Gallager from comment #0)
> Anyways, that's it for the plain-C code from that page; I'll open a separate
> bug for the C++ if it turns out that that needs one, too.

OK I opened bug 105370 for that.

(In reply to Andrew Pinski from comment #2)
> I don't think there is a way to get a warning for the abs really and maybe
> not even wanted.

OK so forget that part I guess; how about the rest of it?

[Bug c/105369] Improved diagnostics for code from statement expressions documentation [C component]

[redi at gcc dot gnu.org]

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

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The preprocessor can't tell if an expression has side effects. I have no idea
how easy it would be (or if it's even possible) for the front end to tell that
an expression was duplicated from a single macro argument.

[Bug c/105369] Improved diagnostics for code from statement expressions documentation [C component]

[egallager at gcc dot gnu.org]

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

Eric Gallager <egallager at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=6906

--- Comment #5 from Eric Gallager <egallager at gcc dot gnu.org> ---
oh wait it looks like the "assertion with side effects" part is actually bug
6906; I'm keeping this bug open for the remaining parts, though