From: Serge Pavlov Date: Thu, 18 Dec 2014 11:14:21 +0000 (+0000) Subject: Fixed warnings on redefine keywords and reserved ids. X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=07c0f04e08c840fc8bf1bfb8d5a2775b40c6e503;p=platform%2Fupstream%2Fllvm.git Fixed warnings on redefine keywords and reserved ids. Repared support for warnings -Wkeyword-macro and -Wreserved-id-macro. The warning -Wkeyword-macro now is not issued in patterns that are used in configuration scripts: #define inline also for 'const', 'extern' and 'static'. If macro repalcement is identical to macro name, the warning also is not issued: #define volatile volatile And finally if macro replacement is also a keyword identical to the replaced one but decorated with leading/trailing underscores: #define inline __inline #define inline __inline__ #define inline _inline // in MSVC compatibility mode Warning -Wreserved-id-macro is off by default, it could help catching things like: #undef __cplusplus llvm-svn: 224512 --- diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 81efb8e..f684ce9 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -338,6 +338,7 @@ 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 65f9f77..b9461c1 100644 --- a/clang/include/clang/Basic/DiagnosticLexKinds.td +++ b/clang/include/clang/Basic/DiagnosticLexKinds.td @@ -292,6 +292,9 @@ def note_pp_ambiguous_macro_other : Note< "other definition of %0">; def warn_pp_macro_hides_keyword : Extension< "keyword is hidden by macro definition">, InGroup; +def warn_pp_macro_is_reserved_id : Warning< + "macro name is a reserved identifier">, DefaultIgnore, + InGroup; def pp_invalid_string_literal : Warning< "invalid string literal, ignoring final '\\'">; diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h index b6dcaa0..326f519 100644 --- a/clang/include/clang/Lex/Preprocessor.h +++ b/clang/include/clang/Lex/Preprocessor.h @@ -1380,7 +1380,8 @@ public: /// followed by EOD. Return true if the token is not a valid on-off-switch. bool LexOnOffSwitch(tok::OnOffSwitch &OOS); - bool CheckMacroName(Token &MacroNameTok, MacroUse isDefineUndef); + bool CheckMacroName(Token &MacroNameTok, MacroUse isDefineUndef, + bool *ShadowFlag = nullptr); private: @@ -1420,11 +1421,17 @@ private: bool isPublic); /// \brief Lex and validate a macro name, which occurs after a - /// \#define or \#undef. + /// \#define or \#undef. + /// + /// \param MacroNameTok Token that represents the name defined or undefined. + /// \param IsDefineUndef Kind if preprocessor directive. + /// \param ShadowFlag Points to flag that is set if macro name shadows + /// a keyword. /// /// This emits a diagnostic, sets the token kind to eod, /// and discards the rest of the macro line if the macro name is invalid. - void ReadMacroName(Token &MacroNameTok, MacroUse isDefineUndef = MU_Other); + void ReadMacroName(Token &MacroNameTok, MacroUse IsDefineUndef = MU_Other, + bool *ShadowFlag = nullptr); /// The ( starting an argument list of a macro definition has just been read. /// Lex the rest of the arguments and the closing ), updating \p MI with diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index 7add526..8f610b4 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -100,18 +100,56 @@ void Preprocessor::DiscardUntilEndOfDirective() { } while (Tmp.isNot(tok::eod)); } -static bool shouldWarnOnMacroDef(Preprocessor &PP, IdentifierInfo *II) { +/// \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) { 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; if (II->isKeyword(Lang)) - return true; - if (Lang.CPlusPlus && (Text.equals("override") || Text.equals("final"))) - return true; - return false; + return MD_KeywordDef; + if (Lang.CPlusPlus11 && (Text.equals("override") || Text.equals("final"))) + return MD_KeywordDef; + return MD_NoWarn; } -bool Preprocessor::CheckMacroName(Token &MacroNameTok, MacroUse isDefineUndef) { +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; +} + +bool Preprocessor::CheckMacroName(Token &MacroNameTok, MacroUse isDefineUndef, + bool *ShadowFlag) { // Missing macro name? if (MacroNameTok.is(tok::eod)) return Diag(MacroNameTok, diag::err_pp_missing_macro_name); @@ -151,12 +189,28 @@ bool Preprocessor::CheckMacroName(Token &MacroNameTok, MacroUse isDefineUndef) { Diag(MacroNameTok, diag::ext_pp_undef_builtin_macro); } - // Warn if defining/undefining reserved identifier including keywords. + // If defining/undefining reserved identifier or a keyword, we need to issue + // a warning. SourceLocation MacroNameLoc = MacroNameTok.getLocation(); + if (ShadowFlag) + *ShadowFlag = false; if (!SourceMgr.isInSystemHeader(MacroNameLoc) && (strcmp(SourceMgr.getBufferName(MacroNameLoc), "") != 0)) { - if (isDefineUndef == MU_Define && shouldWarnOnMacroDef(*this, II)) - Diag(MacroNameTok, diag::warn_pp_macro_hides_keyword); + 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) { + // We do not want to warn on some patterns widely used in configuration + // scripts. This requires analyzing next tokens, so do not issue warnings + // now, only inform caller. + if (ShadowFlag) + *ShadowFlag = true; + } + if (D == MD_ReservedMacro) + Diag(MacroNameTok, diag::warn_pp_macro_is_reserved_id); } // Okay, we got a good identifier. @@ -170,8 +224,10 @@ bool Preprocessor::CheckMacroName(Token &MacroNameTok, MacroUse isDefineUndef) { /// the macro name is invalid. /// /// \param MacroNameTok Token that is expected to be a macro name. -/// \papam isDefineUndef Context in which macro is used. -void Preprocessor::ReadMacroName(Token &MacroNameTok, MacroUse isDefineUndef) { +/// \param isDefineUndef Context in which macro is used. +/// \param ShadowFlag Points to a flag that is set if macro shadows a keyword. +void Preprocessor::ReadMacroName(Token &MacroNameTok, MacroUse isDefineUndef, + bool *ShadowFlag) { // Read the token, don't allow macro expansion on it. LexUnexpandedToken(MacroNameTok); @@ -182,7 +238,7 @@ void Preprocessor::ReadMacroName(Token &MacroNameTok, MacroUse isDefineUndef) { LexUnexpandedToken(MacroNameTok); } - if (!CheckMacroName(MacroNameTok, isDefineUndef)) + if (!CheckMacroName(MacroNameTok, isDefineUndef, ShadowFlag)) return; // Invalid macro name, read and discard the rest of the line and set the @@ -1918,6 +1974,52 @@ bool Preprocessor::ReadMacroDefinitionArgList(MacroInfo *MI, Token &Tok) { } } +static bool isConfigurationPattern(Token &MacroName, MacroInfo *MI, + const LangOptions &LOptions) { + if (MI->getNumTokens() == 1) { + const Token &Value = MI->getReplacementToken(0); + + // Macro that is identity, like '#define inline inline' is a valid pattern. + if (MacroName.getKind() == Value.getKind()) + return true; + + // Macro that maps a keyword to the same keyword decorated with leading/ + // trailing underscores is a valid pattern: + // #define inline __inline + // #define inline __inline__ + // #define inline _inline (in MS compatibility mode) + StringRef MacroText = MacroName.getIdentifierInfo()->getName(); + if (IdentifierInfo *II = Value.getIdentifierInfo()) { + if (!II->isKeyword(LOptions)) + return false; + StringRef ValueText = II->getName(); + StringRef TrimmedValue = ValueText; + if (!ValueText.startswith("__")) { + if (ValueText.startswith("_")) + TrimmedValue = TrimmedValue.drop_front(1); + else + return false; + } else { + TrimmedValue = TrimmedValue.drop_front(2); + if (TrimmedValue.endswith("__")) + TrimmedValue = TrimmedValue.drop_back(2); + } + return TrimmedValue.equals(MacroText); + } else { + return false; + } + } + + // #define inline + if ((MacroName.is(tok::kw_extern) || MacroName.is(tok::kw_inline) || + MacroName.is(tok::kw_static) || MacroName.is(tok::kw_const)) && + MI->getNumTokens() == 0) { + return true; + } + + return false; +} + /// HandleDefineDirective - Implements \#define. This consumes the entire macro /// line then lets the caller lex the next real token. void Preprocessor::HandleDefineDirective(Token &DefineTok, @@ -1925,7 +2027,8 @@ void Preprocessor::HandleDefineDirective(Token &DefineTok, ++NumDefined; Token MacroNameTok; - ReadMacroName(MacroNameTok, MU_Define); + bool MacroShadowsKeyword; + ReadMacroName(MacroNameTok, MU_Define, &MacroShadowsKeyword); // Error reading macro name? If so, diagnostic already issued. if (MacroNameTok.is(tok::eod)) @@ -2102,6 +2205,10 @@ void Preprocessor::HandleDefineDirective(Token &DefineTok, } } + if (MacroShadowsKeyword && + !isConfigurationPattern(MacroNameTok, MI, getLangOpts())) { + Diag(MacroNameTok, diag::warn_pp_macro_hides_keyword); + } // Disable __VA_ARGS__ again. Ident__VA_ARGS__->setIsPoisoned(true); diff --git a/clang/test/Preprocessor/macro-reserved-cxx11.cpp b/clang/test/Preprocessor/macro-reserved-cxx11.cpp new file mode 100644 index 0000000..a740ff6 --- /dev/null +++ b/clang/test/Preprocessor/macro-reserved-cxx11.cpp @@ -0,0 +1,7 @@ +// RUN: %clang_cc1 -fsyntax-only -std=c++11 -pedantic -verify %s + +#define for 0 // expected-warning {{keyword is hidden by macro definition}} +#define final 1 // expected-warning {{keyword is hidden by macro definition}} +#define override // expected-warning {{keyword is hidden by macro definition}} + +int x; diff --git a/clang/test/Preprocessor/macro-reserved-ms.c b/clang/test/Preprocessor/macro-reserved-ms.c new file mode 100644 index 0000000..c533ee3 --- /dev/null +++ b/clang/test/Preprocessor/macro-reserved-ms.c @@ -0,0 +1,7 @@ +// RUN: %clang_cc1 -fsyntax-only -fms-extensions -verify %s +// expected-no-diagnostics + +#define inline _inline +#undef inline + +int x; diff --git a/clang/test/Preprocessor/macro-reserved.c b/clang/test/Preprocessor/macro-reserved.c index a13a37f..84b9262 100644 --- a/clang/test/Preprocessor/macro-reserved.c +++ b/clang/test/Preprocessor/macro-reserved.c @@ -1,25 +1,64 @@ -// RUN: %clang_cc1 -fsyntax-only %s -verify - -#pragma clang diagnostic warning "-Wkeyword-macro" +// RUN: %clang_cc1 -fsyntax-only -pedantic -verify %s #define for 0 // expected-warning {{keyword is hidden by macro definition}} #define final 1 #define __HAVE_X 0 +#define __cplusplus #define _HAVE_X 0 #define X__Y +#undef for +#undef final +#undef __HAVE_X #undef __cplusplus #undef _HAVE_X #undef X__Y +// whitelisted definitions +#define while while +#define const +#define static +#define extern +#define inline + +#undef while +#undef const +#undef static +#undef extern +#undef inline + +#define inline __inline +#undef inline +#define inline __inline__ +#undef inline + +#define inline inline__ // expected-warning {{keyword is hidden by macro definition}} +#undef inline +#define extern __inline // expected-warning {{keyword is hidden by macro definition}} +#undef extern +#define extern __extern // expected-warning {{keyword is hidden by macro definition}} +#undef extern +#define extern __extern__ // expected-warning {{keyword is hidden by macro definition}} +#undef extern + +#define inline _inline // expected-warning {{keyword is hidden by macro definition}} +#undef inline +#define volatile // expected-warning {{keyword is hidden by macro definition}} +#undef volatile + +#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 -#define _HAVE_X 0 +#define __clusplus // expected-warning {{macro name is a reserved identifier}} +#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 -#undef __cplusplus -#undef _HAVE_X +#undef switch +#undef final +#undef __cplusplus // expected-warning {{macro name is a reserved identifier}} +#undef _HAVE_X // expected-warning {{macro name is a reserved identifier}} #undef X__Y int x; diff --git a/clang/test/Preprocessor/macro-reserved.cpp b/clang/test/Preprocessor/macro-reserved.cpp index 087c27e..ba1594a 100644 --- a/clang/test/Preprocessor/macro-reserved.cpp +++ b/clang/test/Preprocessor/macro-reserved.cpp @@ -1,25 +1,63 @@ -// RUN: %clang_cc1 -fsyntax-only %s -verify - -#pragma clang diagnostic warning "-Wkeyword-macro" +// RUN: %clang_cc1 -fsyntax-only -verify -pedantic %s #define for 0 // expected-warning {{keyword is hidden by macro definition}} -#define final 1 // expected-warning {{keyword is hidden by macro definition}} +#define final 1 #define __HAVE_X 0 #define _HAVE_X 0 #define X__Y -#undef __cplusplus +#undef for +#undef final +#undef __HAVE_X #undef _HAVE_X #undef X__Y +#undef __cplusplus +#define __cplusplus + +// whitelisted definitions +#define while while +#define const +#define static +#define extern +#define inline + +#undef while +#undef const +#undef static +#undef extern +#undef inline + +#define inline __inline +#undef inline +#define inline __inline__ +#undef inline + +#define inline inline__ // expected-warning {{keyword is hidden by macro definition}} +#undef inline +#define extern __inline // expected-warning {{keyword is hidden by macro definition}} +#undef extern +#define extern __extern // expected-warning {{keyword is hidden by macro definition}} +#undef extern +#define extern __extern__ // expected-warning {{keyword is hidden by macro definition}} +#undef extern + +#define inline _inline // expected-warning {{keyword is hidden by macro definition}} +#undef inline +#define volatile // expected-warning {{keyword is hidden by macro definition}} +#undef volatile + + +#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 -#define _HAVE_X 0 -#define X__Y +#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 X__Y // expected-warning {{macro name is a reserved identifier}} -#undef __cplusplus -#undef _HAVE_X -#undef 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}} int x;