Permit __VA_OPT__ in all language modes and allow it to be detected with #ifdef.

These changes are intended to give code a path to move away from the GNU
,##__VA_ARGS__ extension, which is non-conforming in some situations and
which we'd like to disable in our conforming mode in those cases.

Author: zygoloid

clang/include/clang/Lex/Preprocessor.h

@@ -447,6 +447,25 @@ class Preprocessor {
           ElseLoc(ElseLoc) {}
   };
 
+  class IfdefMacroNameScopeRAII {
+    Preprocessor &PP;
+    bool VAOPTWasPoisoned;
+
+  public:
+    IfdefMacroNameScopeRAII(Preprocessor &PP)
+        : PP(PP), VAOPTWasPoisoned(PP.Ident__VA_OPT__->isPoisoned()) {
+      PP.Ident__VA_OPT__->setIsPoisoned(false);
+    }
+    IfdefMacroNameScopeRAII(const IfdefMacroNameScopeRAII&) = delete;
+    IfdefMacroNameScopeRAII &operator=(const IfdefMacroNameScopeRAII&) = delete;
+    ~IfdefMacroNameScopeRAII() { Exit(); }
+
+    void Exit() {
+      if (VAOPTWasPoisoned)
+        PP.Ident__VA_OPT__->setIsPoisoned(true);
+    }
+  };
+
 private:
   friend class ASTReader;
   friend class MacroArgs;

clang/include/clang/Lex/VariadicMacroSupport.h

@@ -39,17 +39,14 @@ namespace clang {
       assert(Ident__VA_ARGS__->isPoisoned() && "__VA_ARGS__ should be poisoned "
                                               "outside an ISO C/C++ variadic "
                                               "macro definition!");
-      assert(
-          !Ident__VA_OPT__ ||
-          (Ident__VA_OPT__->isPoisoned() && "__VA_OPT__ should be poisoned!"));
+      assert(Ident__VA_OPT__->isPoisoned() && "__VA_OPT__ should be poisoned!");
     }
 
     /// Client code should call this function just before the Preprocessor is
     /// about to Lex tokens from the definition of a variadic (ISO C/C++) macro.
     void enterScope() {
       Ident__VA_ARGS__->setIsPoisoned(false);
-      if (Ident__VA_OPT__)
-        Ident__VA_OPT__->setIsPoisoned(false);
+      Ident__VA_OPT__->setIsPoisoned(false);
     }
 
     /// Client code should call this function as soon as the Preprocessor has
@@ -58,8 +55,7 @@ namespace clang {
     /// (might be explicitly called, and then reinvoked via the destructor).
     void exitScope() {
       Ident__VA_ARGS__->setIsPoisoned(true);
-      if (Ident__VA_OPT__)
-        Ident__VA_OPT__->setIsPoisoned(true);
+      Ident__VA_OPT__->setIsPoisoned(true);
     }
 
     ~VariadicMacroScopeGuard() { exitScope(); }

clang/lib/Lex/PPDirectives.cpp

@@ -2928,9 +2928,14 @@ void Preprocessor::HandleIfdefDirective(Token &Result,
   ++NumIf;
   Token DirectiveTok = Result;
 
+  // __VA_OPT__ is allowed as the operand of #if[n]def.
+  IfdefMacroNameScopeRAII IfdefMacroNameScope(*this);
+
   Token MacroNameTok;
   ReadMacroName(MacroNameTok);
 
+  IfdefMacroNameScope.Exit();
+
   // Error reading macro name?  If so, diagnostic already issued.
   if (MacroNameTok.is(tok::eod)) {
     // Skip code until we get to #endif.  This helps with recovery by not

clang/lib/Lex/PPExpressions.cpp

@@ -104,6 +104,9 @@ static bool EvaluateDefined(PPValue &Result, Token &PeekTok, DefinedTracker &DT,
   SourceLocation beginLoc(PeekTok.getLocation());
   Result.setBegin(beginLoc);
 
+  // __VA_OPT__ is allowed as the operand of 'defined'.
+  Preprocessor::IfdefMacroNameScopeRAII IfdefMacroNameScope(PP);
+
   // Get the next token, don't expand it.
   PP.LexUnexpandedNonComment(PeekTok);
 
@@ -122,6 +125,8 @@ static bool EvaluateDefined(PPValue &Result, Token &PeekTok, DefinedTracker &DT,
     PP.LexUnexpandedNonComment(PeekTok);
   }
 
+  IfdefMacroNameScope.Exit();
+
   // If we don't have a pp-identifier now, this is an error.
   if (PP.CheckMacroName(PeekTok, MU_Other))
     return true;

clang/lib/Lex/PPMacroExpansion.cpp

@@ -323,13 +323,16 @@ void Preprocessor::dumpMacroInfo(const IdentifierInfo *II) {
 
 /// RegisterBuiltinMacro - Register the specified identifier in the identifier
 /// table and mark it as a builtin macro to be expanded.
-static IdentifierInfo *RegisterBuiltinMacro(Preprocessor &PP, const char *Name){
+static IdentifierInfo *RegisterBuiltinMacro(Preprocessor &PP, const char *Name,
+                                            bool Disabled = false) {
   // Get the identifier.
   IdentifierInfo *Id = PP.getIdentifierInfo(Name);
 
   // Mark it as being a macro that is builtin.
   MacroInfo *MI = PP.AllocateMacroInfo(SourceLocation());
   MI->setIsBuiltinMacro();
+  if (Disabled)
+    MI->DisableMacro();
   PP.appendDefMacroDirective(Id, MI);
   return Id;
 }
@@ -343,6 +346,7 @@ void Preprocessor::RegisterBuiltinMacros() {
   Ident__TIME__ = RegisterBuiltinMacro(*this, "__TIME__");
   Ident__COUNTER__ = RegisterBuiltinMacro(*this, "__COUNTER__");
   Ident_Pragma  = RegisterBuiltinMacro(*this, "_Pragma");
+  Ident__VA_OPT__ = RegisterBuiltinMacro(*this, "__VA_OPT__", true);
 
   // C++ Standing Document Extensions.
   if (getLangOpts().CPlusPlus)

clang/lib/Lex/Preprocessor.cpp

@@ -115,23 +115,20 @@ Preprocessor::Preprocessor(std::shared_ptr<PreprocessorOptions> PPOpts,
 
   BuiltinInfo = std::make_unique<Builtin::Context>();
 
-  // "Poison" __VA_ARGS__, __VA_OPT__ which can only appear in the expansion of
-  // a macro. They get unpoisoned where it is allowed.
-  (Ident__VA_ARGS__ = getIdentifierInfo("__VA_ARGS__"))->setIsPoisoned();
-  SetPoisonReason(Ident__VA_ARGS__,diag::ext_pp_bad_vaargs_use);
-  if (getLangOpts().CPlusPlus20) {
-    (Ident__VA_OPT__ = getIdentifierInfo("__VA_OPT__"))->setIsPoisoned();
-    SetPoisonReason(Ident__VA_OPT__,diag::ext_pp_bad_vaopt_use);
-  } else {
-    Ident__VA_OPT__ = nullptr;
-  }
-
   // Initialize the pragma handlers.
   RegisterBuiltinPragmas();
 
   // Initialize builtin macros like __LINE__ and friends.
   RegisterBuiltinMacros();
 
+  // "Poison" __VA_ARGS__, __VA_OPT__ which can only appear in the expansion of
+  // a macro. They get unpoisoned where it is allowed. Note that we model
+  // __VA_OPT__ as a builtin macro to allow #ifdef and friends to detect it.
+  (Ident__VA_ARGS__ = getIdentifierInfo("__VA_ARGS__"))->setIsPoisoned();
+  SetPoisonReason(Ident__VA_ARGS__, diag::ext_pp_bad_vaargs_use);
+  Ident__VA_OPT__->setIsPoisoned();
+  SetPoisonReason(Ident__VA_OPT__, diag::ext_pp_bad_vaopt_use);
+
   if(LangOpts.Borland) {
     Ident__exception_info        = getIdentifierInfo("_exception_info");
     Ident___exception_info       = getIdentifierInfo("__exception_info");

clang/test/Preprocessor/macro_vaopt_check.cpp

@@ -1,4 +1,20 @@
-// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -pedantic -std=c++2a
+// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -pedantic -std=c++20
+// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -pedantic -std=c++11
+// RUN: %clang_cc1 -x c %s -Eonly -verify -Wno-all -pedantic -std=c99
+
+// Check that support for __VA_OPT__ can be detected by #ifdef.
+#ifndef __VA_OPT__
+#error should be defined
+#endif
+
+#ifdef __VA_OPT__
+#else
+#error should be defined
+#endif
+
+#if !defined(__VA_OPT__)
+#error should be defined
+#endif
 
 //expected-error@+1{{missing '('}}
 #define V1(...) __VA_OPT__  
@@ -62,3 +78,16 @@
 #define V1(...) __VA_OPT__  ((())
 #undef V1
 
+// __VA_OPT__ can't appear anywhere else.
+#if __VA_OPT__ // expected-warning {{__VA_OPT__ can only appear in the expansion of a variadic macro}}
+#endif
+
+#define BAD __VA_OPT__ // expected-warning {{__VA_OPT__ can only appear in the expansion of a variadic macro}}
+
+// Check defined(__VA_OPT__) doesn't leave __VA_OPT__ poisoned.
+#define Z(...) (0 __VA_OPT__(|| 1))
+#if defined(__VA_OPT__) && Z(hello)
+// OK
+#else
+#error bad
+#endif

clang/test/Preprocessor/macro_vaopt_expand.cpp

@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -E %s -pedantic -std=c++2a | FileCheck -strict-whitespace %s
+// RUN: %clang_cc1 -E %s -pedantic -std=c++20 | FileCheck -strict-whitespace %s
+// RUN: %clang_cc1 -E %s -pedantic -std=c++11 | FileCheck -strict-whitespace %s
+// RUN: %clang_cc1 -E -x c %s -pedantic -std=c99 | FileCheck -strict-whitespace %s
 
 #define LPAREN ( 
 #define RPAREN )