On 06/08/2017 12:19 PM, Marek Polacek wrote:
> On Thu, Jun 08, 2017 at 12:01:03PM +0100, Pedro Alves wrote:
>> On 06/08/2017 11:29 AM, Marek Polacek wrote:
>>> On Wed, Jun 07, 2017 at 08:02:42PM +0100, Pedro Alves wrote:
>>>> Hi Marek,
>>>>
>>>> Nice warning! Just to confirm, would the patch warn with code like:
>>>
>>> Thanks. BTW, if you (or anyone) could come up with a better name,
>>> I'm all ears.
>>
>> AFAICS, the warning's intent is catching the case of a
>> a macro expanding to multiple (top level) statements, not lines.
>
> True. I felt that it was implicitly understood what's meant by that,
> but I'll change that. Martin pointed this out, too.
I don't think it's implicit, because it could raise questions, like:
>> +Wmultiline-expansion
>> +C ObjC C++ ObjC++ Var(warn_multiline_expansion) Warning LangEnabledBy(C
>> ObjC C++ ObjC++,Wall)
>> +Warn about macros expanding to multiple statements in a body of a
>> conditional such as if, else, while, or for.
"
Hmm, the description doesn't actually describe what is
meant by "multiple lines". Does "multiline" here mean that
the warning triggers with:
#define FOO() \
foo1(); \
foo2()
but not
#define FOO() foo1(); foo2()
?
"
It's perhaps obvious to seasoned hackers that that's not the
intention, but not all users are experts.
So a general rule I try to follow (in GDB) is: make sure that the
description of an option matches the option's name.
I.e., if the words used to name the option name don't appear in the
option's description, then that's a good indication something
is off and is bound to confuse someone.
>>
>> So it'd seem clearer to me if the warning was named around
>> "-Wmulti-statement-something" instead of "-Wmultiline-something"?
>>
>> -Wmulti-statement-expansion
>> -Wmulti-statement-macros
>> -Wmulti-statement-macro
>> -Wmulti-statement-macro-expansion
>
> I think I'll go with -Wmultistatement-expansion (without the dash).
Fine with me, FWIW.
> I'll post a new version with the warning renamed and the new test added.
Great, thanks much!
Thanks,
Pedro Alves
