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

["egallager at gcc dot gnu.org" <gcc-bugzilla@gcc.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.