Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit ade60c6

Browse files
sdkrystiansdkrystian
committedJul 11, 2024
[Clang][Parse] Fix ambiguity with nested-name-specifiers that may be declarative
1 parent 6c8ff4c commit ade60c6

File tree

8 files changed

+178
-96
lines changed

8 files changed

+178
-96
lines changed
 

‎clang/include/clang/Lex/Preprocessor.h

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1160,6 +1160,9 @@ class Preprocessor {
11601160
/// invoked (at which point the last position is popped).
11611161
std::vector<CachedTokensTy::size_type> BacktrackPositions;
11621162

1163+
std::vector<std::pair<CachedTokensTy, CachedTokensTy::size_type>>
1164+
UnannotatedBacktrackPositions;
1165+
11631166
/// True if \p Preprocessor::SkipExcludedConditionalBlock() is running.
11641167
/// This is used to guard against calling this function recursively.
11651168
///
@@ -1722,7 +1725,7 @@ class Preprocessor {
17221725
/// at some point after EnableBacktrackAtThisPos. If you don't, caching of
17231726
/// tokens will continue indefinitely.
17241727
///
1725-
void EnableBacktrackAtThisPos();
1728+
void EnableBacktrackAtThisPos(bool Unannotated = false);
17261729

17271730
/// Disable the last EnableBacktrackAtThisPos call.
17281731
void CommitBacktrackedTokens();
@@ -1733,7 +1736,11 @@ class Preprocessor {
17331736

17341737
/// True if EnableBacktrackAtThisPos() was called and
17351738
/// caching of tokens is on.
1739+
17361740
bool isBacktrackEnabled() const { return !BacktrackPositions.empty(); }
1741+
bool isUnannotatedBacktrackEnabled() const {
1742+
return !UnannotatedBacktrackPositions.empty();
1743+
}
17371744

17381745
/// Lex the next token for this preprocessor.
17391746
void Lex(Token &Result);
@@ -1841,8 +1848,9 @@ class Preprocessor {
18411848
void RevertCachedTokens(unsigned N) {
18421849
assert(isBacktrackEnabled() &&
18431850
"Should only be called when tokens are cached for backtracking");
1844-
assert(signed(CachedLexPos) - signed(N) >= signed(BacktrackPositions.back())
1845-
&& "Should revert tokens up to the last backtrack position, not more");
1851+
assert(signed(CachedLexPos) - signed(N) >=
1852+
signed(BacktrackPositions.back() >> 1) &&
1853+
"Should revert tokens up to the last backtrack position, not more");
18461854
assert(signed(CachedLexPos) - signed(N) >= 0 &&
18471855
"Corrupted backtrack positions ?");
18481856
CachedLexPos -= N;

‎clang/include/clang/Parse/Parser.h

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1033,15 +1033,15 @@ class Parser : public CodeCompletionHandler {
10331033
bool isActive;
10341034

10351035
public:
1036-
explicit TentativeParsingAction(Parser &p)
1036+
explicit TentativeParsingAction(Parser &p, bool Unannotated = false)
10371037
: P(p), PrevPreferredType(P.PreferredType) {
10381038
PrevTok = P.Tok;
10391039
PrevTentativelyDeclaredIdentifierCount =
10401040
P.TentativelyDeclaredIdentifiers.size();
10411041
PrevParenCount = P.ParenCount;
10421042
PrevBracketCount = P.BracketCount;
10431043
PrevBraceCount = P.BraceCount;
1044-
P.PP.EnableBacktrackAtThisPos();
1044+
P.PP.EnableBacktrackAtThisPos(Unannotated);
10451045
isActive = true;
10461046
}
10471047
void Commit() {
@@ -1072,13 +1072,11 @@ class Parser : public CodeCompletionHandler {
10721072
class RevertingTentativeParsingAction
10731073
: private Parser::TentativeParsingAction {
10741074
public:
1075-
RevertingTentativeParsingAction(Parser &P)
1076-
: Parser::TentativeParsingAction(P) {}
1075+
using TentativeParsingAction::TentativeParsingAction;
1076+
10771077
~RevertingTentativeParsingAction() { Revert(); }
10781078
};
10791079

1080-
class UnannotatedTentativeParsingAction;
1081-
10821080
/// ObjCDeclContextSwitch - An object used to switch context from
10831081
/// an objective-c decl context to its enclosing decl context and
10841082
/// back.
@@ -1979,7 +1977,8 @@ class Parser : public CodeCompletionHandler {
19791977
CXXScopeSpec &SS, ParsedType ObjectType, bool ObjectHasErrors,
19801978
bool EnteringContext, bool *MayBePseudoDestructor = nullptr,
19811979
bool IsTypename = false, const IdentifierInfo **LastII = nullptr,
1982-
bool OnlyNamespace = false, bool InUsingDeclaration = false);
1980+
bool OnlyNamespace = false, bool InUsingDeclaration = false,
1981+
bool Disambiguation = false);
19831982

19841983
//===--------------------------------------------------------------------===//
19851984
// C++11 5.1.2: Lambda expressions

‎clang/lib/Lex/PPCaching.cpp

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,26 +22,52 @@ using namespace clang;
2222
// Nested backtracks are allowed, meaning that EnableBacktrackAtThisPos can
2323
// be called multiple times and CommitBacktrackedTokens/Backtrack calls will
2424
// be combined with the EnableBacktrackAtThisPos calls in reverse order.
25-
void Preprocessor::EnableBacktrackAtThisPos() {
25+
void Preprocessor::EnableBacktrackAtThisPos(bool Unannotated) {
2626
assert(LexLevel == 0 && "cannot use lookahead while lexing");
27-
BacktrackPositions.push_back(CachedLexPos);
27+
BacktrackPositions.push_back((CachedLexPos << 1) | Unannotated);
28+
if (Unannotated)
29+
UnannotatedBacktrackPositions.emplace_back(CachedTokens,
30+
CachedTokens.size());
2831
EnterCachingLexMode();
2932
}
3033

3134
// Disable the last EnableBacktrackAtThisPos call.
3235
void Preprocessor::CommitBacktrackedTokens() {
3336
assert(!BacktrackPositions.empty()
3437
&& "EnableBacktrackAtThisPos was not called!");
38+
auto BacktrackPos = BacktrackPositions.back();
3539
BacktrackPositions.pop_back();
40+
if (BacktrackPos & 1) {
41+
assert(!UnannotatedBacktrackPositions.empty() &&
42+
"missing unannotated tokens?");
43+
auto [UnannotatedTokens, NumCachedToks] =
44+
std::move(UnannotatedBacktrackPositions.back());
45+
if (!UnannotatedBacktrackPositions.empty())
46+
UnannotatedBacktrackPositions.back().first.append(
47+
UnannotatedTokens.begin() + NumCachedToks, UnannotatedTokens.end());
48+
UnannotatedBacktrackPositions.pop_back();
49+
}
3650
}
3751

3852
// Make Preprocessor re-lex the tokens that were lexed since
3953
// EnableBacktrackAtThisPos() was previously called.
4054
void Preprocessor::Backtrack() {
4155
assert(!BacktrackPositions.empty()
4256
&& "EnableBacktrackAtThisPos was not called!");
43-
CachedLexPos = BacktrackPositions.back();
57+
auto BacktrackPos = BacktrackPositions.back();
4458
BacktrackPositions.pop_back();
59+
CachedLexPos = BacktrackPos >> 1;
60+
if (BacktrackPos & 1) {
61+
assert(!UnannotatedBacktrackPositions.empty() &&
62+
"missing unannotated tokens?");
63+
auto [UnannotatedTokens, NumCachedToks] =
64+
std::move(UnannotatedBacktrackPositions.back());
65+
UnannotatedBacktrackPositions.pop_back();
66+
if (!UnannotatedBacktrackPositions.empty())
67+
UnannotatedBacktrackPositions.back().first.append(
68+
UnannotatedTokens.begin() + NumCachedToks, UnannotatedTokens.end());
69+
CachedTokens = std::move(UnannotatedTokens);
70+
}
4571
recomputeCurLexerKind();
4672
}
4773

@@ -67,6 +93,8 @@ void Preprocessor::CachingLex(Token &Result) {
6793
EnterCachingLexModeUnchecked();
6894
CachedTokens.push_back(Result);
6995
++CachedLexPos;
96+
if (isUnannotatedBacktrackEnabled())
97+
UnannotatedBacktrackPositions.back().first.push_back(Result);
7098
return;
7199
}
72100

@@ -108,6 +136,8 @@ const Token &Preprocessor::PeekAhead(unsigned N) {
108136
for (size_t C = CachedLexPos + N - CachedTokens.size(); C > 0; --C) {
109137
CachedTokens.push_back(Token());
110138
Lex(CachedTokens.back());
139+
if (isUnannotatedBacktrackEnabled())
140+
UnannotatedBacktrackPositions.back().first.push_back(CachedTokens.back());
111141
}
112142
EnterCachingLexMode();
113143
return CachedTokens.back();
@@ -124,7 +154,8 @@ void Preprocessor::AnnotatePreviousCachedTokens(const Token &Tok) {
124154
for (CachedTokensTy::size_type i = CachedLexPos; i != 0; --i) {
125155
CachedTokensTy::iterator AnnotBegin = CachedTokens.begin() + i-1;
126156
if (AnnotBegin->getLocation() == Tok.getLocation()) {
127-
assert((BacktrackPositions.empty() || BacktrackPositions.back() <= i) &&
157+
assert((BacktrackPositions.empty() ||
158+
(BacktrackPositions.back() >> 1) <= i) &&
128159
"The backtrack pos points inside the annotated tokens!");
129160
// Replace the cached tokens with the single annotation token.
130161
if (i < CachedLexPos)

‎clang/lib/Parse/ParseCXXInlineMethods.cpp

Lines changed: 2 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1188,41 +1188,6 @@ bool Parser::ConsumeAndStoreConditional(CachedTokens &Toks) {
11881188
return true;
11891189
}
11901190

1191-
/// A tentative parsing action that can also revert token annotations.
1192-
class Parser::UnannotatedTentativeParsingAction : public TentativeParsingAction {
1193-
public:
1194-
explicit UnannotatedTentativeParsingAction(Parser &Self,
1195-
tok::TokenKind EndKind)
1196-
: TentativeParsingAction(Self), Self(Self), EndKind(EndKind) {
1197-
// Stash away the old token stream, so we can restore it once the
1198-
// tentative parse is complete.
1199-
TentativeParsingAction Inner(Self);
1200-
Self.ConsumeAndStoreUntil(EndKind, Toks, true, /*ConsumeFinalToken*/false);
1201-
Inner.Revert();
1202-
}
1203-
1204-
void RevertAnnotations() {
1205-
Revert();
1206-
1207-
// Put back the original tokens.
1208-
Self.SkipUntil(EndKind, StopAtSemi | StopBeforeMatch);
1209-
if (Toks.size()) {
1210-
auto Buffer = std::make_unique<Token[]>(Toks.size());
1211-
std::copy(Toks.begin() + 1, Toks.end(), Buffer.get());
1212-
Buffer[Toks.size() - 1] = Self.Tok;
1213-
Self.PP.EnterTokenStream(std::move(Buffer), Toks.size(), true,
1214-
/*IsReinject*/ true);
1215-
1216-
Self.Tok = Toks.front();
1217-
}
1218-
}
1219-
1220-
private:
1221-
Parser &Self;
1222-
CachedTokens Toks;
1223-
tok::TokenKind EndKind;
1224-
};
1225-
12261191
/// ConsumeAndStoreInitializer - Consume and store the token at the passed token
12271192
/// container until the end of the current initializer expression (either a
12281193
/// default argument or an in-class initializer for a non-static data member).
@@ -1260,9 +1225,7 @@ bool Parser::ConsumeAndStoreInitializer(CachedTokens &Toks,
12601225
// syntactically-valid init-declarator-list, then this comma ends
12611226
// the default initializer.
12621227
{
1263-
UnannotatedTentativeParsingAction PA(*this,
1264-
CIK == CIK_DefaultInitializer
1265-
? tok::semi : tok::r_paren);
1228+
TentativeParsingAction TPA(*this, /*Unannotated=*/true);
12661229
Sema::TentativeAnalysisScope Scope(Actions);
12671230

12681231
TPResult Result = TPResult::Error;
@@ -1290,7 +1253,7 @@ bool Parser::ConsumeAndStoreInitializer(CachedTokens &Toks,
12901253
// Put the token stream back and undo any annotations we performed
12911254
// after the comma. They may reflect a different parse than the one
12921255
// we will actually perform at the end of the class.
1293-
PA.RevertAnnotations();
1256+
TPA.Revert();
12941257

12951258
// If what follows could be a declaration, it is a declaration.
12961259
if (Result != TPResult::False && Result != TPResult::Error)

‎clang/lib/Parse/ParseDecl.cpp

Lines changed: 51 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -6650,48 +6650,67 @@ void Parser::ParseDeclaratorInternal(Declarator &D,
66506650
(Tok.is(tok::identifier) &&
66516651
(NextToken().is(tok::coloncolon) || NextToken().is(tok::less))) ||
66526652
Tok.is(tok::annot_cxxscope))) {
6653+
TentativeParsingAction TPA(*this, /*Unannotated=*/true);
66536654
bool EnteringContext = D.getContext() == DeclaratorContext::File ||
66546655
D.getContext() == DeclaratorContext::Member;
6656+
66556657
CXXScopeSpec SS;
66566658
SS.setTemplateParamLists(D.getTemplateParameterLists());
6657-
ParseOptionalCXXScopeSpecifier(SS, /*ObjectType=*/nullptr,
6658-
/*ObjectHasErrors=*/false, EnteringContext);
66596659

6660-
if (SS.isNotEmpty()) {
6661-
if (Tok.isNot(tok::star)) {
6662-
// The scope spec really belongs to the direct-declarator.
6663-
if (D.mayHaveIdentifier())
6664-
D.getCXXScopeSpec() = SS;
6665-
else
6666-
AnnotateScopeToken(SS, true);
6660+
if (ParseOptionalCXXScopeSpecifier(SS, /*ObjectType=*/nullptr,
6661+
/*ObjectHasErrors=*/false,
6662+
/*EnteringContext=*/false,
6663+
/*MayBePseudoDestructor=*/nullptr,
6664+
/*IsTypename=*/false, /*LastII=*/nullptr,
6665+
/*OnlyNamespace=*/false,
6666+
/*InUsingDeclaration=*/false,
6667+
/*Disambiguation=*/EnteringContext) ||
6668+
6669+
SS.isEmpty() || SS.isInvalid() || !EnteringContext ||
6670+
Tok.is(tok::star)) {
6671+
TPA.Commit();
6672+
if (SS.isNotEmpty() && Tok.is(tok::star)) {
6673+
if (SS.isValid()) {
6674+
checkCompoundToken(SS.getEndLoc(), tok::coloncolon,
6675+
CompoundToken::MemberPtr);
6676+
}
66676677

6668-
if (DirectDeclParser)
6669-
(this->*DirectDeclParser)(D);
6678+
SourceLocation StarLoc = ConsumeToken();
6679+
D.SetRangeEnd(StarLoc);
6680+
DeclSpec DS(AttrFactory);
6681+
ParseTypeQualifierListOpt(DS);
6682+
D.ExtendWithDeclSpec(DS);
6683+
6684+
// Recurse to parse whatever is left.
6685+
Actions.runWithSufficientStackSpace(D.getBeginLoc(), [&] {
6686+
ParseDeclaratorInternal(D, DirectDeclParser);
6687+
});
6688+
6689+
// Sema will have to catch (syntactically invalid) pointers into global
6690+
// scope. It has to catch pointers into namespace scope anyway.
6691+
D.AddTypeInfo(DeclaratorChunk::getMemberPointer(
6692+
SS, DS.getTypeQualifiers(), StarLoc, DS.getEndLoc()),
6693+
std::move(DS.getAttributes()),
6694+
/*EndLoc=*/SourceLocation());
66706695
return;
66716696
}
6697+
} else {
6698+
TPA.Revert();
6699+
SS.clear();
6700+
ParseOptionalCXXScopeSpecifier(SS, /*ObjectType=*/nullptr,
6701+
/*ObjectHasErrors=*/false,
6702+
/*EnteringContext=*/true);
6703+
}
66726704

6673-
if (SS.isValid()) {
6674-
checkCompoundToken(SS.getEndLoc(), tok::coloncolon,
6675-
CompoundToken::MemberPtr);
6676-
}
6705+
if (SS.isNotEmpty()) {
6706+
// The scope spec really belongs to the direct-declarator.
6707+
if (D.mayHaveIdentifier())
6708+
D.getCXXScopeSpec() = SS;
6709+
else
6710+
AnnotateScopeToken(SS, true);
66776711

6678-
SourceLocation StarLoc = ConsumeToken();
6679-
D.SetRangeEnd(StarLoc);
6680-
DeclSpec DS(AttrFactory);
6681-
ParseTypeQualifierListOpt(DS);
6682-
D.ExtendWithDeclSpec(DS);
6683-
6684-
// Recurse to parse whatever is left.
6685-
Actions.runWithSufficientStackSpace(D.getBeginLoc(), [&] {
6686-
ParseDeclaratorInternal(D, DirectDeclParser);
6687-
});
6688-
6689-
// Sema will have to catch (syntactically invalid) pointers into global
6690-
// scope. It has to catch pointers into namespace scope anyway.
6691-
D.AddTypeInfo(DeclaratorChunk::getMemberPointer(
6692-
SS, DS.getTypeQualifiers(), StarLoc, DS.getEndLoc()),
6693-
std::move(DS.getAttributes()),
6694-
/* Don't replace range end. */ SourceLocation());
6712+
if (DirectDeclParser)
6713+
(this->*DirectDeclParser)(D);
66956714
return;
66966715
}
66976716
}

‎clang/lib/Parse/ParseExprCXX.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,8 @@ void Parser::CheckForTemplateAndDigraph(Token &Next, ParsedType ObjectType,
160160
bool Parser::ParseOptionalCXXScopeSpecifier(
161161
CXXScopeSpec &SS, ParsedType ObjectType, bool ObjectHadErrors,
162162
bool EnteringContext, bool *MayBePseudoDestructor, bool IsTypename,
163-
const IdentifierInfo **LastII, bool OnlyNamespace,
164-
bool InUsingDeclaration) {
163+
const IdentifierInfo **LastII, bool OnlyNamespace, bool InUsingDeclaration,
164+
bool Disambiguation) {
165165
assert(getLangOpts().CPlusPlus &&
166166
"Call sites of this function should be guarded by checking for C++");
167167

@@ -533,8 +533,7 @@ bool Parser::ParseOptionalCXXScopeSpecifier(
533533
getCurScope(), SS,
534534
/*hasTemplateKeyword=*/false, TemplateName, ObjectType,
535535
EnteringContext, Template, MemberOfUnknownSpecialization,
536-
/*Disambiguation=*/false,
537-
/*MayBeNNS=*/true)) {
536+
Disambiguation, /*MayBeNNS=*/true)) {
538537
// If lookup didn't find anything, we treat the name as a template-name
539538
// anyway. C++20 requires this, and in prior language modes it improves
540539
// error recovery. But before we commit to this, check that we actually
@@ -559,7 +558,8 @@ bool Parser::ParseOptionalCXXScopeSpecifier(
559558
continue;
560559
}
561560

562-
if (MemberOfUnknownSpecialization && (ObjectType || SS.isSet()) &&
561+
if (MemberOfUnknownSpecialization && !Disambiguation &&
562+
(ObjectType || SS.isSet()) &&
563563
(IsTypename || isTemplateArgumentList(1) == TPResult::True)) {
564564
// If we had errors before, ObjectType can be dependent even without any
565565
// templates. Do not report missing template keyword in that case.

‎clang/test/CXX/dcl.decl/dcl.meaning/dcl.mptr/p2.cpp

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
// RUN: %clang_cc1 -fsyntax-only -verify %s
2+
3+
template<typename T>
4+
struct A0 {
5+
struct B0;
6+
7+
template<typename U>
8+
struct C0 {
9+
struct D0;
10+
11+
template<typename V>
12+
struct E0;
13+
};
14+
};
15+
16+
template<typename T>
17+
int A0<T>::B0::* f0();
18+
19+
template<typename T>
20+
int A0<T>::B1::* f1();
21+
22+
template<typename T>
23+
int A0<T>::C0<int>::* f2(); // expected-error {{expected unqualified-id}}
24+
25+
template<typename T>
26+
int A0<T>::C1<int>::* f3(); // expected-error {{no member named 'C1' in 'A0<T>'}}
27+
// expected-error@-1 {{expected ';' after top level declarator}}
28+
29+
template<typename T>
30+
int A0<T>::template C2<int>::* f4();
31+
32+
template<typename T>
33+
int A0<T>::template C0<int>::D0::* f5();
34+
35+
template<typename T>
36+
int A0<T>::template C2<int>::D1::* f6();
37+
38+
template<typename T>
39+
int A0<T>::template C0<int>::E0<int>::* f7(); // expected-error {{use 'template' keyword to treat 'E0' as a dependent template name}}
40+
// expected-error@-1 {{expected unqualified-id}}
41+
42+
template<typename T>
43+
int A0<T>::template C2<int>::E1<int>::* f8(); // expected-error {{no member named 'C2' in 'A0<T>'}}
44+
45+
template<typename T>
46+
int A0<T>::template C0<int>::template E0<int>::* f9();
47+
48+
template<typename T>
49+
int A0<T>::template C2<int>::template E1<int>::* f10();
50+
51+
namespace TypoCorrection {
52+
template<typename T>
53+
struct A {
54+
template<typename U>
55+
struct Typo; // expected-note {{'Typo' declared here}}
56+
};
57+
58+
template<typename T>
59+
int A<T>::template typo<int>::* f();
60+
61+
template<typename T>
62+
int A<T>::typo<int>::* g(); // expected-error {{no template named 'typo' in 'A<T>'; did you mean 'Typo'?}}
63+
// expected-error@-1 {{expected unqualified-id}}
64+
}

‎clang/test/CXX/temp/temp.res/p3.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
template<typename T> struct A {
44
template<typename U> struct B;
5-
template<typename U> using C = U; // expected-note {{here}}
5+
template<typename U> using C = U;
66
};
77

88
struct X {
@@ -20,12 +20,10 @@ template<typename T> A<T>::C<T> f2(); // expected-warning {{missing 'typename'}}
2020
template<typename T> A<T>::C<X>::X(T) {}
2121
template<typename T> A<T>::C<X>::X::Y::Y(T) {}
2222

23-
// FIXME: This is ill-formed
24-
template<typename T> int A<T>::B<T>::*f3() {}
25-
template<typename T> int A<T>::C<X>::*f4() {}
23+
template<typename T> int A<T>::B<T>::*f3() {} // expected-error {{expected unqualified-id}}
24+
template<typename T> int A<T>::C<X>::*f4() {} // expected-error {{expected unqualified-id}}
2625

27-
// FIXME: This is valid
28-
template<typename T> int A<T>::template C<int>::*f5() {} // expected-error {{has no members}}
26+
template<typename T> int A<T>::template C<int>::*f5() {}
2927

3028
template<typename T> template<typename U> struct A<T>::B {
3129
friend A<T>::C<T> f6(); // ok, same as 'friend T f6();'

0 commit comments

Comments
 (0)
Please sign in to comment.