Re: C/C++ PATCH to implement -Wmultistatement-macros (PR c/80116)

[Martin Sebor <msebor@gmail.com>]

On 06/08/2017 11:24 AM, David Malcolm wrote:
> On Thu, 2017-06-08 at 18:49 +0200, Marek Polacek wrote:
>> This is the hopefully last incarnation of the patch.  The change from
>> the
>> last time[0] is simpy that I've added a new test and the warning has
>> been
>> renamed to -Wmultistatement-macros.
>>
>> David - any another comments?
>
> Thanks for working on this; looks useful.
>
> The new name is more accurate, but is rather long; oh well.  As part of
> -Wall, users won't typically have to type it, so that's OK.
>
> [...]
>
>> diff --git gcc/c-family/c-warn.c gcc/c-family/c-warn.c
>> index 35321a6..d883330 100644
>> --- gcc/c-family/c-warn.c
>> +++ gcc/c-family/c-warn.c
> [...]
>
>> +  if (warning_at (body_loc, OPT_Wmultistatement_macros,
>> +		  "macro expands to multiple statements"))
>> +    inform (guard_loc, "some parts of macro expansion are not
>> guarded by "
>> +	    "this conditional");
>
> Is the guard necessarily a "conditional"?  I take a "conditional" to
> mean an "if"; the guard could be a "for" or a "while" (or an "else",
> which still seems something of a stretch to me to call a
> "conditional").
>
> Suggestion: word "this conditional" as "this %qs clause" and either (a)
> rework the code in c-indentation.c's guard_tinfo_to_string so that it's
> shared between these two warnings (i.e. to go from a RID_ to a const
> char *), or (b) just pass in a const char * identifying the guard
> clause token.
...
>
> Likewise; is "conditional" the right word here?  Also, whether of not
> the statements are actually "in" the body of the guard is the issue
> here.
>
> How about:
>
> "Warn about unsafe multiple statement macros that appear to be guarded
> by a clause such as if, else, while, or for, in which only the first
> statement is actually guarded after the macro is expanded."
>
> or somesuch?

FWIW, I agree with David that "conditional" isn't entirely accurate.

At the same time, referring to any of do, for, if, or switch as
clauses isn't quite precise either(*).  In the C language they are
the names of selection and iteration statements, and what follows
is called the controlling expression (with for being special) and
the next thing is a substatement.  I think many people will
informally call the two a condition or conditional and the body.

I don't have strong feelings about the current wording but if it
should be tweaked for accuracy I would suggest to use the formal
term "controlling expression", similarly to -Wswitch-unreachable.

Martin

PS [*] To be completely pedantic, the word clause in the C and
C++ standards has a precise meaning: it refers to a chapter of
the text (such as Scope, Conformance, Language, etc.)