faisalv updated this revision to Diff 109883.
faisalv marked 7 inline comments as done.
faisalv added a comment.

Hi Richard,

  This patch attempts to incorporate all the changes you requested.  

You asked for some consideration on the rationale for sticking with my approach 
(as opposed to the alternative you posed), so here are my reasons (FWTW) for 
not modeling the VA_OPT as a macro argument:

- the standard does not specify it as behaving exactly as a macro argument 
(specifically no rescanning, after placemarker token removal) - and so i would 
have to special case the argument expansion logic - which currently composes 
well by just launching a tokenlexer on the argument.
- it would be the only argument that would need to have *other* arguments 
substituted into it
- unlike every other argument it would not be represented by just 1 token
- in regards to the source-location mapping, my sense is that the ExpansionInfo 
that represents the various expansions might need to add a representation for 
this 'fourth' kind of expansion (since expanded args are represented by a valid 
LocStart but an invalid LocEnd, and since this is more than one token, things 
could get hairy)
- it might require complicating 'Parsing' the macro definition (and the 
MacroArgs, MacroInfo objects) and the general tokenLexer, 
ExpandFunctionArguments() logic more than any gained conceptual elegance (in my 
opinion)

Thanks Richard - Look forward to your review and thoughts!


Repository:
  rL LLVM

https://reviews.llvm.org/D35782

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  include/clang/Lex/Preprocessor.h
  include/clang/Lex/TokenLexer.h
  include/clang/Lex/VariadicMacroSupport.h
  lib/Lex/PPDirectives.cpp
  lib/Lex/Preprocessor.cpp
  lib/Lex/TokenLexer.cpp
  test/Preprocessor/macro_vaopt_check.cpp
  test/Preprocessor/macro_vaopt_expand.cpp

Index: test/Preprocessor/macro_vaopt_expand.cpp
===================================================================
--- test/Preprocessor/macro_vaopt_expand.cpp
+++ test/Preprocessor/macro_vaopt_expand.cpp
@@ -0,0 +1,148 @@
+// RUN: %clang_cc1 -E %s -pedantic -std=c++2a | FileCheck -strict-whitespace %s
+
+#define LPAREN ( 
+#define RPAREN ) 
+
+#define A0 expandedA0
+#define A1  expandedA1 A0
+#define A2  expandedA2 A1
+#define A3  expandedA3 A2
+
+#define A() B LPAREN )
+#define B() C LPAREN )
+#define C() D LPAREN )
+
+
+#define F(x, y) x + y 
+#define ELLIP_FUNC(...) __VA_OPT__(__VA_ARGS__)
+
+1: ELLIP_FUNC(F, LPAREN, 'a', 'b', RPAREN); 
+2: ELLIP_FUNC(F LPAREN 'a', 'b' RPAREN); 
+#undef F
+#undef ELLIP_FUNC
+
+// CHECK: 1: F, (, 'a', 'b', );
+// CHECK: 2: 'a' + 'b';
+
+#define F(...) f(0 __VA_OPT__(,) __VA_ARGS__)
+3: F(a, b, c) // replaced by f(0, a, b, c) 
+4: F() // replaced by f(0)
+
+// CHECK: 3: f(0 , a, b, c) 
+// CHECK: 4: f(0 )
+#undef F
+
+#define G(X, ...) f(0, X __VA_OPT__(,) __VA_ARGS__)
+
+5: G(a, b, c) // replaced by f(0, a , b, c) 
+6: G(a) // replaced by f(0, a) 
+7: G(a,) // replaced by f(0, a) 
+7.1: G(a,,)
+
+
+// CHECK: 5: f(0, a , b, c) 
+// CHECK: 6: f(0, a ) 
+// CHECK: 7: f(0, a ) 
+// CHECK: 7.1: f(0, a , ,)
+#undef G 
+
+#define HT_B() TONG
+
+#define F(x, ...) HT_ ## __VA_OPT__(x x A()  #x)
+
+8: F(1)
+9: F(A(),1)
+
+// CHECK: 8: HT_
+// CHECK: 9: TONG C ( ) B ( ) "A()"
+#undef HT_B
+#undef F
+
+#define F(a,...) #__VA_OPT__(A1 a)
+
+10: F(A())
+11: F(A1 A(), 1)
+// CHECK: 10: ""
+// CHECK: 11: "A1 expandedA1 expandedA0 B ( )"
+#undef F
+
+
+#define F(a,...) a ## __VA_OPT__(A1 a) ## __VA_ARGS__ ## a
+12.0: F()
+12: F(,)
+13: F(B,)
+// CHECK: 12.0: 
+// CHECK: 12: 
+// CHECK: 13: BB 
+#undef F
+
+#define F(...) #__VA_OPT__()  X ## __VA_OPT__()  #__VA_OPT__(        )
+
+14: F()
+15: F(1)
+
+// CHECK: 14: "" X ""
+// CHECK: 15: "" X ""
+
+#undef F
+
+#define SDEF(sname, ...) S sname __VA_OPT__(= { __VA_ARGS__ })
+
+16: SDEF(foo); // replaced by S foo; 
+17: SDEF(bar, 1, 2); // replaced by S bar = { 1, 2 }; 
+
+// CHECK: 16: S foo ;
+// CHECK: 17: S bar = { 1, 2 }; 
+#undef SDEF
+
+#define F(a,...) A() #__VA_OPT__(A3 __VA_ARGS__ a ## __VA_ARGS__ ## a ## C A3) A()
+
+18: F()
+19: F(,)
+20: F(,A3)
+21: F(A3, A(),A0)
+
+
+// CHECK: 18: B ( ) "" B ( ) 
+// CHECK: 19: B ( ) "" B ( ) 
+// CHECK: 20: B ( ) "A3 expandedA3 expandedA2 expandedA1 expandedA0 A3C A3" B ( )
+// CHECK: 21: B ( ) "A3 B ( ),expandedA0 A3A(),A0A3C A3" B ( )
+
+#undef F
+
+#define F(a,...) A() #__VA_OPT__(A3 __VA_ARGS__ a ## __VA_ARGS__ ## a ## C A3) a __VA_OPT__(A0 __VA_ARGS__ a ## __VA_ARGS__ ## a ## C A0) A()
+
+22: F()
+23: F(,)
+24: F(,A0)
+25: F(A0, A(),A0)
+
+
+// CHECK: 22: B ( ) "" B ( ) 
+// CHECK: 23: B ( ) "" B ( ) 
+// CHECK: 24: B ( ) "A3 expandedA0 A0C A3" expandedA0 expandedA0 A0C expandedA0 B ( )
+// CHECK: 25: B ( ) "A3 B ( ),expandedA0 A0A(),A0A0C A3" expandedA0 expandedA0 C ( ),expandedA0 A0A(),A0A0C expandedA0 B ( )
+
+#undef F
+
+#define F(a,...)  __VA_OPT__(B a ## a) ## 1
+#define G(a,...)  __VA_OPT__(B a) ## 1
+26: F(,1)
+26_1: G(,1)
+// CHECK: 26: B1
+// CHECK: 26_1: B1
+#undef F
+#undef G
+
+#define F(a,...)  B ## __VA_OPT__(a 1) ## 1
+#define G(a,...)  B ## __VA_OPT__(a ## a 1) ## 1
+
+27: F(,1)
+27_1: F(A0,1)
+28: G(,1)
+// CHECK: 27: B11
+// CHECK: 27_1: BexpandedA0 11
+// CHECK: 28: B11
+
+#undef F
+#undef G
Index: test/Preprocessor/macro_vaopt_check.cpp
===================================================================
--- test/Preprocessor/macro_vaopt_check.cpp
+++ test/Preprocessor/macro_vaopt_check.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -pedantic -std=c++2a
+
+//expected-error@+1{{missing '('}}
+#define V1(...) __VA_OPT__  
+#undef V1
+// OK
+#define V1(...) __VA_OPT__  ()
+#undef V1 
+
+//expected-warning@+1{{can only appear in the expansion of a variadic macro}}
+#define V2() __VA_OPT__(x) 
+#undef V2
+
+//expected-error@+2{{missing ')' after}}
+//expected-note@+1{{to match this '('}}
+#define V3(...) __VA_OPT__(
+#undef V3
+
+#define V4(...) __VA_OPT__(__VA_ARGS__)
+#undef V4
+
+//expected-error@+1{{nested}}
+#define V5(...) __VA_OPT__(__VA_OPT__())
+#undef V5
+
+//expected-error@+1{{not followed by}}
+#define V1(...) __VA_OPT__  (#)
+#undef V1
+
+//expected-error@+1{{cannot appear at start}}
+#define V1(...) __VA_OPT__  (##)
+#undef V1
+
+//expected-error@+1{{cannot appear at start}}
+#define V1(...) __VA_OPT__  (## X) x
+#undef V1
+
+//expected-error@+1{{cannot appear at end}}
+#define V1(...) y __VA_OPT__  (X ##)
+#undef V1
+                            
+
+#define FOO(x,...) # __VA_OPT__(x) #x #__VA_OPT__(__VA_ARGS__) //OK
+
+//expected-error@+1{{not followed by a macro parameter}}
+#define V1(...) __VA_OPT__(#)
+#undef V1
+
+//expected-error@+1{{cannot appear at start}}
+#define V1(...) a __VA_OPT__(##) b
+#undef V1
+
+//expected-error@+1{{cannot appear at start}}
+#define V1(...) a __VA_OPT__(a ## b) b __VA_OPT__(##)
+#undef V1
+
+#define V1(x,...) # __VA_OPT__(b x) // OK
+#undef V1
+
+//expected-error@+2{{missing ')' after}}
+//expected-note@+1{{to match this '('}}
+#define V1(...) __VA_OPT__  ((())
+#undef V1
+
Index: lib/Lex/TokenLexer.cpp
===================================================================
--- lib/Lex/TokenLexer.cpp
+++ lib/Lex/TokenLexer.cpp
@@ -17,6 +17,7 @@
 #include "clang/Lex/MacroArgs.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/VariadicMacroSupport.h"
 #include "llvm/ADT/SmallString.h"
 
 using namespace clang;
@@ -178,10 +179,26 @@
   // we install the newly expanded sequence as the new 'Tokens' list.
   bool MadeChange = false;
 
+  // Check to see if this invocation supplied any variadic arguments. Use
+  // this information to expand __VA_OPT__  into its arguments (if true) or
+  // empty (if false) appropriately.  Note, for a macro defined as: 
+  // 
+  //    #define F(a, ...) __VA_OPT__(x x x)
+  //    F(,) <-- This is NOT called with variadic arguments (tokens need to be
+  //             supplied).
+  const bool CalledWithVariadicArguments = [this] {
+    if (Macro->isVariadic()) {
+      const int VariadicArgIndex = Macro->getNumParams() - 1;
+      const Token *VarArgs = ActualArgs->getUnexpArgument(VariadicArgIndex);
+      return VarArgs->isNot(tok::eof);
+    }
+    return false;
+  }();
+  
+  VAOptExpansionContext VCtx(PP);
+  
   for (unsigned I = 0, E = NumTokens; I != E; ++I) {
-    // If we found the stringify operator, get the argument stringified.  The
-    // preprocessor already verified that the following token is a macro name
-    // when the #define was parsed.
+    
     const Token &CurTok = Tokens[I];
     // We don't want a space for the next token after a paste
     // operator.  In valid code, the token will get smooshed onto the
@@ -192,10 +209,174 @@
     if (I != 0 && !Tokens[I-1].is(tok::hashhash) && CurTok.hasLeadingSpace())
       NextTokGetsSpace = true;
 
+    if (VCtx.isVAOptToken(CurTok)) {
+      MadeChange = true;
+      assert(Tokens[I + 1].is(tok::l_paren) &&
+             "__VA_OPT__ must be followed by '('");
+      
+      const bool PasteBefore = I ? Tokens[I - 1].is(tok::hashhash) : false;
+      ++I; // Skip the l_paren
+      VCtx.sawVAOptFollowedByOpeningParens(
+          CurTok.getLocation(), ResultToks.size());
+      if (PasteBefore)
+        VCtx.sawHashHashBefore();
+      
+      continue;
+    }
+
+    // We have entered into the __VA_OPT__ context, so handle tokens
+    // appropriately.
+    if (VCtx.isInVAOpt()) {
+      // If we are about to process a token that is an argument to __VA_OPT__ we
+      // need to:
+      //  1) if macro was called without variadic arguments skip all the
+      //  intervening tokens
+      //  2) always process the closing right-parens, and skip the current 
+      //  iteration.
+      //  3) otherwise let the current iteration process this token.
+
+      // Use this variable to skip to the next iteration as described above.
+      bool SkipToNextIterationOfOuterLoop = !CalledWithVariadicArguments;
+      do {
+        if (Tokens[I].is(tok::l_paren))
+          VCtx.sawOpeningParen();
+
+        // Continue skipping tokens within __VA_OPT__ if the macro was not
+        // called with variadic arguments, else let the rest of the loop handle
+        // this token. Note sawClosingParen() returns true only if the r_paren matches
+        // the closing r_paren of the __VA_OPT__.
+        if (!Tokens[I].is(tok::r_paren) || !VCtx.sawClosingParen()) {
+          if (!CalledWithVariadicArguments) {
+            ++I; // Skip Tokens in between.
+            continue /*inner do loop*/;
+          }
+          // The macro was called with variadic arguments, and we do not have a
+          // closing rparen so let the outer loop process this token.
+          break;
+        }
+
+        // Current token is the closing r_paren which marks the end of the
+        // __VA_OPT__ invocation, so handle any place-marker pasting (if
+        // empty) by removing hashhash either before (if exists) or after. And
+        // also stringify the entire contents if VAOPT was preceded by a hash,
+        // but do so only after if any token concatenation needs to occur within
+        // the contents of VAOPT.
+        const int NumToksPriorToVAOpt = VCtx.getNumberOfTokensPriorToVAOpt();
+        const unsigned int NumVAOptTokens =
+            ResultToks.size() - NumToksPriorToVAOpt;
+        const bool HasPasteAfter =
+            (I + 1 != E) ? Tokens[I + 1].is(tok::hashhash) : false;
+        // We'll have to eat any hashhashes before or after these empty tokens.
+        if (!NumVAOptTokens) {
+          if (VCtx.hasPasteBefore()) {
+            assert(!VCtx.hasStringifyOrCharifyBefore() &&
+                   "Can't have both hashhash and hash just before __VA_OPT__");
+
+            // If VAOPT had a hashhash before it (that was itself not preceded
+            // by a placemarker token and therefore added to ResultToks) and if 
+            // VAOPT reduces to a placemarker, remove that hashhash.
+            // 
+            // #define F(a,...) a ## __VA_OPT__(x)
+            //   F()  <-- ResultToks does not contain preceding hashhash.
+            //   F(1) <-- ResultToks does contain the preceding hashhash.
+
+            if (ResultToks.size() && ResultToks.back().is(tok::hashhash))
+              ResultToks.pop_back();
+          } else if (HasPasteAfter && !VCtx.hasStringifyOrCharifyBefore()) {
+            ++I; // Skip the hashhash if the empty tokens are not stringified
+                 // (leads to a pasting error).
+          }
+        }
+
+        if (VCtx.hasStringifyOrCharifyBefore()) {
+          // Replace all the tokens just added from within VAOPT into a single
+          // stringified token. This requires token-pasting to eagerly occur
+          // within these tokens. If either the contents of VAOPT were empty
+          // or the macro wasn't called with any variadic arguments result in
+          // an empty string.
+
+          Token *const VAOPTTokens =
+              NumVAOptTokens ? &ResultToks[NumToksPriorToVAOpt] : nullptr;
+
+          SmallVector<Token, 64> ConcatenatedVAOPTResultToks;
+          // FIXME: Should we keep track within VCtx that we did or didnot
+          // encounter pasting - and only then perform this loop.
+
+          for (unsigned int CurTokenIdx = 0; CurTokenIdx != NumVAOptTokens;
+               ++CurTokenIdx) {
+            const unsigned int PrevTokenIdx = CurTokenIdx;
+
+            if (VAOPTTokens[CurTokenIdx].is(tok::hashhash)) {
+              assert(CurTokenIdx != 0 &&
+                     "Can not have __VAOPT__ contents begin with a ##");
+              Token &LHS = VAOPTTokens[CurTokenIdx - 1];
+              PasteTokens(LHS, llvm::makeArrayRef(VAOPTTokens, NumVAOptTokens),
+                          &CurTokenIdx);
+              // CurTokenIdx is either the same as NumTokens or one past the
+              // last token concatenated.
+              // PrevTokenIdx is the index of the hashhash
+              const unsigned NumTokensPastedTogether =
+                  CurTokenIdx - PrevTokenIdx + 1;
+              // Replace the token prior to the first ## in this iteration.
+              ConcatenatedVAOPTResultToks[ConcatenatedVAOPTResultToks.size() -
+                                          1] = LHS;
+              if (CurTokenIdx == NumVAOptTokens)
+                break;
+              ConcatenatedVAOPTResultToks.push_back(VAOPTTokens[CurTokenIdx]);
+            } else {
+              ConcatenatedVAOPTResultToks.push_back(VAOPTTokens[CurTokenIdx]);
+            }
+          }
+
+          ConcatenatedVAOPTResultToks.push_back(VCtx.getEOFTok());
+          // Get the SourceLocation that represents the start location within
+          // the macro definition that marks where this string is substituted
+          // into: i.e. the __VA_OPT__ and the ')' within the spelling of the
+          // macro definition.
+          const SourceLocation ExpansionLocStartWithinMacro =
+              getExpansionLocForMacroDefLoc(VCtx.getVAOptLoc());
+          const SourceLocation ExpansionLocEndWithinMacro =
+              getExpansionLocForMacroDefLoc(Tokens[I].getLocation());
+
+          Token StringifiedVAOPT = MacroArgs::StringifyArgument(
+              &ConcatenatedVAOPTResultToks[0], PP,
+              VCtx.hasCharifyBefore() /*Charify*/,
+              ExpansionLocStartWithinMacro, ExpansionLocEndWithinMacro);
+
+          if (VCtx.getLeadingSpaceForStringifiedToken())
+            StringifiedVAOPT.setFlag(Token::LeadingSpace);
+
+          StringifiedVAOPT.setFlag(Token::StringifiedInMacro);
+
+          ResultToks.resize(NumToksPriorToVAOpt + 1);
+          ResultToks[NumToksPriorToVAOpt] = StringifiedVAOPT;
+        }
+        VCtx.reset();
+        SkipToNextIterationOfOuterLoop = true;
+        break /*out of inner loop*/;
+      } while (!CalledWithVariadicArguments && I != E);  
+      
+      if (I == E) break /*out of outer loop*/;
+      
+      if (SkipToNextIterationOfOuterLoop)
+        continue;
+    }
+    // If we found the stringify operator, get the argument stringified.  The
+    // preprocessor already verified that the following token is a macro 
+    // parameter or __VA_OPT__ when the #define was lexed.
+    
     if (CurTok.isOneOf(tok::hash, tok::hashat)) {
       int ArgNo = Macro->getParameterNum(Tokens[I+1].getIdentifierInfo());
-      assert(ArgNo != -1 && "Token following # is not an argument?");
-
+      assert((ArgNo != -1 || VCtx.isVAOptToken(Tokens[I + 1])) &&
+             "Token following # is not an argument or __VA_OPT__!");
+      
+      if (ArgNo == -1) {
+        // Handle the __VA_OPT__ case.
+        VCtx.sawHashOrHashAtBefore(NextTokGetsSpace,
+                                   CurTok.is(tok::hashat));
+        continue;
+      }
+      // Else handle the simple argument case.
       SourceLocation ExpansionLocStart =
           getExpansionLocForMacroDefLoc(CurTok.getLocation());
       SourceLocation ExpansionLocEnd =
@@ -232,8 +413,12 @@
       !ResultToks.empty() && ResultToks.back().is(tok::hashhash);
     bool PasteBefore = I != 0 && Tokens[I-1].is(tok::hashhash);
     bool PasteAfter = I+1 != E && Tokens[I+1].is(tok::hashhash);
-    assert(!NonEmptyPasteBefore || PasteBefore);
 
+    assert((!NonEmptyPasteBefore || PasteBefore || VCtx.isInVAOpt()) &&
+           "If the substituted ResultToks has hashhash as its most recent "
+           "token, but the original replacement tokens doesn't, then this can "
+           "only happen if we just entered into VAOPT after hashhash");
+
     // Otherwise, if this is not an argument token, just add the token to the
     // output buffer.
     IdentifierInfo *II = CurTok.getIdentifierInfo();
@@ -384,7 +569,13 @@
     assert(PasteBefore);
     if (NonEmptyPasteBefore) {
       assert(ResultToks.back().is(tok::hashhash));
-      ResultToks.pop_back();
+      // Do not remove the paste operator if it is the one before __VA_OPT__
+      // (and we are still processing tokens within VA_OPT).  We handle the case
+      // of removing the paste operator if __VA_OPT__ reduces to the notional
+      // placemarker above when we encounter the closing paren of VA_OPT.
+      if (!VCtx.isInVAOpt() ||
+          ResultToks.size() > VCtx.getNumberOfTokensPriorToVAOpt())
+        ResultToks.pop_back();
     }
 
     // If this is the __VA_ARGS__ token, and if the argument wasn't provided,
@@ -521,11 +712,27 @@
   return true;
 }
 
+
 /// PasteTokens - Tok is the LHS of a ## operator, and CurToken is the ##
 /// operator.  Read the ## and RHS, and paste the LHS/RHS together.  If there
 /// are more ## after it, chomp them iteratively.  Return the result as Tok.
 /// If this returns true, the caller should immediately return the token.
-bool TokenLexer::PasteTokens(Token &Tok) {
+bool TokenLexer::PasteTokens(Token &Tok, llvm::ArrayRef<Token> AltTokens,
+                             unsigned int *const AltCurTokenIdx) {
+
+  // FIXME: This is a ugly hack to minimize changes for review.  This function
+  // should have this functionality factored out into a common function that
+  // takes these following as arguments either before or after this commit.
+  const Token *const Tokens =
+      AltTokens.size() ? AltTokens.data() : this->Tokens;
+  assert(!AltTokens.size() ||
+         AltCurTokenIdx &&
+             "Must specify AltCurTokenIdx if providing AltTokens");
+  unsigned int &CurToken = AltCurTokenIdx ? *AltCurTokenIdx : this->CurToken;
+
+  auto IsAtEnd = [&CurToken, &AltTokens, this] {
+    return !AltTokens.size() ? this->isAtEnd() : CurToken == AltTokens.size();
+  };
   // MSVC: If previous token was pasted, this must be a recovery from an invalid
   // paste operation. Ignore spaces before this token to mimic MSVC output.
   // Required for generating valid UUID strings in some MS headers.
@@ -542,7 +749,7 @@
     PasteOpLoc = Tokens[CurToken].getLocation();
     if (Tokens[CurToken].is(tok::hashhash))
       ++CurToken;
-    assert(!isAtEnd() && "No token on the RHS of a paste operator!");
+    assert(!IsAtEnd() && "No token on the RHS of a paste operator!");
 
     // Get the RHS token.
     const Token &RHS = Tokens[CurToken];
@@ -670,7 +877,7 @@
     // Finally, replace LHS with the result, consume the RHS, and iterate.
     ++CurToken;
     Tok = Result;
-  } while (!isAtEnd() && Tokens[CurToken].is(tok::hashhash));
+  } while (!IsAtEnd() && Tokens[CurToken].is(tok::hashhash));
 
   SourceLocation EndLoc = Tokens[CurToken - 1].getLocation();
 
Index: lib/Lex/Preprocessor.cpp
===================================================================
--- lib/Lex/Preprocessor.cpp
+++ lib/Lex/Preprocessor.cpp
@@ -121,12 +121,18 @@
 
   // We haven't read anything from the external source.
   ReadMacrosFromExternalSource = false;
-  
-  // "Poison" __VA_ARGS__, which can only appear in the expansion of a macro.
-  // This gets unpoisoned where it is allowed.
+
+  // "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().CPlusPlus2a) {
+    (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();
   
@@ -663,13 +669,15 @@
   // unpoisoned it if we're defining a C99 macro.
   if (II.isOutOfDate()) {
     bool CurrentIsPoisoned = false;
-    if (&II == Ident__VA_ARGS__)
-      CurrentIsPoisoned = Ident__VA_ARGS__->isPoisoned();
+    const bool IsSpecialVariadicMacro =
+        &II == Ident__VA_ARGS__ || &II == Ident__VA_OPT__;
+    if (IsSpecialVariadicMacro)
+      CurrentIsPoisoned = II.isPoisoned();
 
     updateOutOfDateIdentifier(II);
     Identifier.setKind(II.getTokenID());
 
-    if (&II == Ident__VA_ARGS__)
+    if (IsSpecialVariadicMacro)
       II.setIsPoisoned(CurrentIsPoisoned);
   }
   
Index: lib/Lex/PPDirectives.cpp
===================================================================
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -2372,6 +2372,9 @@
     // Otherwise, read the body of a function-like macro.  While we are at it,
     // check C99 6.10.3.2p1: ensure that # operators are followed by macro
     // parameters in function-like macro expansions.
+
+    VAOptDefinitionContext VAOCtx(*this);
+
     while (Tok.isNot(tok::eod)) {
       LastTok = Tok;
 
@@ -2378,6 +2381,41 @@
       if (!Tok.isOneOf(tok::hash, tok::hashat, tok::hashhash)) {
         MI->AddTokenToBody(Tok);
 
+        if (VAOCtx.isVAOptToken(Tok)) {
+          // If we're already within a VAOPT, emit an error.
+          if (VAOCtx.isInVAOpt()) {
+            Diag(Tok, diag::err_pp_vaopt_nested_use);
+            return nullptr;
+          }
+          // Ensure VAOPT is followed by a '(' .
+          LexUnexpandedToken(Tok);
+          if (Tok.isNot(tok::l_paren)) {
+            Diag(Tok, diag::err_pp_missing_lparen_in_vaopt_use);
+            return nullptr;
+          }
+          MI->AddTokenToBody(Tok);
+          VAOCtx.sawVAOptFollowedByOpeningParens(Tok.getLocation());
+          LexUnexpandedToken(Tok);
+          if (Tok.is(tok::hashhash)) {
+            Diag(Tok, diag::err_vaopt_paste_at_start);
+            return nullptr;
+          }
+          continue;
+        } else if (VAOCtx.isInVAOpt()) {
+          if (Tok.is(tok::r_paren)) {
+            if (VAOCtx.sawClosingParen()) {
+              const unsigned NumTokens = MI->getNumTokens();
+              assert(NumTokens >= 3 && "Must have seen at least __VA_OPT__( "
+                                       "and a subsequent tok::r_paren");
+              if (MI->getReplacementToken(NumTokens - 2).is(tok::hashhash)) {
+                Diag(Tok, diag::err_vaopt_paste_at_end);
+                return nullptr;
+              }
+            }
+          } else if (Tok.is(tok::l_paren)) {
+            VAOCtx.sawOpeningParen(Tok.getLocation());
+          }
+        }
         // Get the next token of the macro.
         LexUnexpandedToken(Tok);
         continue;
@@ -2418,12 +2456,14 @@
         continue;
       }
 
+      // Our Token is a stringization operator.
       // Get the next token of the macro.
       LexUnexpandedToken(Tok);
 
-      // Check for a valid macro arg identifier.
-      if (Tok.getIdentifierInfo() == nullptr ||
-          MI->getParameterNum(Tok.getIdentifierInfo()) == -1) {
+      // Check for a valid macro arg identifier or __VA_OPT__.
+      if (!VAOCtx.isVAOptToken(Tok) &&
+          (Tok.getIdentifierInfo() == nullptr ||
+           MI->getParameterNum(Tok.getIdentifierInfo()) == -1)) {
 
         // If this is assembler-with-cpp mode, we accept random gibberish after
         // the '#' because '#' is often a comment character.  However, change
@@ -2442,12 +2482,25 @@
 
       // Things look ok, add the '#' and param name tokens to the macro.
       MI->AddTokenToBody(LastTok);
-      MI->AddTokenToBody(Tok);
-      LastTok = Tok;
 
-      // Get the next token of the macro.
-      LexUnexpandedToken(Tok);
+      // If the token following '#' is VAOPT, let the next iteration handle it
+      // and check it for correctness, otherwise add the token and prime the
+      // loop with the next one.
+      if (!VAOCtx.isVAOptToken(Tok)) {
+        MI->AddTokenToBody(Tok);
+        LastTok = Tok;
+
+        // Get the next token of the macro.
+        LexUnexpandedToken(Tok);
+      }
     }
+    if (VAOCtx.isInVAOpt()) {
+      assert(Tok.is(tok::eod) && "Must be at End Of preprocessing Directive");
+      Diag(Tok, diag::err_pp_expected_after)
+        << LastTok.getKind() << tok::r_paren;
+      Diag(VAOCtx.getUnmatchedOpeningParenLoc(), diag::note_matching) << tok::l_paren;
+      return nullptr;
+    }
   }
   MI->setDefinitionEndLoc(LastTok.getLocation());
   return MI;
Index: include/clang/Lex/VariadicMacroSupport.h
===================================================================
--- include/clang/Lex/VariadicMacroSupport.h
+++ include/clang/Lex/VariadicMacroSupport.h
@@ -1,4 +1,4 @@
-//===- VariadicMacroSupport.h - scope-guards etc. -*- C++ -*---------------===//
+//===- VariadicMacroSupport.h - state-machines and scope-guards -*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
@@ -17,40 +17,224 @@
 #define LLVM_CLANG_LEX_VARIADICMACROSUPPORT_H
 
 #include "clang/Lex/Preprocessor.h"
+#include "llvm/ADT/SmallVector.h"
 
 namespace clang {
+  class Preprocessor;
 
-/// An RAII class that tracks when the Preprocessor starts and stops lexing the
-/// definition of a (ISO C/C++) variadic macro.  As an example, this is useful
-/// for unpoisoning and repoisoning certain identifiers (such as __VA_ARGS__)
-/// that are only allowed in this context.  Also, being a friend of the
-/// Preprocessor class allows it to access PP's cached identifiers directly (as
-/// opposed to performing a lookup each time).
-class VariadicMacroScopeGuard {
-  const Preprocessor &PP;
-  IdentifierInfo &Ident__VA_ARGS__;
+  /// An RAII class that tracks when the Preprocessor starts and stops lexing
+  /// the definition of a (ISO C/C++) variadic macro.  As an example, this is
+  /// useful for unpoisoning and repoisoning certain identifiers (such as
+  /// __VA_ARGS__) that are only allowed in this context.  Also, being a friend
+  /// of the Preprocessor class allows it to access PP's cached identifiers
+  /// directly (as opposed to performing a lookup each time).
+  class VariadicMacroScopeGuard {
+    const Preprocessor &PP;
+    IdentifierInfo *const Ident__VA_ARGS__;
+    IdentifierInfo *const Ident__VA_OPT__;
 
-public:
-  VariadicMacroScopeGuard(const Preprocessor &P)
-      : PP(P), Ident__VA_ARGS__(*PP.Ident__VA_ARGS__) {
-    assert(Ident__VA_ARGS__.isPoisoned() && "__VA_ARGS__ should be poisoned "
-                                            "outside an ISO C/C++ variadic "
-                                            "macro definition!");
-  }
+  public:
+    VariadicMacroScopeGuard(const Preprocessor &P)
+        : PP(P), Ident__VA_ARGS__(PP.Ident__VA_ARGS__),
+          Ident__VA_OPT__(PP.Ident__VA_OPT__) {
+      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!"));
+    }
 
-  /// 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); }
+    /// 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);
+    }
 
-  /// Client code should call this function as soon as the Preprocessor has
-  /// either completed lexing the macro's definition tokens, or an error occured
-  /// and the context is being exited.  This function is idempotent (might be
-  /// explicitly called, and then reinvoked via the destructor).
-  void exitScope() { Ident__VA_ARGS__.setIsPoisoned(true); }
+    /// Client code should call this function as soon as the Preprocessor has
+    /// either completed lexing the macro's definition tokens, or an error
+    /// occured and the context is being exited.  This function is idempotent
+    /// (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);
+    }
 
-  ~VariadicMacroScopeGuard() { exitScope(); }
-};
+    ~VariadicMacroScopeGuard() { exitScope(); }
+  };
 
+  class VAOptDefinitionContext {
+    Preprocessor &PP;
+    
+    /// Contains all the locations of so far unmatched lparens.
+    SmallVector<SourceLocation, 8> UnmatchedOpeningParens;
+    
+    const IdentifierInfo *const Ident__VA_OPT__;
+    
+    
+  public:
+    VAOptDefinitionContext(Preprocessor &PP)
+        : PP(PP), Ident__VA_OPT__(PP.Ident__VA_OPT__) {}
+
+    bool isVAOptToken(const Token &T) const {
+      return Ident__VA_OPT__ && T.getIdentifierInfo() == Ident__VA_OPT__;
+    }
+
+    /// Returns true if we have seen the __VA_OPT__ and '(' but before having
+    /// seen the matching ')'.
+    bool isInVAOpt() const { return UnmatchedOpeningParens.size(); }
+    
+    /// Call this function as soon as you see __VA_OPT__ and '('.
+    void sawVAOptFollowedByOpeningParens(const SourceLocation LParenLoc) {
+      assert(!isInVAOpt() && "Must NOT be within VAOPT context to call this");
+      UnmatchedOpeningParens.push_back(LParenLoc);
+      
+    }
+
+    SourceLocation getUnmatchedOpeningParenLoc() const {
+      assert(isInVAOpt() && "Must be within VAOPT context to call this");
+      return UnmatchedOpeningParens.back();
+    }
+
+    /// Call this function each time an rparen is seen.  It returns true only if
+    /// the rparen that was just seen was the eventual (non-nested) closing
+    /// paren for VAOPT, and ejects us out of the VAOPT context.
+    bool sawClosingParen() {
+      assert(isInVAOpt() && "Must be within VAOPT context to call this");
+      UnmatchedOpeningParens.pop_back();
+      return !UnmatchedOpeningParens.size();
+    }
+    
+    /// Call this function each time an lparen is seen.
+    void sawOpeningParen(SourceLocation LParenLoc) {
+      assert(isInVAOpt() && "Must be within VAOPT context to call this");
+      UnmatchedOpeningParens.push_back(LParenLoc);
+    }
+    
+  };
+
+  class VAOptExpansionContext : VAOptDefinitionContext {
+
+    Token SyntheticEOFToken;
+
+    // The (spelling) location of the current __VA_OPT__ in the replacement list
+    // of the function-like macro being expanded.
+    SourceLocation VAOptLoc;
+
+    // NumOfTokensPriorToVAOpt : when != -1, contains the index *of* the first
+    // token of the current VAOPT contents (so we know where to start eager
+    // token-pasting and stringification) *within*  the substituted tokens of
+    // the function-like macro's new replacement list.
+    int NumOfTokensPriorToVAOpt = -1;
+
+    unsigned LeadingSpaceForStringifiedToken : 1;
+    unsigned PasteBefore : 1;
+    unsigned StringifyBefore : 1;
+    unsigned CharifyBefore : 1;
+    
+    
+    bool hasStringifyBefore() const {
+      assert(!isReset() &&
+             "Must only be called if the state has not been reset");
+      return StringifyBefore;
+    }
+
+    bool isReset() const {
+      return NumOfTokensPriorToVAOpt == -1 ||
+             VAOptLoc.isInvalid();
+    }
+
+  public:
+    VAOptExpansionContext(Preprocessor &PP)
+        : VAOptDefinitionContext(PP), LeadingSpaceForStringifiedToken(false),
+          PasteBefore(false), StringifyBefore(false), CharifyBefore(false) {
+      SyntheticEOFToken.startToken();
+      SyntheticEOFToken.setKind(tok::eof);
+    }
+
+    void reset() {
+      VAOptLoc = SourceLocation();
+      NumOfTokensPriorToVAOpt = -1;
+      LeadingSpaceForStringifiedToken = false;
+      PasteBefore = false;
+      StringifyBefore = false;
+      CharifyBefore = false;
+    }
+
+    const Token &getEOFTok() const { return SyntheticEOFToken; }
+
+    void sawHashOrHashAtBefore(const bool HasLeadingSpace,
+                               const bool IsHashAt) {
+      
+      StringifyBefore = !IsHashAt;
+      CharifyBefore = IsHashAt;
+      LeadingSpaceForStringifiedToken = HasLeadingSpace;
+    }
+
+    void sawHashHashBefore() {
+      assert(isInVAOpt() &&
+             "Must only be called if we have entered VAOPT context");
+      PasteBefore = true;
+    }
+
+    bool hasPasteBefore() const {
+      assert(!isReset() &&
+             "Must only be called if the state has not been reset");
+      return PasteBefore;
+    }
+
+    
+    bool hasCharifyBefore() const {
+      assert(!isReset() &&
+             "Must only be called if the state has not been reset");
+      return CharifyBefore;
+    }
+    bool hasStringifyOrCharifyBefore() const {
+      return hasStringifyBefore() || hasCharifyBefore();
+    }
+    
+    unsigned int getNumberOfTokensPriorToVAOpt() const {
+      assert(!isReset() &&
+             "Must only be called if the state has not been reset");
+      return NumOfTokensPriorToVAOpt;
+    }
+    
+    bool getLeadingSpaceForStringifiedToken() const {
+      assert(hasStringifyBefore() &&
+             "Must only be called if this has been marked for stringification");
+      return LeadingSpaceForStringifiedToken;
+    }
+
+    void sawVAOptFollowedByOpeningParens(const SourceLocation VAOptLoc,
+                                         const unsigned int NumPriorTokens) {
+      assert(VAOptLoc.isFileID() && "Must not come from a macro expansion");
+      assert(isReset() && "Must only be called if the state has been reset");
+      VAOptDefinitionContext::sawVAOptFollowedByOpeningParens(SourceLocation());
+      this->VAOptLoc = VAOptLoc;
+      NumOfTokensPriorToVAOpt = NumPriorTokens;
+      assert(NumOfTokensPriorToVAOpt > -1 &&
+             "Too many prior tokens");
+    }
+
+    SourceLocation getVAOptLoc() const {
+      assert(!isReset() &&
+             "Must only be called if the state has not been reset");
+      assert(VAOptLoc.isValid() && "__VA_OPT__ location must be valid");
+      return VAOptLoc;
+    }
+    void sawOpeningParen() {
+      // When expanding __VA_OPT__ contents, we don't really care about the
+      // location of the parens, since we know they are appropriately matched.
+      VAOptDefinitionContext::sawOpeningParen(SourceLocation());
+    }
+    using VAOptDefinitionContext::isVAOptToken;
+    using VAOptDefinitionContext::isInVAOpt;
+    using VAOptDefinitionContext::sawClosingParen;
+    
+  };
 }  // end namespace clang
 
 #endif
Index: include/clang/Lex/TokenLexer.h
===================================================================
--- include/clang/Lex/TokenLexer.h
+++ include/clang/Lex/TokenLexer.h
@@ -15,6 +15,7 @@
 #define LLVM_CLANG_LEX_TOKENLEXER_H
 
 #include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/ArrayRef.h"
 
 namespace clang {
   class MacroInfo;
@@ -164,7 +165,9 @@
   /// are is another ## after it, chomp it iteratively.  Return the result as
   /// Tok.  If this returns true, the caller should immediately return the
   /// token.
-  bool PasteTokens(Token &Tok);
+  bool PasteTokens(Token &Tok,
+                   llvm::ArrayRef<Token> AltTokens = llvm::ArrayRef<Token>(),
+                   unsigned int *const AltCurTokenIdx = nullptr);
 
   /// Expand the arguments of a function-like macro so that we can quickly
   /// return preexpanded tokens from Tokens.
Index: include/clang/Lex/Preprocessor.h
===================================================================
--- include/clang/Lex/Preprocessor.h
+++ include/clang/Lex/Preprocessor.h
@@ -97,6 +97,7 @@
 /// token expansion, etc.
 class Preprocessor {
   friend class VariadicMacroScopeGuard;
+  friend class VAOptDefinitionContext;
   std::shared_ptr<PreprocessorOptions> PPOpts;
   DiagnosticsEngine        *Diags;
   LangOptions       &LangOpts;
@@ -131,6 +132,7 @@
   IdentifierInfo *Ident_Pragma, *Ident__pragma;    // _Pragma, __pragma
   IdentifierInfo *Ident__identifier;               // __identifier
   IdentifierInfo *Ident__VA_ARGS__;                // __VA_ARGS__
+  IdentifierInfo *Ident__VA_OPT__;                 // __VA_OPT__
   IdentifierInfo *Ident__has_feature;              // __has_feature
   IdentifierInfo *Ident__has_extension;            // __has_extension
   IdentifierInfo *Ident__has_builtin;              // __has_builtin
Index: include/clang/Basic/DiagnosticLexKinds.td
===================================================================
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -346,6 +346,20 @@
 def ext_pp_comma_expr : Extension<"comma operator in operand of #if">;
 def ext_pp_bad_vaargs_use : Extension<
   "__VA_ARGS__ can only appear in the expansion of a C99 variadic macro">;
+
+def ext_pp_bad_vaopt_use : Extension<
+  "__VA_OPT__ can only appear in the expansion of a variadic macro">;
+def err_pp_missing_lparen_in_vaopt_use : Error<
+  "missing '(' following __VA_OPT__">;
+def err_pp_vaopt_nested_use : Error<
+  "__VA_OPT__ cannot be nested within its own replacement tokens">;
+
+def err_vaopt_paste_at_start : Error<
+  "'##' cannot appear at start of __VA_OPT__ argument">;
+
+def err_vaopt_paste_at_end : Error<"'##' cannot appear at end within __VA_OPT__">;
+
+
 def ext_pp_macro_redef : ExtWarn<"%0 macro redefined">, InGroup<MacroRedefined>;
 def ext_variadic_macro : Extension<"variadic macros are a C99 feature">,
   InGroup<VariadicMacros>;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D35782: [... Richard Smith - zygoloid via Phabricator via cfe-commits
    • [PATCH] D357... Faisal Vali via Phabricator via cfe-commits
    • [PATCH] D357... Faisal Vali via Phabricator via cfe-commits
    • [PATCH] D357... David Majnemer via Phabricator via cfe-commits
    • [PATCH] D357... Richard Smith - zygoloid via Phabricator via cfe-commits

Reply via email to