r173952 - [preprocessor] Don't warn about "disabled expansion of recursive macro"

[Douglas Gregor]

On Jan 30, 2013, at 1:23 PM, Richard Smith <richard at metafoo.co.uk> wrote:

> On Wed, Jan 30, 2013 at 12:44 PM, Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
>> On Jan 30, 2013, at 11:54 AM, Abramo Bagnara <abramo.bagnara at gmail.com> wrote:
>> 
>>> Il 30/01/2013 19:55, Argyrios Kyrtzidis ha scritto:
>>>> Author: akirtzidis
>>>> Date: Wed Jan 30 12:55:52 2013
>>>> New Revision: 173952
>>>> 
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=173952&view=rev
>>>> Log:
>>>> [preprocessor] Don't warn about "disabled expansion of recursive macro"
>>>> for "#define X X".
>>>> 
>>>> This is a pattern that, for example, stdbool.h uses.
>>>> rdar://12435773
>>> 
>>> Please revert this commit: this has already been discussed in detail in
>>> past and we decided otherwise. Last time it has been discussed in this
>>> thread:
>>> 
>>> http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20121029/067098.html
>> 
>> Reverted in r173970.
>> 
>>> 
>>>> 
>>>> Modified:
>>>>   cfe/trunk/lib/Lex/PPMacroExpansion.cpp
>>>>   cfe/trunk/test/Headers/stdbool.cpp
>>>>   cfe/trunk/test/Preprocessor/warn-disabled-macro-expansion.c
>>>> 
>>>> Modified: cfe/trunk/lib/Lex/PPMacroExpansion.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPMacroExpansion.cpp?rev=173952&r1=173951&r2=173952&view=diff
>>>> ==============================================================================
>>>> --- cfe/trunk/lib/Lex/PPMacroExpansion.cpp (original)
>>>> +++ cfe/trunk/lib/Lex/PPMacroExpansion.cpp Wed Jan 30 12:55:52 2013
>>>> @@ -459,7 +459,10 @@ bool Preprocessor::HandleMacroExpandedId
>>>>      if (MacroInfo *NewMI = getMacroInfo(NewII))
>>>>        if (!NewMI->isEnabled() || NewMI == MI) {
>>>>          Identifier.setFlag(Token::DisableExpand);
>>>> -          Diag(Identifier, diag::pp_disabled_macro_expansion);
>>>> +          // Don't warn for "#define X X" like "#define bool bool" from
>>>> +          // stdbool.h.
>>> 
>>> Would you be so kind to add a comment here to remember that the choice
>>> to have this warning for thar case (although disabled by default) is
>>> deliberate?
>> 
>> Just to make sure, are you of the opinion that we should warn when <stdbool.h> is included and its macros are used ?
> 
> As discussed on that thread, we should modify <stdbool.h> to suppress
> the warning.

How do you propose to do that? #pragmas don't help, because the diagnostic comes from the expansion location, not the definition location. Changing #define bool bool to something that involves _Pragma is very likely to break code elsewhere.

None of Abramo's arguments apply to

	#define bool bool

The requirements Abramo refers to are meant to deal with unintentionally complicated macros. This is not such a macro: it's a device for making sure that some name is a macro and ensuring that other code doesn't accidentally #define it to something else. We should not be warning on this macro.

	- Doug