Author: Richard Smith
Date: 2021-01-27T13:34:15-08:00
New Revision: 5dfa37a76153f2a18ac7fe30721cc1332b672ea2

URL: 
https://github.com/llvm/llvm-project/commit/5dfa37a76153f2a18ac7fe30721cc1332b672ea2
DIFF: 
https://github.com/llvm/llvm-project/commit/5dfa37a76153f2a18ac7fe30721cc1332b672ea2.diff

LOG: Don't allow __VA_OPT__ to be detected by #ifdef.

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.

Added: 
    

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Lex/Preprocessor.h 
b/clang/include/clang/Lex/Preprocessor.h
index ba8bdaa23c4c..68139cb24b31 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -447,25 +447,6 @@ 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;

diff  --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index e2aa93455ea5..d6b03d85913d 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -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

diff  --git a/clang/lib/Lex/PPExpressions.cpp b/clang/lib/Lex/PPExpressions.cpp
index 952fb8f121dc..8c120c13d7d2 100644
--- a/clang/lib/Lex/PPExpressions.cpp
+++ b/clang/lib/Lex/PPExpressions.cpp
@@ -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;

diff  --git a/clang/lib/Lex/PPMacroExpansion.cpp 
b/clang/lib/Lex/PPMacroExpansion.cpp
index f6ca04defeb9..43d31d6c5732 100644
--- a/clang/lib/Lex/PPMacroExpansion.cpp
+++ b/clang/lib/Lex/PPMacroExpansion.cpp
@@ -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)

diff  --git a/clang/lib/Lex/Preprocessor.cpp b/clang/lib/Lex/Preprocessor.cpp
index 9baba204b324..177786d90390 100644
--- a/clang/lib/Lex/Preprocessor.cpp
+++ b/clang/lib/Lex/Preprocessor.cpp
@@ -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");

diff  --git a/clang/test/Preprocessor/macro_vaopt_check.cpp 
b/clang/test/Preprocessor/macro_vaopt_check.cpp
index 84f3b85871dd..c5c0ac518bc0 100644
--- a/clang/test/Preprocessor/macro_vaopt_check.cpp
+++ b/clang/test/Preprocessor/macro_vaopt_check.cpp
@@ -2,20 +2,6 @@
 // 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__  
 #undef V1
@@ -82,12 +68,7 @@
 #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
+#ifdef __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}}


        
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to