[clang] [Clang][Parse] Fix ambiguity with nested-name-specifiers that may declarative (PR #96364)

Krystian Stasiowski via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 29 10:07:12 PDT 2024


https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/96364

>From 496e0cf24b07622bd9354297c20091c2057a55c6 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Wed, 12 Jun 2024 14:14:26 -0400
Subject: [PATCH 1/8] [Clang][Parse] Fix ambiguity with nested-name-specifiers
 that may be declarative

---
 clang/include/clang/Lex/Preprocessor.h        | 14 +++-
 clang/include/clang/Parse/Parser.h            | 13 ++-
 clang/lib/Lex/PPCaching.cpp                   | 39 ++++++++-
 clang/lib/Parse/ParseCXXInlineMethods.cpp     | 41 +--------
 clang/lib/Parse/ParseDecl.cpp                 | 83 ++++++++++++-------
 clang/lib/Parse/ParseExprCXX.cpp              | 19 ++---
 .../CXX/dcl.decl/dcl.meaning/dcl.mptr/p2.cpp  | 64 ++++++++++++++
 clang/test/CXX/temp/temp.res/p3.cpp           | 10 +--
 8 files changed, 182 insertions(+), 101 deletions(-)
 create mode 100644 clang/test/CXX/dcl.decl/dcl.meaning/dcl.mptr/p2.cpp

diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index fc7d0053f2323..b2dbd7d5375b5 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -1160,6 +1160,9 @@ class Preprocessor {
   /// invoked (at which point the last position is popped).
   std::vector<CachedTokensTy::size_type> BacktrackPositions;
 
+  std::vector<std::pair<CachedTokensTy, CachedTokensTy::size_type>>
+      UnannotatedBacktrackPositions;
+
   /// True if \p Preprocessor::SkipExcludedConditionalBlock() is running.
   /// This is used to guard against calling this function recursively.
   ///
@@ -1722,7 +1725,7 @@ class Preprocessor {
   /// at some point after EnableBacktrackAtThisPos. If you don't, caching of
   /// tokens will continue indefinitely.
   ///
-  void EnableBacktrackAtThisPos();
+  void EnableBacktrackAtThisPos(bool Unannotated = false);
 
   /// Disable the last EnableBacktrackAtThisPos call.
   void CommitBacktrackedTokens();
@@ -1733,7 +1736,11 @@ class Preprocessor {
 
   /// True if EnableBacktrackAtThisPos() was called and
   /// caching of tokens is on.
+
   bool isBacktrackEnabled() const { return !BacktrackPositions.empty(); }
+  bool isUnannotatedBacktrackEnabled() const {
+    return !UnannotatedBacktrackPositions.empty();
+  }
 
   /// Lex the next token for this preprocessor.
   void Lex(Token &Result);
@@ -1841,8 +1848,9 @@ class Preprocessor {
   void RevertCachedTokens(unsigned N) {
     assert(isBacktrackEnabled() &&
            "Should only be called when tokens are cached for backtracking");
-    assert(signed(CachedLexPos) - signed(N) >= signed(BacktrackPositions.back())
-         && "Should revert tokens up to the last backtrack position, not more");
+    assert(signed(CachedLexPos) - signed(N) >=
+               signed(BacktrackPositions.back() >> 1) &&
+           "Should revert tokens up to the last backtrack position, not more");
     assert(signed(CachedLexPos) - signed(N) >= 0 &&
            "Corrupted backtrack positions ?");
     CachedLexPos -= N;
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 35bb1a19d40f0..7f5e21127d1e5 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1034,7 +1034,7 @@ class Parser : public CodeCompletionHandler {
     bool isActive;
 
   public:
-    explicit TentativeParsingAction(Parser &p)
+    explicit TentativeParsingAction(Parser &p, bool Unannotated = false)
         : P(p), PrevPreferredType(P.PreferredType) {
       PrevTok = P.Tok;
       PrevTentativelyDeclaredIdentifierCount =
@@ -1042,7 +1042,7 @@ class Parser : public CodeCompletionHandler {
       PrevParenCount = P.ParenCount;
       PrevBracketCount = P.BracketCount;
       PrevBraceCount = P.BraceCount;
-      P.PP.EnableBacktrackAtThisPos();
+      P.PP.EnableBacktrackAtThisPos(Unannotated);
       isActive = true;
     }
     void Commit() {
@@ -1073,13 +1073,11 @@ class Parser : public CodeCompletionHandler {
   class RevertingTentativeParsingAction
       : private Parser::TentativeParsingAction {
   public:
-    RevertingTentativeParsingAction(Parser &P)
-        : Parser::TentativeParsingAction(P) {}
+    using TentativeParsingAction::TentativeParsingAction;
+
     ~RevertingTentativeParsingAction() { Revert(); }
   };
 
-  class UnannotatedTentativeParsingAction;
-
   /// ObjCDeclContextSwitch - An object used to switch context from
   /// an objective-c decl context to its enclosing decl context and
   /// back.
@@ -1984,7 +1982,8 @@ class Parser : public CodeCompletionHandler {
       CXXScopeSpec &SS, ParsedType ObjectType, bool ObjectHasErrors,
       bool EnteringContext, bool *MayBePseudoDestructor = nullptr,
       bool IsTypename = false, const IdentifierInfo **LastII = nullptr,
-      bool OnlyNamespace = false, bool InUsingDeclaration = false);
+      bool OnlyNamespace = false, bool InUsingDeclaration = false,
+      bool Disambiguation = false);
 
   //===--------------------------------------------------------------------===//
   // C++11 5.1.2: Lambda expressions
diff --git a/clang/lib/Lex/PPCaching.cpp b/clang/lib/Lex/PPCaching.cpp
index f38ff62ebf437..bc52bfb237e5c 100644
--- a/clang/lib/Lex/PPCaching.cpp
+++ b/clang/lib/Lex/PPCaching.cpp
@@ -22,9 +22,12 @@ using namespace clang;
 // Nested backtracks are allowed, meaning that EnableBacktrackAtThisPos can
 // be called multiple times and CommitBacktrackedTokens/Backtrack calls will
 // be combined with the EnableBacktrackAtThisPos calls in reverse order.
-void Preprocessor::EnableBacktrackAtThisPos() {
+void Preprocessor::EnableBacktrackAtThisPos(bool Unannotated) {
   assert(LexLevel == 0 && "cannot use lookahead while lexing");
-  BacktrackPositions.push_back(CachedLexPos);
+  BacktrackPositions.push_back((CachedLexPos << 1) | Unannotated);
+  if (Unannotated)
+    UnannotatedBacktrackPositions.emplace_back(CachedTokens,
+                                               CachedTokens.size());
   EnterCachingLexMode();
 }
 
@@ -32,7 +35,18 @@ void Preprocessor::EnableBacktrackAtThisPos() {
 void Preprocessor::CommitBacktrackedTokens() {
   assert(!BacktrackPositions.empty()
          && "EnableBacktrackAtThisPos was not called!");
+  auto BacktrackPos = BacktrackPositions.back();
   BacktrackPositions.pop_back();
+  if (BacktrackPos & 1) {
+    assert(!UnannotatedBacktrackPositions.empty() &&
+           "missing unannotated tokens?");
+    auto [UnannotatedTokens, NumCachedToks] =
+        std::move(UnannotatedBacktrackPositions.back());
+    if (!UnannotatedBacktrackPositions.empty())
+      UnannotatedBacktrackPositions.back().first.append(
+          UnannotatedTokens.begin() + NumCachedToks, UnannotatedTokens.end());
+    UnannotatedBacktrackPositions.pop_back();
+  }
 }
 
 // Make Preprocessor re-lex the tokens that were lexed since
@@ -40,8 +54,20 @@ void Preprocessor::CommitBacktrackedTokens() {
 void Preprocessor::Backtrack() {
   assert(!BacktrackPositions.empty()
          && "EnableBacktrackAtThisPos was not called!");
-  CachedLexPos = BacktrackPositions.back();
+  auto BacktrackPos = BacktrackPositions.back();
   BacktrackPositions.pop_back();
+  CachedLexPos = BacktrackPos >> 1;
+  if (BacktrackPos & 1) {
+    assert(!UnannotatedBacktrackPositions.empty() &&
+           "missing unannotated tokens?");
+    auto [UnannotatedTokens, NumCachedToks] =
+        std::move(UnannotatedBacktrackPositions.back());
+    UnannotatedBacktrackPositions.pop_back();
+    if (!UnannotatedBacktrackPositions.empty())
+      UnannotatedBacktrackPositions.back().first.append(
+          UnannotatedTokens.begin() + NumCachedToks, UnannotatedTokens.end());
+    CachedTokens = std::move(UnannotatedTokens);
+  }
   recomputeCurLexerKind();
 }
 
@@ -67,6 +93,8 @@ void Preprocessor::CachingLex(Token &Result) {
     EnterCachingLexModeUnchecked();
     CachedTokens.push_back(Result);
     ++CachedLexPos;
+    if (isUnannotatedBacktrackEnabled())
+      UnannotatedBacktrackPositions.back().first.push_back(Result);
     return;
   }
 
@@ -108,6 +136,8 @@ const Token &Preprocessor::PeekAhead(unsigned N) {
   for (size_t C = CachedLexPos + N - CachedTokens.size(); C > 0; --C) {
     CachedTokens.push_back(Token());
     Lex(CachedTokens.back());
+    if (isUnannotatedBacktrackEnabled())
+      UnannotatedBacktrackPositions.back().first.push_back(CachedTokens.back());
   }
   EnterCachingLexMode();
   return CachedTokens.back();
@@ -124,7 +154,8 @@ void Preprocessor::AnnotatePreviousCachedTokens(const Token &Tok) {
   for (CachedTokensTy::size_type i = CachedLexPos; i != 0; --i) {
     CachedTokensTy::iterator AnnotBegin = CachedTokens.begin() + i-1;
     if (AnnotBegin->getLocation() == Tok.getLocation()) {
-      assert((BacktrackPositions.empty() || BacktrackPositions.back() <= i) &&
+      assert((BacktrackPositions.empty() ||
+              (BacktrackPositions.back() >> 1) <= i) &&
              "The backtrack pos points inside the annotated tokens!");
       // Replace the cached tokens with the single annotation token.
       if (i < CachedLexPos)
diff --git a/clang/lib/Parse/ParseCXXInlineMethods.cpp b/clang/lib/Parse/ParseCXXInlineMethods.cpp
index 9ccbbf9a7d5d0..b461743833c82 100644
--- a/clang/lib/Parse/ParseCXXInlineMethods.cpp
+++ b/clang/lib/Parse/ParseCXXInlineMethods.cpp
@@ -1205,41 +1205,6 @@ bool Parser::ConsumeAndStoreConditional(CachedTokens &Toks) {
   return true;
 }
 
-/// A tentative parsing action that can also revert token annotations.
-class Parser::UnannotatedTentativeParsingAction : public TentativeParsingAction {
-public:
-  explicit UnannotatedTentativeParsingAction(Parser &Self,
-                                             tok::TokenKind EndKind)
-      : TentativeParsingAction(Self), Self(Self), EndKind(EndKind) {
-    // Stash away the old token stream, so we can restore it once the
-    // tentative parse is complete.
-    TentativeParsingAction Inner(Self);
-    Self.ConsumeAndStoreUntil(EndKind, Toks, true, /*ConsumeFinalToken*/false);
-    Inner.Revert();
-  }
-
-  void RevertAnnotations() {
-    Revert();
-
-    // Put back the original tokens.
-    Self.SkipUntil(EndKind, StopAtSemi | StopBeforeMatch);
-    if (Toks.size()) {
-      auto Buffer = std::make_unique<Token[]>(Toks.size());
-      std::copy(Toks.begin() + 1, Toks.end(), Buffer.get());
-      Buffer[Toks.size() - 1] = Self.Tok;
-      Self.PP.EnterTokenStream(std::move(Buffer), Toks.size(), true,
-                               /*IsReinject*/ true);
-
-      Self.Tok = Toks.front();
-    }
-  }
-
-private:
-  Parser &Self;
-  CachedTokens Toks;
-  tok::TokenKind EndKind;
-};
-
 /// ConsumeAndStoreInitializer - Consume and store the token at the passed token
 /// container until the end of the current initializer expression (either a
 /// default argument or an in-class initializer for a non-static data member).
@@ -1277,9 +1242,7 @@ bool Parser::ConsumeAndStoreInitializer(CachedTokens &Toks,
       //    syntactically-valid init-declarator-list, then this comma ends
       //    the default initializer.
       {
-        UnannotatedTentativeParsingAction PA(*this,
-                                             CIK == CIK_DefaultInitializer
-                                               ? tok::semi : tok::r_paren);
+        TentativeParsingAction TPA(*this, /*Unannotated=*/true);
         Sema::TentativeAnalysisScope Scope(Actions);
 
         TPResult Result = TPResult::Error;
@@ -1307,7 +1270,7 @@ bool Parser::ConsumeAndStoreInitializer(CachedTokens &Toks,
         // Put the token stream back and undo any annotations we performed
         // after the comma. They may reflect a different parse than the one
         // we will actually perform at the end of the class.
-        PA.RevertAnnotations();
+        TPA.Revert();
 
         // If what follows could be a declaration, it is a declaration.
         if (Result != TPResult::False && Result != TPResult::Error)
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 7ce9a9cea1c7a..46f3baa73d85a 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -6650,48 +6650,67 @@ void Parser::ParseDeclaratorInternal(Declarator &D,
        (Tok.is(tok::identifier) &&
         (NextToken().is(tok::coloncolon) || NextToken().is(tok::less))) ||
        Tok.is(tok::annot_cxxscope))) {
+    TentativeParsingAction TPA(*this, /*Unannotated=*/true);
     bool EnteringContext = D.getContext() == DeclaratorContext::File ||
                            D.getContext() == DeclaratorContext::Member;
+
     CXXScopeSpec SS;
     SS.setTemplateParamLists(D.getTemplateParameterLists());
-    ParseOptionalCXXScopeSpecifier(SS, /*ObjectType=*/nullptr,
-                                   /*ObjectHasErrors=*/false, EnteringContext);
 
-    if (SS.isNotEmpty()) {
-      if (Tok.isNot(tok::star)) {
-        // The scope spec really belongs to the direct-declarator.
-        if (D.mayHaveIdentifier())
-          D.getCXXScopeSpec() = SS;
-        else
-          AnnotateScopeToken(SS, true);
+    if (ParseOptionalCXXScopeSpecifier(SS, /*ObjectType=*/nullptr,
+                                       /*ObjectHasErrors=*/false,
+                                       /*EnteringContext=*/false,
+                                       /*MayBePseudoDestructor=*/nullptr,
+                                       /*IsTypename=*/false, /*LastII=*/nullptr,
+                                       /*OnlyNamespace=*/false,
+                                       /*InUsingDeclaration=*/false,
+                                       /*Disambiguation=*/EnteringContext) ||
+
+        SS.isEmpty() || SS.isInvalid() || !EnteringContext ||
+        Tok.is(tok::star)) {
+      TPA.Commit();
+      if (SS.isNotEmpty() && Tok.is(tok::star)) {
+        if (SS.isValid()) {
+          checkCompoundToken(SS.getEndLoc(), tok::coloncolon,
+                             CompoundToken::MemberPtr);
+        }
 
-        if (DirectDeclParser)
-          (this->*DirectDeclParser)(D);
+        SourceLocation StarLoc = ConsumeToken();
+        D.SetRangeEnd(StarLoc);
+        DeclSpec DS(AttrFactory);
+        ParseTypeQualifierListOpt(DS);
+        D.ExtendWithDeclSpec(DS);
+
+        // Recurse to parse whatever is left.
+        Actions.runWithSufficientStackSpace(D.getBeginLoc(), [&] {
+          ParseDeclaratorInternal(D, DirectDeclParser);
+        });
+
+        // Sema will have to catch (syntactically invalid) pointers into global
+        // scope. It has to catch pointers into namespace scope anyway.
+        D.AddTypeInfo(DeclaratorChunk::getMemberPointer(
+                          SS, DS.getTypeQualifiers(), StarLoc, DS.getEndLoc()),
+                      std::move(DS.getAttributes()),
+                      /*EndLoc=*/SourceLocation());
         return;
       }
+    } else {
+      TPA.Revert();
+      SS.clear();
+      ParseOptionalCXXScopeSpecifier(SS, /*ObjectType=*/nullptr,
+                                     /*ObjectHasErrors=*/false,
+                                     /*EnteringContext=*/true);
+    }
 
-      if (SS.isValid()) {
-        checkCompoundToken(SS.getEndLoc(), tok::coloncolon,
-                           CompoundToken::MemberPtr);
-      }
+    if (SS.isNotEmpty()) {
+      // The scope spec really belongs to the direct-declarator.
+      if (D.mayHaveIdentifier())
+        D.getCXXScopeSpec() = SS;
+      else
+        AnnotateScopeToken(SS, true);
 
-      SourceLocation StarLoc = ConsumeToken();
-      D.SetRangeEnd(StarLoc);
-      DeclSpec DS(AttrFactory);
-      ParseTypeQualifierListOpt(DS);
-      D.ExtendWithDeclSpec(DS);
-
-      // Recurse to parse whatever is left.
-      Actions.runWithSufficientStackSpace(D.getBeginLoc(), [&] {
-        ParseDeclaratorInternal(D, DirectDeclParser);
-      });
-
-      // Sema will have to catch (syntactically invalid) pointers into global
-      // scope. It has to catch pointers into namespace scope anyway.
-      D.AddTypeInfo(DeclaratorChunk::getMemberPointer(
-                        SS, DS.getTypeQualifiers(), StarLoc, DS.getEndLoc()),
-                    std::move(DS.getAttributes()),
-                    /* Don't replace range end. */ SourceLocation());
+      if (DirectDeclParser)
+        (this->*DirectDeclParser)(D);
       return;
     }
   }
diff --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp
index 1d364f77a8146..c0eae73927cdd 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -159,8 +159,8 @@ void Parser::CheckForTemplateAndDigraph(Token &Next, ParsedType ObjectType,
 bool Parser::ParseOptionalCXXScopeSpecifier(
     CXXScopeSpec &SS, ParsedType ObjectType, bool ObjectHadErrors,
     bool EnteringContext, bool *MayBePseudoDestructor, bool IsTypename,
-    const IdentifierInfo **LastII, bool OnlyNamespace,
-    bool InUsingDeclaration) {
+    const IdentifierInfo **LastII, bool OnlyNamespace, bool InUsingDeclaration,
+    bool Disambiguation) {
   assert(getLangOpts().CPlusPlus &&
          "Call sites of this function should be guarded by checking for C++");
 
@@ -528,13 +528,11 @@ bool Parser::ParseOptionalCXXScopeSpecifier(
       UnqualifiedId TemplateName;
       TemplateName.setIdentifier(&II, Tok.getLocation());
       bool MemberOfUnknownSpecialization;
-      if (TemplateNameKind TNK = Actions.isTemplateName(getCurScope(), SS,
-                                              /*hasTemplateKeyword=*/false,
-                                                        TemplateName,
-                                                        ObjectType,
-                                                        EnteringContext,
-                                                        Template,
-                                              MemberOfUnknownSpecialization)) {
+      if (TemplateNameKind TNK = Actions.isTemplateName(
+              getCurScope(), SS,
+              /*hasTemplateKeyword=*/false, TemplateName, ObjectType,
+              EnteringContext, Template, MemberOfUnknownSpecialization,
+              Disambiguation)) {
         // If lookup didn't find anything, we treat the name as a template-name
         // anyway. C++20 requires this, and in prior language modes it improves
         // error recovery. But before we commit to this, check that we actually
@@ -557,7 +555,8 @@ bool Parser::ParseOptionalCXXScopeSpecifier(
         continue;
       }
 
-      if (MemberOfUnknownSpecialization && (ObjectType || SS.isSet()) &&
+      if (MemberOfUnknownSpecialization && !Disambiguation &&
+          (ObjectType || SS.isSet()) &&
           (IsTypename || isTemplateArgumentList(1) == TPResult::True)) {
         // If we had errors before, ObjectType can be dependent even without any
         // templates. Do not report missing template keyword in that case.
diff --git a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.mptr/p2.cpp b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.mptr/p2.cpp
new file mode 100644
index 0000000000000..a06b107755596
--- /dev/null
+++ b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.mptr/p2.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template<typename T>
+struct A0 {
+  struct B0;
+
+  template<typename U>
+  struct C0 {
+    struct D0;
+
+    template<typename V>
+    struct E0;
+  };
+};
+
+template<typename T>
+int A0<T>::B0::* f0();
+
+template<typename T>
+int A0<T>::B1::* f1();
+
+template<typename T>
+int A0<T>::C0<int>::* f2(); // expected-error {{expected unqualified-id}}
+
+template<typename T>
+int A0<T>::C1<int>::* f3(); // expected-error {{no member named 'C1' in 'A0<T>'}}
+                            // expected-error at -1 {{expected ';' after top level declarator}}
+
+template<typename T>
+int A0<T>::template C2<int>::* f4();
+
+template<typename T>
+int A0<T>::template C0<int>::D0::* f5();
+
+template<typename T>
+int A0<T>::template C2<int>::D1::* f6();
+
+template<typename T>
+int A0<T>::template C0<int>::E0<int>::* f7(); // expected-error {{use 'template' keyword to treat 'E0' as a dependent template name}}
+                                              // expected-error at -1 {{expected unqualified-id}}
+
+template<typename T>
+int A0<T>::template C2<int>::E1<int>::* f8(); // expected-error {{no member named 'C2' in 'A0<T>'}}
+
+template<typename T>
+int A0<T>::template C0<int>::template E0<int>::* f9();
+
+template<typename T>
+int A0<T>::template C2<int>::template E1<int>::* f10();
+
+namespace TypoCorrection {
+  template<typename T>
+  struct A {
+    template<typename U>
+    struct Typo; // expected-note {{'Typo' declared here}}
+  };
+
+  template<typename T>
+  int A<T>::template typo<int>::* f();
+
+  template<typename T>
+  int A<T>::typo<int>::* g(); // expected-error {{no template named 'typo' in 'A<T>'; did you mean 'Typo'?}}
+                              // expected-error at -1 {{expected unqualified-id}}
+}
diff --git a/clang/test/CXX/temp/temp.res/p3.cpp b/clang/test/CXX/temp/temp.res/p3.cpp
index 37ab93559e369..1eda967523a59 100644
--- a/clang/test/CXX/temp/temp.res/p3.cpp
+++ b/clang/test/CXX/temp/temp.res/p3.cpp
@@ -2,7 +2,7 @@
 
 template<typename T> struct A {
   template<typename U> struct B;
-  template<typename U> using C = U; // expected-note {{here}}
+  template<typename U> using C = U;
 };
 
 struct X {
@@ -20,12 +20,10 @@ template<typename T> A<T>::C<T> f2(); // expected-warning {{missing 'typename'}}
 template<typename T> A<T>::C<X>::X(T) {}
 template<typename T> A<T>::C<X>::X::Y::Y(T) {}
 
-// FIXME: This is ill-formed
-template<typename T> int A<T>::B<T>::*f3() {}
-template<typename T> int A<T>::C<X>::*f4() {}
+template<typename T> int A<T>::B<T>::*f3() {} // expected-error {{expected unqualified-id}}
+template<typename T> int A<T>::C<X>::*f4() {} // expected-error {{expected unqualified-id}}
 
-// FIXME: This is valid
-template<typename T> int A<T>::template C<int>::*f5() {} // expected-error {{has no members}}
+template<typename T> int A<T>::template C<int>::*f5() {}
 
 template<typename T> template<typename U> struct A<T>::B {
   friend A<T>::C<T> f6(); // ok, same as 'friend T f6();'

>From fde73c10938bbd729ed0558d64691b6b5d2bf003 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Thu, 11 Jul 2024 17:30:10 -0400
Subject: [PATCH 2/8] [FOLD] fix bug when propagating newly lexed tokens from
 current unannotated backtrack

---
 clang/include/clang/Lex/Preprocessor.h |  8 +++--
 clang/lib/Lex/PPCaching.cpp            | 45 ++++++++++++--------------
 2 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index b2dbd7d5375b5..99cd679dd55b0 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -1161,7 +1161,7 @@ class Preprocessor {
   std::vector<CachedTokensTy::size_type> BacktrackPositions;
 
   std::vector<std::pair<CachedTokensTy, CachedTokensTy::size_type>>
-      UnannotatedBacktrackPositions;
+      UnannotatedBacktrackTokens;
 
   /// True if \p Preprocessor::SkipExcludedConditionalBlock() is running.
   /// This is used to guard against calling this function recursively.
@@ -1727,6 +1727,10 @@ class Preprocessor {
   ///
   void EnableBacktrackAtThisPos(bool Unannotated = false);
 
+private:
+  CachedTokensTy PopUnannotatedBacktrackTokens();
+
+public:
   /// Disable the last EnableBacktrackAtThisPos call.
   void CommitBacktrackedTokens();
 
@@ -1739,7 +1743,7 @@ class Preprocessor {
 
   bool isBacktrackEnabled() const { return !BacktrackPositions.empty(); }
   bool isUnannotatedBacktrackEnabled() const {
-    return !UnannotatedBacktrackPositions.empty();
+    return !UnannotatedBacktrackTokens.empty();
   }
 
   /// Lex the next token for this preprocessor.
diff --git a/clang/lib/Lex/PPCaching.cpp b/clang/lib/Lex/PPCaching.cpp
index bc52bfb237e5c..0f8ef733f4c44 100644
--- a/clang/lib/Lex/PPCaching.cpp
+++ b/clang/lib/Lex/PPCaching.cpp
@@ -26,27 +26,31 @@ void Preprocessor::EnableBacktrackAtThisPos(bool Unannotated) {
   assert(LexLevel == 0 && "cannot use lookahead while lexing");
   BacktrackPositions.push_back((CachedLexPos << 1) | Unannotated);
   if (Unannotated)
-    UnannotatedBacktrackPositions.emplace_back(CachedTokens,
-                                               CachedTokens.size());
+    UnannotatedBacktrackTokens.emplace_back(CachedTokens, CachedTokens.size());
   EnterCachingLexMode();
 }
 
+Preprocessor::CachedTokensTy Preprocessor::PopUnannotatedBacktrackTokens() {
+  assert(isUnannotatedBacktrackEnabled() && "missing unannotated tokens?");
+  auto [UnannotatedTokens, NumCachedToks] =
+      std::move(UnannotatedBacktrackTokens.back());
+  UnannotatedBacktrackTokens.pop_back();
+  // If another unannotated backtrack is active, propagate any tokens that were
+  // lexed (not cached) since EnableBacktrackAtThisPos was last called.
+  if (isUnannotatedBacktrackEnabled())
+    UnannotatedBacktrackTokens.back().first.append(
+        UnannotatedTokens.begin() + NumCachedToks, UnannotatedTokens.end());
+  return std::move(UnannotatedTokens);
+}
+
 // Disable the last EnableBacktrackAtThisPos call.
 void Preprocessor::CommitBacktrackedTokens() {
   assert(!BacktrackPositions.empty()
          && "EnableBacktrackAtThisPos was not called!");
   auto BacktrackPos = BacktrackPositions.back();
   BacktrackPositions.pop_back();
-  if (BacktrackPos & 1) {
-    assert(!UnannotatedBacktrackPositions.empty() &&
-           "missing unannotated tokens?");
-    auto [UnannotatedTokens, NumCachedToks] =
-        std::move(UnannotatedBacktrackPositions.back());
-    if (!UnannotatedBacktrackPositions.empty())
-      UnannotatedBacktrackPositions.back().first.append(
-          UnannotatedTokens.begin() + NumCachedToks, UnannotatedTokens.end());
-    UnannotatedBacktrackPositions.pop_back();
-  }
+  if (BacktrackPos & 1)
+    PopUnannotatedBacktrackTokens();
 }
 
 // Make Preprocessor re-lex the tokens that were lexed since
@@ -57,17 +61,8 @@ void Preprocessor::Backtrack() {
   auto BacktrackPos = BacktrackPositions.back();
   BacktrackPositions.pop_back();
   CachedLexPos = BacktrackPos >> 1;
-  if (BacktrackPos & 1) {
-    assert(!UnannotatedBacktrackPositions.empty() &&
-           "missing unannotated tokens?");
-    auto [UnannotatedTokens, NumCachedToks] =
-        std::move(UnannotatedBacktrackPositions.back());
-    UnannotatedBacktrackPositions.pop_back();
-    if (!UnannotatedBacktrackPositions.empty())
-      UnannotatedBacktrackPositions.back().first.append(
-          UnannotatedTokens.begin() + NumCachedToks, UnannotatedTokens.end());
-    CachedTokens = std::move(UnannotatedTokens);
-  }
+  if (BacktrackPos & 1)
+    CachedTokens = PopUnannotatedBacktrackTokens();
   recomputeCurLexerKind();
 }
 
@@ -94,7 +89,7 @@ void Preprocessor::CachingLex(Token &Result) {
     CachedTokens.push_back(Result);
     ++CachedLexPos;
     if (isUnannotatedBacktrackEnabled())
-      UnannotatedBacktrackPositions.back().first.push_back(Result);
+      UnannotatedBacktrackTokens.back().first.push_back(Result);
     return;
   }
 
@@ -137,7 +132,7 @@ const Token &Preprocessor::PeekAhead(unsigned N) {
     CachedTokens.push_back(Token());
     Lex(CachedTokens.back());
     if (isUnannotatedBacktrackEnabled())
-      UnannotatedBacktrackPositions.back().first.push_back(CachedTokens.back());
+      UnannotatedBacktrackTokens.back().first.push_back(CachedTokens.back());
   }
   EnterCachingLexMode();
   return CachedTokens.back();

>From 3cdfbd16fcf6b61a2cfe32d35aaa64f77123a3b4 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Thu, 11 Jul 2024 20:36:21 -0400
Subject: [PATCH 3/8] [FOLD] change expected-error to expected-warning in test

---
 clang/test/CXX/dcl.decl/dcl.meaning/dcl.mptr/p2.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.mptr/p2.cpp b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.mptr/p2.cpp
index a06b107755596..33fb2b5fa82d8 100644
--- a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.mptr/p2.cpp
+++ b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.mptr/p2.cpp
@@ -36,7 +36,7 @@ template<typename T>
 int A0<T>::template C2<int>::D1::* f6();
 
 template<typename T>
-int A0<T>::template C0<int>::E0<int>::* f7(); // expected-error {{use 'template' keyword to treat 'E0' as a dependent template name}}
+int A0<T>::template C0<int>::E0<int>::* f7(); // expected-warning {{use 'template' keyword to treat 'E0' as a dependent template name}}
                                               // expected-error at -1 {{expected unqualified-id}}
 
 template<typename T>

>From 558ef7aac254d7cc643ec141e43c1454bfe1c71c Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Wed, 17 Jul 2024 08:59:27 -0400
Subject: [PATCH 4/8] [FOLD] update test

---
 clang/test/CXX/dcl.decl/dcl.meaning/dcl.mptr/p2.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.mptr/p2.cpp b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.mptr/p2.cpp
index 33fb2b5fa82d8..a06b107755596 100644
--- a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.mptr/p2.cpp
+++ b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.mptr/p2.cpp
@@ -36,7 +36,7 @@ template<typename T>
 int A0<T>::template C2<int>::D1::* f6();
 
 template<typename T>
-int A0<T>::template C0<int>::E0<int>::* f7(); // expected-warning {{use 'template' keyword to treat 'E0' as a dependent template name}}
+int A0<T>::template C0<int>::E0<int>::* f7(); // expected-error {{use 'template' keyword to treat 'E0' as a dependent template name}}
                                               // expected-error at -1 {{expected unqualified-id}}
 
 template<typename T>

>From b060aa9f228d74eb710dcb6680c4e99646b5527b Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Tue, 23 Jul 2024 07:03:21 -0400
Subject: [PATCH 5/8] [FOLD] minor cleanup

---
 clang/include/clang/Lex/Preprocessor.h | 2 +-
 clang/lib/Parse/ParseDecl.cpp          | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index 99cd679dd55b0..f7f3995a550ca 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -1740,8 +1740,8 @@ class Preprocessor {
 
   /// True if EnableBacktrackAtThisPos() was called and
   /// caching of tokens is on.
-
   bool isBacktrackEnabled() const { return !BacktrackPositions.empty(); }
+
   bool isUnannotatedBacktrackEnabled() const {
     return !UnannotatedBacktrackTokens.empty();
   }
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 46f3baa73d85a..8d7d37c75134e 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -6653,7 +6653,6 @@ void Parser::ParseDeclaratorInternal(Declarator &D,
     TentativeParsingAction TPA(*this, /*Unannotated=*/true);
     bool EnteringContext = D.getContext() == DeclaratorContext::File ||
                            D.getContext() == DeclaratorContext::Member;
-
     CXXScopeSpec SS;
     SS.setTemplateParamLists(D.getTemplateParameterLists());
 

>From 130840a24249bf67713fc19c337248f04c6d2017 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Tue, 23 Jul 2024 07:19:03 -0400
Subject: [PATCH 6/8] [TEST] different representation of unannotated backtrack

---
 clang/include/clang/Lex/Preprocessor.h |  4 +++-
 clang/lib/Lex/PPCaching.cpp            | 30 +++++++++++++++-----------
 clang/lib/Lex/Preprocessor.cpp         |  2 +-
 3 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index f7f3995a550ca..5b8eda12cb858 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -1728,6 +1728,8 @@ class Preprocessor {
   void EnableBacktrackAtThisPos(bool Unannotated = false);
 
 private:
+  std::pair<CachedTokensTy::size_type, bool> LastBacktrackPos();
+
   CachedTokensTy PopUnannotatedBacktrackTokens();
 
 public:
@@ -1853,7 +1855,7 @@ class Preprocessor {
     assert(isBacktrackEnabled() &&
            "Should only be called when tokens are cached for backtracking");
     assert(signed(CachedLexPos) - signed(N) >=
-               signed(BacktrackPositions.back() >> 1) &&
+               signed(LastBacktrackPos().first) &&
            "Should revert tokens up to the last backtrack position, not more");
     assert(signed(CachedLexPos) - signed(N) >= 0 &&
            "Corrupted backtrack positions ?");
diff --git a/clang/lib/Lex/PPCaching.cpp b/clang/lib/Lex/PPCaching.cpp
index 0f8ef733f4c44..cbacda9d31ae2 100644
--- a/clang/lib/Lex/PPCaching.cpp
+++ b/clang/lib/Lex/PPCaching.cpp
@@ -14,6 +14,15 @@
 #include "clang/Lex/Preprocessor.h"
 using namespace clang;
 
+std::pair<Preprocessor::CachedTokensTy::size_type, bool>
+Preprocessor::LastBacktrackPos() {
+  assert(isBacktrackEnabled());
+  auto BacktrackPos = BacktrackPositions.back();
+  bool Unannotated =
+      static_cast<CachedTokensTy::difference_type>(BacktrackPos) < 0;
+  return {Unannotated ? ~BacktrackPos : BacktrackPos, Unannotated};
+}
+
 // EnableBacktrackAtThisPos - From the point that this method is called, and
 // until CommitBacktrackedTokens() or Backtrack() is called, the Preprocessor
 // keeps track of the lexed tokens so that a subsequent Backtrack() call will
@@ -24,7 +33,7 @@ using namespace clang;
 // be combined with the EnableBacktrackAtThisPos calls in reverse order.
 void Preprocessor::EnableBacktrackAtThisPos(bool Unannotated) {
   assert(LexLevel == 0 && "cannot use lookahead while lexing");
-  BacktrackPositions.push_back((CachedLexPos << 1) | Unannotated);
+  BacktrackPositions.push_back(Unannotated ? ~CachedLexPos : CachedLexPos);
   if (Unannotated)
     UnannotatedBacktrackTokens.emplace_back(CachedTokens, CachedTokens.size());
   EnterCachingLexMode();
@@ -45,23 +54,21 @@ Preprocessor::CachedTokensTy Preprocessor::PopUnannotatedBacktrackTokens() {
 
 // Disable the last EnableBacktrackAtThisPos call.
 void Preprocessor::CommitBacktrackedTokens() {
-  assert(!BacktrackPositions.empty()
-         && "EnableBacktrackAtThisPos was not called!");
-  auto BacktrackPos = BacktrackPositions.back();
+  assert(isBacktrackEnabled() && "EnableBacktrackAtThisPos was not called!");
+  auto [BacktrackPos, Unannotated] = LastBacktrackPos();
   BacktrackPositions.pop_back();
-  if (BacktrackPos & 1)
+  if (Unannotated)
     PopUnannotatedBacktrackTokens();
 }
 
 // Make Preprocessor re-lex the tokens that were lexed since
 // EnableBacktrackAtThisPos() was previously called.
 void Preprocessor::Backtrack() {
-  assert(!BacktrackPositions.empty()
-         && "EnableBacktrackAtThisPos was not called!");
-  auto BacktrackPos = BacktrackPositions.back();
+  assert(isBacktrackEnabled() && "EnableBacktrackAtThisPos was not called!");
+  auto [BacktrackPos, Unannotated] = LastBacktrackPos();
   BacktrackPositions.pop_back();
-  CachedLexPos = BacktrackPos >> 1;
-  if (BacktrackPos & 1)
+  CachedLexPos = BacktrackPos;
+  if (Unannotated)
     CachedTokens = PopUnannotatedBacktrackTokens();
   recomputeCurLexerKind();
 }
@@ -149,8 +156,7 @@ void Preprocessor::AnnotatePreviousCachedTokens(const Token &Tok) {
   for (CachedTokensTy::size_type i = CachedLexPos; i != 0; --i) {
     CachedTokensTy::iterator AnnotBegin = CachedTokens.begin() + i-1;
     if (AnnotBegin->getLocation() == Tok.getLocation()) {
-      assert((BacktrackPositions.empty() ||
-              (BacktrackPositions.back() >> 1) <= i) &&
+      assert((!isBacktrackEnabled() || LastBacktrackPos().first <= i) &&
              "The backtrack pos points inside the annotated tokens!");
       // Replace the cached tokens with the single annotation token.
       if (i < CachedLexPos)
diff --git a/clang/lib/Lex/Preprocessor.cpp b/clang/lib/Lex/Preprocessor.cpp
index 63e27e62cffc8..f0b4593e0cc22 100644
--- a/clang/lib/Lex/Preprocessor.cpp
+++ b/clang/lib/Lex/Preprocessor.cpp
@@ -170,7 +170,7 @@ Preprocessor::Preprocessor(std::shared_ptr<PreprocessorOptions> PPOpts,
 }
 
 Preprocessor::~Preprocessor() {
-  assert(BacktrackPositions.empty() && "EnableBacktrack/Backtrack imbalance!");
+  assert(!isBacktrackEnabled() && "EnableBacktrack/Backtrack imbalance!");
 
   IncludeMacroStack.clear();
 

>From a311192c138e127da5caa1a73a1ecf235aaeb2e5 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Mon, 29 Jul 2024 13:02:15 -0400
Subject: [PATCH 7/8] [FOLD] add comments

---
 clang/include/clang/Lex/Preprocessor.h | 6 ++++++
 clang/include/clang/Parse/Parser.h     | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index 5b8eda12cb858..623f868ca1e64 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -1160,6 +1160,8 @@ class Preprocessor {
   /// invoked (at which point the last position is popped).
   std::vector<CachedTokensTy::size_type> BacktrackPositions;
 
+  /// Stack of cached tokens/initial number of cached tokens pairs, allowing
+  /// nested unannotated backtracks.
   std::vector<std::pair<CachedTokensTy, CachedTokensTy::size_type>>
       UnannotatedBacktrackTokens;
 
@@ -1725,6 +1727,8 @@ class Preprocessor {
   /// at some point after EnableBacktrackAtThisPos. If you don't, caching of
   /// tokens will continue indefinitely.
   ///
+  /// \param Unannotated Whether token annotations are reverted upon calling
+  /// Backtrack().
   void EnableBacktrackAtThisPos(bool Unannotated = false);
 
 private:
@@ -1744,6 +1748,8 @@ class Preprocessor {
   /// caching of tokens is on.
   bool isBacktrackEnabled() const { return !BacktrackPositions.empty(); }
 
+  /// True if EnableBacktrackAtThisPos() was called and
+  /// caching of unannotated tokens is on.
   bool isUnannotatedBacktrackEnabled() const {
     return !UnannotatedBacktrackTokens.empty();
   }
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 7f5e21127d1e5..ba7d6866ebacd 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1025,6 +1025,8 @@ class Parser : public CodeCompletionHandler {
   ///   ....
   ///   TPA.Revert();
   ///
+  /// If the Unannotated parameter is true, any token annotations created
+  /// during the tentative parse are reverted.
   class TentativeParsingAction {
     Parser &P;
     PreferredTypeBuilder PrevPreferredType;

>From 2603c38500b9776e811156aa75b972802bc47489 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Mon, 29 Jul 2024 13:05:02 -0400
Subject: [PATCH 8/8] [FOLD] add release note

---
 clang/docs/ReleaseNotes.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index dad44f45a847f..43e67f26461e2 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -164,6 +164,7 @@ Bug Fixes to C++ Support
 - Fixed a crash when an expression with a dependent ``__typeof__`` type is used as the operand of a unary operator. (#GH97646)
 - Fixed a failed assertion when checking invalid delete operator declaration. (#GH96191)
 - Fix a crash when checking destructor reference with an invalid initializer. (#GH97230)
+- Clang now correctly parses potentially declarative nested-name-specifiers in pointer-to-member declarators.
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^



More information about the cfe-commits mailing list