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.)
