From 92c14bb2ff85d16785140c93493d97060ffcf2e7 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 16 Dec 2014 21:16:10 +0000 Subject: [PATCH] Move -Wkeyword-macro into -pedantic, remove -Wreserved-id-macro. As discussed on the post-commit review thread for r224012, -Wkeyword-macro fires mostly on headers trying to set up portable defines and doesn't find much bad stuff in practice. But [macro.names]p2 does disallow defining or undefining keywords, override and final, and alignas, so keep the warning but move it into -pedantic. -Wreserved-id-macro warns on #define __need_size_t which is more or less public api for glibc headers. Since this warning isn't motivated by a standard, remove it. (See also r223114 for a previous follow-up to r224012.) llvm-svn: 224371 --- clang/include/clang/Basic/DiagnosticGroups.td | 1 - clang/include/clang/Basic/DiagnosticLexKinds.td | 8 +--- clang/lib/Lex/PPDirectives.cpp | 55 +++---------------------- clang/test/Preprocessor/macro-reserved.c | 12 +++--- clang/test/Preprocessor/macro-reserved.cpp | 16 +++---- 5 files changed, 21 insertions(+), 71 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 34609b2..65ff044 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -338,7 +338,6 @@ def : DiagGroup<"sequence-point", [Unsequenced]>; // Preprocessor warnings. def AmbiguousMacro : DiagGroup<"ambiguous-macro">; def KeywordAsMacro : DiagGroup<"keyword-macro">; -def ReservedIdAsMacro : DiagGroup<"reserved-id-macro">; // Just silence warnings about -Wstrict-aliasing for now. def : DiagGroup<"strict-aliasing=0">; diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td index 325ab0e..65f9f77 100644 --- a/clang/include/clang/Basic/DiagnosticLexKinds.td +++ b/clang/include/clang/Basic/DiagnosticLexKinds.td @@ -290,12 +290,8 @@ def note_pp_ambiguous_macro_chosen : Note< "expanding this definition of %0">; def note_pp_ambiguous_macro_other : Note< "other definition of %0">; -def warn_pp_macro_hides_keyword : Warning< - "keyword is hidden by macro definition">, - InGroup; -def warn_pp_macro_is_reserved_id : Warning< - "macro name is a reserved identifier">, DefaultIgnore, - InGroup; +def warn_pp_macro_hides_keyword : Extension< + "keyword is hidden by macro definition">, InGroup; def pp_invalid_string_literal : Warning< "invalid string literal, ignoring final '\\'">; diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index e359ea5..7add526 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -100,53 +100,15 @@ void Preprocessor::DiscardUntilEndOfDirective() { } while (Tmp.isNot(tok::eod)); } -/// \brief Enumerates possible cases of #define/#undef a reserved identifier. -enum MacroDiag { - MD_NoWarn, //> Not a reserved identifier - MD_KeywordDef, //> Macro hides keyword, enabled by default - MD_ReservedMacro //> #define of #undef reserved id, disabled by default -}; - -/// \brief Checks if the specified identifier is reserved in the specified -/// language. -/// This function does not check if the identifier is a keyword. -static bool isReservedId(StringRef Text, const LangOptions &Lang) { - // C++ [macro.names], C11 7.1.3: - // All identifiers that begin with an underscore and either an uppercase - // letter or another underscore are always reserved for any use. - if (Text.size() >= 2 && Text[0] == '_' && - (isUppercase(Text[1]) || Text[1] == '_')) - return true; - // C++ [global.names] - // Each name that contains a double underscore ... is reserved to the - // implementation for any use. - if (Lang.CPlusPlus) { - if (Text.find("__") != StringRef::npos) - return true; - } - return false; -} - -static MacroDiag shouldWarnOnMacroDef(Preprocessor &PP, IdentifierInfo *II) { +static bool shouldWarnOnMacroDef(Preprocessor &PP, IdentifierInfo *II) { const LangOptions &Lang = PP.getLangOpts(); StringRef Text = II->getName(); - if (isReservedId(Text, Lang)) - return MD_ReservedMacro; // Do not warn on keyword undef. It is generally harmless and widely used. if (II->isKeyword(Lang)) - return MD_KeywordDef; + return true; if (Lang.CPlusPlus && (Text.equals("override") || Text.equals("final"))) - return MD_KeywordDef; - return MD_NoWarn; -} - -static MacroDiag shouldWarnOnMacroUndef(Preprocessor &PP, IdentifierInfo *II) { - const LangOptions &Lang = PP.getLangOpts(); - StringRef Text = II->getName(); - // Do not warn on keyword undef. It is generally harmless and widely used. - if (isReservedId(Text, Lang)) - return MD_ReservedMacro; - return MD_NoWarn; + return true; + return false; } bool Preprocessor::CheckMacroName(Token &MacroNameTok, MacroUse isDefineUndef) { @@ -193,15 +155,8 @@ bool Preprocessor::CheckMacroName(Token &MacroNameTok, MacroUse isDefineUndef) { SourceLocation MacroNameLoc = MacroNameTok.getLocation(); if (!SourceMgr.isInSystemHeader(MacroNameLoc) && (strcmp(SourceMgr.getBufferName(MacroNameLoc), "") != 0)) { - MacroDiag D = MD_NoWarn; - if (isDefineUndef == MU_Define) - D = shouldWarnOnMacroDef(*this, II); - else if (isDefineUndef == MU_Undef) - D = shouldWarnOnMacroUndef(*this, II); - if (D == MD_KeywordDef) + if (isDefineUndef == MU_Define && shouldWarnOnMacroDef(*this, II)) Diag(MacroNameTok, diag::warn_pp_macro_hides_keyword); - if (D == MD_ReservedMacro) - Diag(MacroNameTok, diag::warn_pp_macro_is_reserved_id); } // Okay, we got a good identifier. diff --git a/clang/test/Preprocessor/macro-reserved.c b/clang/test/Preprocessor/macro-reserved.c index 2a0e26f..a13a37f 100644 --- a/clang/test/Preprocessor/macro-reserved.c +++ b/clang/test/Preprocessor/macro-reserved.c @@ -1,5 +1,7 @@ // RUN: %clang_cc1 -fsyntax-only %s -verify +#pragma clang diagnostic warning "-Wkeyword-macro" + #define for 0 // expected-warning {{keyword is hidden by macro definition}} #define final 1 #define __HAVE_X 0 @@ -10,16 +12,14 @@ #undef _HAVE_X #undef X__Y -#pragma clang diagnostic warning "-Wreserved-id-macro" - #define switch if // expected-warning {{keyword is hidden by macro definition}} #define final 1 -#define __HAVE_X 0 // expected-warning {{macro name is a reserved identifier}} -#define _HAVE_X 0 // expected-warning {{macro name is a reserved identifier}} +#define __HAVE_X 0 +#define _HAVE_X 0 #define X__Y -#undef __cplusplus // expected-warning {{macro name is a reserved identifier}} -#undef _HAVE_X // expected-warning {{macro name is a reserved identifier}} +#undef __cplusplus +#undef _HAVE_X #undef X__Y int x; diff --git a/clang/test/Preprocessor/macro-reserved.cpp b/clang/test/Preprocessor/macro-reserved.cpp index 8529ddf..087c27e 100644 --- a/clang/test/Preprocessor/macro-reserved.cpp +++ b/clang/test/Preprocessor/macro-reserved.cpp @@ -1,5 +1,7 @@ // RUN: %clang_cc1 -fsyntax-only %s -verify +#pragma clang diagnostic warning "-Wkeyword-macro" + #define for 0 // expected-warning {{keyword is hidden by macro definition}} #define final 1 // expected-warning {{keyword is hidden by macro definition}} #define __HAVE_X 0 @@ -10,16 +12,14 @@ #undef _HAVE_X #undef X__Y -#pragma clang diagnostic warning "-Wreserved-id-macro" - #define switch if // expected-warning {{keyword is hidden by macro definition}} #define final 1 // expected-warning {{keyword is hidden by macro definition}} -#define __HAVE_X 0 // expected-warning {{macro name is a reserved identifier}} -#define _HAVE_X 0 // expected-warning {{macro name is a reserved identifier}} -#define X__Y // expected-warning {{macro name is a reserved identifier}} +#define __HAVE_X 0 +#define _HAVE_X 0 +#define X__Y -#undef __cplusplus // expected-warning {{macro name is a reserved identifier}} -#undef _HAVE_X // expected-warning {{macro name is a reserved identifier}} -#undef X__Y // expected-warning {{macro name is a reserved identifier}} +#undef __cplusplus +#undef _HAVE_X +#undef X__Y int x; -- 2.7.4