Don't allow __VA_OPT__ to be detected by #ifdef.
authorRichard Smith <richard@metafoo.co.uk>
Wed, 27 Jan 2021 21:14:02 +0000 (13:14 -0800)
committerRichard Smith <richard@metafoo.co.uk>
Wed, 27 Jan 2021 21:34:15 +0000 (13:34 -0800)
More study has discovered this to not actually be useful: because
current C++20 implementations reject `#ifdef __VA_OPT__`, this can't
really be used as a feature-test mechanism. And it's not too hard to
detect __VA_OPT__ without this, for example:

  #define THIRD_ARG(a, b, c, ...) c
  #define HAS_VA_OPT(...) THIRD_ARG(__VA_OPT__(,), 1, 0, )
  #if HAS_VA_OPT(?)

Partially reverts 0436ec2128c9775ba13b0308937238fc79673fdd.

clang/include/clang/Lex/Preprocessor.h
clang/lib/Lex/PPDirectives.cpp
clang/lib/Lex/PPExpressions.cpp
clang/lib/Lex/PPMacroExpansion.cpp
clang/lib/Lex/Preprocessor.cpp
clang/test/Preprocessor/macro_vaopt_check.cpp

index ba8bdaa..68139cb 100644 (file)
@@ -447,25 +447,6 @@ public:
           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;
index e2aa934..d6b03d8 100644 (file)
@@ -2928,14 +2928,9 @@ 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
index 952fb8f..8c120c1 100644 (file)
@@ -104,9 +104,6 @@ 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);
 
@@ -125,8 +122,6 @@ 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;
index f6ca04d..43d31d6 100644 (file)
@@ -323,16 +323,13 @@ 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,
-                                            bool Disabled = false) {
+static IdentifierInfo *RegisterBuiltinMacro(Preprocessor &PP, const char *Name){
   // 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;
 }
@@ -346,7 +343,6 @@ 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)
index 9baba20..177786d 100644 (file)
@@ -115,20 +115,19 @@ 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);
+  (Ident__VA_OPT__ = getIdentifierInfo("__VA_OPT__"))->setIsPoisoned();
+  SetPoisonReason(Ident__VA_OPT__,diag::ext_pp_bad_vaopt_use);
+
   // 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");
index 84f3b85..c5c0ac5 100644 (file)
@@ -2,20 +2,6 @@
 // RUN: %clang_cc1 %s -Eonly -verify -Wno-all -pedantic -std=c++11\r
 // RUN: %clang_cc1 -x c %s -Eonly -verify -Wno-all -pedantic -std=c99\r
 \r
-// Check that support for __VA_OPT__ can be detected by #ifdef.\r
-#ifndef __VA_OPT__\r
-#error should be defined\r
-#endif\r
-\r
-#ifdef __VA_OPT__\r
-#else\r
-#error should be defined\r
-#endif\r
-\r
-#if !defined(__VA_OPT__)\r
-#error should be defined\r
-#endif\r
-\r
 //expected-error@+1{{missing '('}}\r
 #define V1(...) __VA_OPT__  \r
 #undef V1\r
 #if __VA_OPT__ // expected-warning {{__VA_OPT__ can only appear in the expansion of a variadic macro}}\r
 #endif\r
 \r
-#define BAD __VA_OPT__ // expected-warning {{__VA_OPT__ can only appear in the expansion of a variadic macro}}\r
-\r
-// Check defined(__VA_OPT__) doesn't leave __VA_OPT__ poisoned.\r
-#define Z(...) (0 __VA_OPT__(|| 1))\r
-#if defined(__VA_OPT__) && Z(hello)\r
-// OK\r
-#else\r
-#error bad\r
+#ifdef __VA_OPT__ // expected-warning {{__VA_OPT__ can only appear in the expansion of a variadic macro}}\r
 #endif\r
+\r
+#define BAD __VA_OPT__ // expected-warning {{__VA_OPT__ can only appear in the expansion of a variadic macro}}\r