Preprocessor: support defined() with operator names for MS compatibility

Also flesh out missing tests, improve diagnostic QOI and fix a couple of corner
cases found in the process.

Fixes PR10606.

llvm-svn: 209276

9 files changed, 94 insertions, 59 deletions

diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td
index 6240ce0705a..5d3a4be61a3 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -353,7 +353,7 @@ def err_pp_error_opening_file : Error<
 def err_pp_empty_filename : Error<"empty filename">;
 def err_pp_include_too_deep : Error<"#include nested too deeply">;
 def err_pp_expects_filename : Error<"expected \"FILENAME\" or <FILENAME>">;
-def err_pp_macro_not_identifier : Error<"macro names must be identifiers">;
+def err_pp_macro_not_identifier : Error<"macro name must be an identifier">;
 def err_pp_missing_macro_name : Error<"macro name missing">;
 def err_pp_missing_rparen_in_macro_def : Error<
   "missing ')' in macro parameter list">;
@@ -379,8 +379,6 @@ def err_pp_expected_value_in_expr : Error<"expected value in expression">;
 def err_pp_expected_rparen : Error<"expected ')' in preprocessor expression">;
 def err_pp_expected_eol : Error<
   "expected end of line in preprocessor expression">;
-def err_pp_defined_requires_identifier : Error<
-  "operator 'defined' requires an identifier">;
 def err_pp_expected_after : Error<"missing %1 after %0">;
 def err_pp_colon_without_question : Error<"':' without preceding '?'">;
 def err_pp_division_by_zero : Error<
diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index ec5c9756168..d4b4ba24696 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -1343,6 +1343,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, char isDefineUndef);
+
 private:
 
   void PushIncludeMacroStack() {
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index ba213a878ad..5f80f4487fb 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -129,70 +129,78 @@ void Preprocessor::DiscardUntilEndOfDirective() {
   } while (Tmp.isNot(tok::eod));
 }
 
-/// \brief Lex and validate a macro name, which occurs after a
-/// \#define or \#undef.
-///
-/// This sets the token kind to eod and discards the rest
-/// of the macro line if the macro name is invalid.  \p isDefineUndef is 1 if
-/// this is due to a a \#define, 2 if \#undef directive, 0 if it is something
-/// else (e.g. \#ifdef).
-void Preprocessor::ReadMacroName(Token &MacroNameTok, char isDefineUndef) {
-  // Read the token, don't allow macro expansion on it.
-  LexUnexpandedToken(MacroNameTok);
-
-  if (MacroNameTok.is(tok::code_completion)) {
-    if (CodeComplete)
-      CodeComplete->CodeCompleteMacroName(isDefineUndef == 1);
-    setCodeCompletionReached();
-    LexUnexpandedToken(MacroNameTok);
-  }
-  
+bool Preprocessor::CheckMacroName(Token &MacroNameTok, char isDefineUndef) {
   // Missing macro name?
-  if (MacroNameTok.is(tok::eod)) {
-    Diag(MacroNameTok, diag::err_pp_missing_macro_name);
-    return;
-  }
+  if (MacroNameTok.is(tok::eod))
+    return Diag(MacroNameTok, diag::err_pp_missing_macro_name);
 
   IdentifierInfo *II = MacroNameTok.getIdentifierInfo();
   if (!II) {
     bool Invalid = false;
     std::string Spelling = getSpelling(MacroNameTok, &Invalid);
     if (Invalid)
-      return;
+      return Diag(MacroNameTok, diag::err_pp_macro_not_identifier);
 
     const IdentifierInfo &Info = Identifiers.get(Spelling);
 
     // Allow #defining |and| and friends in microsoft mode.
     if (Info.isCPlusPlusOperatorKeyword() && getLangOpts().MSVCCompat) {
       MacroNameTok.setIdentifierInfo(getIdentifierInfo(Spelling));
-      return;
+      return false;
     }
 
     if (Info.isCPlusPlusOperatorKeyword())
       // C++ 2.5p2: Alternative tokens behave the same as its primary token
       // except for their spellings.
-      Diag(MacroNameTok, diag::err_pp_operator_used_as_macro_name) << Spelling;
-    else
-      Diag(MacroNameTok, diag::err_pp_macro_not_identifier);
-    // Fall through on error.
-  } else if (isDefineUndef && II->getPPKeywordID() == tok::pp_defined) {
+      return Diag(MacroNameTok, diag::err_pp_operator_used_as_macro_name)
+             << Spelling;
+
+    return Diag(MacroNameTok, diag::err_pp_macro_not_identifier);
+  }
+
+  if (isDefineUndef && II->getPPKeywordID() == tok::pp_defined) {
     // Error if defining "defined": C99 6.10.8/4, C++ [cpp.predefined]p4.
-    Diag(MacroNameTok, diag::err_defined_macro_name);
-  } else if (isDefineUndef == 2 && II->hasMacroDefinition() &&
-             getMacroInfo(II)->isBuiltinMacro()) {
+    return Diag(MacroNameTok, diag::err_defined_macro_name);
+  }
+
+  if (isDefineUndef == 2 && II->hasMacroDefinition() &&
+      getMacroInfo(II)->isBuiltinMacro()) {
     // Warn if undefining "__LINE__" and other builtins, per C99 6.10.8/4
     // and C++ [cpp.predefined]p4], but allow it as an extension.
     Diag(MacroNameTok, diag::ext_pp_undef_builtin_macro);
-    return;
-  } else {
-    // Okay, we got a good identifier node.  Return it.
-    return;
   }
 
-  // Invalid macro name, read and discard the rest of the line.  Then set the
-  // token kind to tok::eod.
-  MacroNameTok.setKind(tok::eod);
-  return DiscardUntilEndOfDirective();
+  // Okay, we got a good identifier.
+  return false;
+}
+
+/// \brief Lex and validate a macro name, which occurs after a
+/// \#define or \#undef.
+///
+/// This sets the token kind to eod and discards the rest
+/// of the macro line if the macro name is invalid.  \p isDefineUndef is 1 if
+/// this is due to a a \#define, 2 if \#undef directive, 0 if it is something
+/// else (e.g. \#ifdef).
+void Preprocessor::ReadMacroName(Token &MacroNameTok, char isDefineUndef) {
+  // Read the token, don't allow macro expansion on it.
+  LexUnexpandedToken(MacroNameTok);
+
+  if (MacroNameTok.is(tok::code_completion)) {
+    if (CodeComplete)
+      CodeComplete->CodeCompleteMacroName(isDefineUndef == 1);
+    setCodeCompletionReached();
+    LexUnexpandedToken(MacroNameTok);
+  }
+
+  if (!CheckMacroName(MacroNameTok, isDefineUndef))
+    return;
+
+  // Invalid macro name, read and discard the rest of the line and set the
+  // token kind to tok::eod if necessary.
+  if (MacroNameTok.isNot(tok::eod)) {
+    MacroNameTok.setKind(tok::eod);
+    DiscardUntilEndOfDirective();
+  }
 }
 
 /// \brief Ensure that the next token is a tok::eod token.
diff --git a/clang/lib/Lex/PPExpressions.cpp b/clang/lib/Lex/PPExpressions.cpp
index 8be7a54f827..2260bf98f7a 100644
--- a/clang/lib/Lex/PPExpressions.cpp
+++ b/clang/lib/Lex/PPExpressions.cpp
@@ -81,7 +81,6 @@ struct DefinedTracker {
 /// EvaluateDefined - Process a 'defined(sym)' expression.
 static bool EvaluateDefined(PPValue &Result, Token &PeekTok, DefinedTracker &DT,
                             bool ValueLive, Preprocessor &PP) {
-  IdentifierInfo *II;
   SourceLocation beginLoc(PeekTok.getLocation());
   Result.setBegin(beginLoc);
 
@@ -102,14 +101,13 @@ static bool EvaluateDefined(PPValue &Result, Token &PeekTok, DefinedTracker &DT,
     PP.setCodeCompletionReached();
     PP.LexUnexpandedNonComment(PeekTok);
   }
-  
+
   // If we don't have a pp-identifier now, this is an error.
-  if ((II = PeekTok.getIdentifierInfo()) == nullptr) {
-    PP.Diag(PeekTok, diag::err_pp_defined_requires_identifier);
+  if (PP.CheckMacroName(PeekTok, 0))
     return true;
-  }
 
   // Otherwise, we got an identifier, is it defined to something?
+  IdentifierInfo *II = PeekTok.getIdentifierInfo();
   Result.Val = II->hasMacroDefinition();
   Result.Val.setIsUnsigned(false);  // Result is signed intmax_t.
 
diff --git a/clang/test/Parser/cxx11-user-defined-literals.cpp b/clang/test/Parser/cxx11-user-defined-literals.cpp
index a7446529aec..b89a5741865 100644
--- a/clang/test/Parser/cxx11-user-defined-literals.cpp
+++ b/clang/test/Parser/cxx11-user-defined-literals.cpp
@@ -139,7 +139,7 @@ void operator""_\u212e""_\u212f(unsigned long long) {} // expected-error {{diffe
 void operator""_℮""_℮(unsigned long long) {} // expected-note {{previous}}
 void operator""_\u212e""_\u212e(unsigned long long) {} // expected-error {{redefinition}}
 
-#define ¢ *0.01 // expected-error {{macro names must be identifiers}}
+#define ¢ *0.01 // expected-error {{macro name must be an identifier}}
 constexpr int operator""_¢(long double d) { return d * 100; } // expected-error {{non-ASCII}}
 constexpr int operator""_¢(unsigned long long n) { return n; } // expected-error {{non-ASCII}}
 static_assert(0.02_¢ == 2_¢, ""); // expected-error 2{{non-ASCII}}
diff --git a/clang/test/Preprocessor/cxx_oper_keyword.cpp b/clang/test/Preprocessor/cxx_oper_keyword.cpp
index 3fc246dd6c2..5a4653f1a7a 100644
--- a/clang/test/Preprocessor/cxx_oper_keyword.cpp
+++ b/clang/test/Preprocessor/cxx_oper_keyword.cpp
@@ -1,7 +1,22 @@
-// RUN: not %clang_cc1 %s -E
-// RUN: %clang_cc1 %s -E -fno-operator-names
+// RUN: %clang_cc1 %s -E -verify -DOPERATOR_NAMES
+// RUN: %clang_cc1 %s -E -verify -fno-operator-names
 
-// Not valid in C++ unless -fno-operator-names is passed.
-#define and foo
+#ifndef OPERATOR_NAMES
+//expected-error@+3 {{token is not a valid binary operator in a preprocessor subexpression}}
+#endif
+// Valid because 'and' is a spelling of '&&'
+#if defined foo and bar
+#endif
+
+// Not valid in C++ unless -fno-operator-names is passed:
 
+#ifdef OPERATOR_NAMES
+//expected-error@+2 {{C++ operator 'and' cannot be used as a macro name}}
+#endif
+#define and foo
 
+#ifdef OPERATOR_NAMES
+//expected-error@+2 {{C++ operator 'and' cannot be used as a macro name}}
+#endif
+#if defined and
+#endif
diff --git a/clang/test/Preprocessor/cxx_oper_keyword_ms_compat.cpp b/clang/test/Preprocessor/cxx_oper_keyword_ms_compat.cpp
index 1c6ef90c8b8..dcf6908c329 100644
--- a/clang/test/Preprocessor/cxx_oper_keyword_ms_compat.cpp
+++ b/clang/test/Preprocessor/cxx_oper_keyword_ms_compat.cpp
@@ -7,6 +7,13 @@ bool f() {
 #endif
 }
 
+#ifdef and
+#endif
+
+// The second 'and' is a valid C++ operator name for '&&'.
+#if defined and and defined(and)
+#endif
+
 // All c++ keywords should be #define-able in ms mode.
 // (operators like "and" aren't normally, the rest always is.)
 #define and
diff --git a/clang/test/Preprocessor/ifdef-recover.c b/clang/test/Preprocessor/ifdef-recover.c
index 3d652dc604d..a6481359f43 100644
--- a/clang/test/Preprocessor/ifdef-recover.c
+++ b/clang/test/Preprocessor/ifdef-recover.c
@@ -1,15 +1,22 @@
-/* RUN: not %clang_cc1 -E %s 2>&1 >/dev/null | grep error: | count 3
+/* RUN: %clang_cc1 -E -verify %s
  */
 
+/* expected-error@+1 {{macro name missing}} */
 #ifdef
+#endif
+
+/* expected-error@+1 {{macro name must be an identifier}} */
+#ifdef !
+#endif
 
+/* expected-error@+1 {{macro name missing}} */
+#if defined
 #endif
 
-/* End of function-like macro invocation in #ifdef */
 /* PR1936 */
+/* expected-error@+2 {{unterminated function-like macro invocation}} expected-error@+2 {{expected value in expression}} expected-note@+1 {{macro 'f' defined here}} */
 #define f(x) x
 #if f(2
 #endif
 
 int x;
-
diff --git a/clang/test/Preprocessor/ucn-pp-identifier.c b/clang/test/Preprocessor/ucn-pp-identifier.c
index 6936ed92cb5..f045e38e94a 100644
--- a/clang/test/Preprocessor/ucn-pp-identifier.c
+++ b/clang/test/Preprocessor/ucn-pp-identifier.c
@@ -24,9 +24,9 @@
 #endif
 
 // Make sure we reject disallowed UCNs
-#define \ufffe // expected-error {{macro names must be identifiers}}
-#define \U10000000  // expected-error {{macro names must be identifiers}}
-#define \u0061  // expected-error {{character 'a' cannot be specified by a universal character name}} expected-error {{macro names must be identifiers}}
+#define \ufffe // expected-error {{macro name must be an identifier}}
+#define \U10000000  // expected-error {{macro name must be an identifier}}
+#define \u0061  // expected-error {{character 'a' cannot be specified by a universal character name}} expected-error {{macro name must be an identifier}}
 
 // FIXME: Not clear what our behavior should be here; \u0024 is "$".
 #define a\u0024  // expected-warning {{whitespace}}