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

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">;

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<KeywordAsMacro>;
+def warn_pp_macro_is_reserved_id : Warning<
+  "macro name is a reserved identifier">, DefaultIgnore,
+  InGroup<ReservedIdAsMacro>;
 
 def pp_invalid_string_literal : Warning<
   "invalid string literal, ignoring final '\\'">;

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

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.
-  const LangOptions &Lang = PP.getLangOpts();
+enum MacroDiag {
-  StringRef Text = II->getName();
+  MD_NoWarn,        //> Not a reserved identifier
-  // Do not warn on keyword undef.  It is generally harmless and widely used.
+  MD_KeywordDef,    //> Macro hides keyword, enabled by default
-  if (II->isKeyword(Lang))
+  MD_ReservedMacro  //> #define of #undef reserved id, disabled by default
-    return true;
+};
-  if (Lang.CPlusPlus && (Text.equals("override") || Text.equals("final")))
+
-    return true;
+/// \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;
 }
 
-bool Preprocessor::CheckMacroName(Token &MacroNameTok, MacroUse isDefineUndef) {
+static MacroDiag shouldWarnOnMacroDef(Preprocessor &PP, IdentifierInfo *II) {
+  const LangOptions &Lang = PP.getLangOpts();
+  StringRef Text = II->getName();
+  if (isReservedId(Text, Lang))
+    return MD_ReservedMacro;
+  if (II->isKeyword(Lang))
+    return MD_KeywordDef;
+  if (Lang.CPlusPlus11 && (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;
+}
+
+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), "<built-in>") != 0)) {
-    if (isDefineUndef == MU_Define && shouldWarnOnMacroDef(*this, II))
+    MacroDiag D = MD_NoWarn;
-      Diag(MacroNameTok, diag::warn_pp_macro_hides_keyword);
+    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.
+/// \param isDefineUndef Context in which macro is used.
-void Preprocessor::ReadMacroName(Token &MacroNameTok, MacroUse isDefineUndef) {
+/// \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);

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;

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;

clang/test/Preprocessor/macro-reserved.c

@@ -1,25 +1,64 @@
-// RUN: %clang_cc1 -fsyntax-only %s -verify
+// RUN: %clang_cc1 -fsyntax-only -pedantic -verify %s
-
-#pragma clang diagnostic warning "-Wkeyword-macro"
 
 #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 __clusplus // expected-warning {{macro name is a reserved identifier}}
-#define _HAVE_X 0
+#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 switch
-#undef _HAVE_X
+#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;

clang/test/Preprocessor/macro-reserved.cpp

@@ -1,25 +1,63 @@
-// RUN: %clang_cc1 -fsyntax-only %s -verify
+// RUN: %clang_cc1 -fsyntax-only -verify -pedantic %s
-
-#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 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 final 1
-#define __HAVE_X 0
+#define __HAVE_X 0 // expected-warning {{macro name is a reserved identifier}}
-#define _HAVE_X 0
+#define _HAVE_X 0  // expected-warning {{macro name is a reserved identifier}}
-#define X__Y
+#define X__Y       // expected-warning {{macro name is a reserved identifier}}
 
-#undef __cplusplus
+#undef __cplusplus // expected-warning {{macro name is a reserved identifier}}
-#undef _HAVE_X
+#undef _HAVE_X     // expected-warning {{macro name is a reserved identifier}}
-#undef X__Y
+#undef X__Y        // expected-warning {{macro name is a reserved identifier}}
 
 int x;