On Jan 30, 2013, at 2:21 PM, Richard Smith <[email protected]> wrote:
> On Wed, Jan 30, 2013 at 2:16 PM, Richard Smith <[email protected]> wrote: >> On Wed, Jan 30, 2013 at 1:39 PM, Douglas Gregor <[email protected]> wrote: >>> >>> On Jan 30, 2013, at 1:23 PM, Richard Smith <[email protected]> wrote: >>> >>>> On Wed, Jan 30, 2013 at 12:44 PM, Argyrios Kyrtzidis <[email protected]> >>>> wrote: >>>>> On Jan 30, 2013, at 11:54 AM, Abramo Bagnara <[email protected]> >>>>> 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. >> >> To my reading of C11 6.10.3.4, that's a bug in Clang's preprocessor. >> _Pragmas are supposed to take effect (and, per 6.10.9/1, be removed >> from the token stream) during macro rescanning, so I think this should >> work: >> >> #define true _Pragma("clang diagnostic push") _Pragma("clang >> diagnostic ignored \"-Wdisabled-macro-expansion\"") true >> _Pragma("clang diagnostic pop") >> >> #define CAT2(x,y) x##y >> #define CAT(x,y) CAT2(x,y) >> bool b = CAT(true,n); >> >> ... because the pragma should take effect during the rescan of the >> expansion of 'true', before we perform the token paste. >> >> Another example we seem to get wrong: >> >> #define foo _Pragma("GCC diagnostic ignored \"-Wuninitialized\"") >> #define str(x) #x >> #define STR(x) str(x) >> const char *p = STR(foo); >> int n() { int k; return k; } >> >> This should produce an empty string and no warning, as far as I can tell. > > FWIW, gcc gets this right, producing an empty string and suppressing > the warning here: > > #define foo _Pragma("GCC diagnostic ignored \"-Wuninitialized\"") > #define str(x) #x > #define STR(x) str(x) > void f() { > const char *p = ({ > STR(foo); > }); > } > int n() { int k; return k; } > > (extra gunk to get around gcc's restrictions on where pragmas can appear). Yes, we should fix this. That's still a horrible workaround for a warning that doesn't make sense. #define X X is *not* what this warning is supposed to diagnose. I've reinstated Argyrios' patch in r173991 with a fix to only suppress the warning in this case. - Doug _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
