[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
sdkrystian wrote: This does seem to cause an error in libstdc++: ``` include/c++/11/bits/shared_ptr.h:365:10: error: use 'template' keyword to treat '__shared_ptr' as a dependent template name this->__shared_ptr<_Tp>::operator=(__r); ^ ``` Since `this` is dependent (it's of type `shared_ptr*`) and `shared_ptr` inherits from `__shared_ptr` (i.e. it has a dependent base), `template` is required to treat `<` as the start of a template argument list. I think we can detect these cases and diagnose this as a language extension. https://github.com/llvm/llvm-project/pull/92957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
https://github.com/sdkrystian edited https://github.com/llvm/llvm-project/pull/92957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
sdkrystian wrote: > > Per [[basic.lookup.qual.general] > > p1](http://eel.is/c++draft/basic.lookup.qual.general#1), lookup for a > > member-qualified name is type-only if it's an identifier followed by ::; > > Per my reading, type-only lookup is performed only for elaborated type > specifiers (http://eel.is/c++draft/basic.lookup#elab-1.sentence-1) and > base-specifiers (http://eel.is/c++draft/class.derived#general-2.sentence-4). > What [basic.lookup.qual.general] p1 describes is much like type-only lookup, > but namespaces are considered. Another important difference is that > http://eel.is/c++draft/basic.lookup#general-4.sentence-2 does apply to lookup > of identifiers followed by `::`. > > Not saying that this materially changes anything, but we should be more > cautious with terminology. Sorry, I used "type-only" there but I intended to write "only considers namespaces, types, and templates whose specializations are types". I'll edit my comment to fix that. The intended phrasing is what I implemented. https://github.com/llvm/llvm-project/pull/92957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
Endilll wrote: > This means that we must perform the second (unqualified) lookup during > parsing even when the type of the object expression is dependent, but those > results are not used to determine whether a < token is the start of a > template-argument_list; they are stored so we can replicate the second lookup > during instantiation. If I understand correctly, you point to a conflict between http://eel.is/c++draft/basic.lookup#qual.general-3.sentence-3 and http://eel.is/c++draft/temp.res#temp.dep.type-6. Have you considered filing a Core issue? https://github.com/llvm/llvm-project/pull/92957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
Endilll wrote: > Per [[basic.lookup.qual.general] > p1](http://eel.is/c++draft/basic.lookup.qual.general#1), lookup for a > member-qualified name is type-only if it's an identifier followed by ::; Per my reading, type-only lookup is performed only for elaborated type specifiers (http://eel.is/c++draft/basic.lookup#elab-1.sentence-1) and base-specifiers (http://eel.is/c++draft/class.derived#general-2.sentence-4). What [basic.lookup.qual.general] p1 describes is much like type-only lookup, but namespaces are considered. Another important difference is that http://eel.is/c++draft/basic.lookup#general-4.sentence-2 do apply to lookup of identifiers followed by `::`. Not saying that this materially changes anything, but we should be more cautions with terminology. https://github.com/llvm/llvm-project/pull/92957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
sdkrystian wrote: Ping @erichkeane @shafik @Endilll (not sure if drafts send emails for review requests) https://github.com/llvm/llvm-project/pull/92957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
sdkrystian wrote: I still need to add more comments and a release note, but this is otherwise functionally complete https://github.com/llvm/llvm-project/pull/92957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
llvmbot wrote: @llvm/pr-subscribers-coroutines Author: Krystian Stasiowski (sdkrystian) Changes [CWG1835](https://cplusplus.github.io/CWG/issues/1835.html) was one of the many core issues resolved by [P1787R6: Declarations and where to find them](http://wg21.link/p1787r6). Its resolution changes how [member-qualified names](http://eel.is/c++draft/basic.lookup.qual.general#2) are looked up. This patch is a draft implementation of that resolution. Previously, an _identifier_ following `.` or `->` would be first looked up in the type of the object expression (i.e. qualified lookup), and then in the context of the _postfix-expression_ (i.e. unqualified lookup) if nothing was found; the result of the second lookup was required to name a class template. Notably, this second lookup would occur even when the object expression was dependent, and its result would be used to determine whether a `<` token is the start of a _template-argument_list_. The new wording in [[basic.lookup.qual.general] p2](eel.is/c++draft/basic.lookup.qual.general#2) states: > A member-qualified name is the (unique) component name, if any, of > - an _unqualified-id_ or > - a _nested-name-specifier_ of the form _`type-name ::`_ or _`namespace-name ::`_ > > in the id-expression of a class member access expression. A ***qualified name*** is > - a member-qualified name or > - the terminal name of > - a _qualified-id_, > - a _using-declarator_, > - a _typename-specifier_, > - a _qualified-namespace-specifier_, or > - a _nested-name-specifier_, _elaborated-type-specifier_, or _class-or-decltype_ that has a _nested-name-specifier_. > > The _lookup context_ of a member-qualified name is the type of its associated object expression (considered dependent if the object expression is type-dependent). The lookup context of any other qualified name is the type, template, or namespace nominated by the preceding _nested-name-specifier_. And [[basic.lookup.qual.general] p3](eel.is/c++draft/basic.lookup.qual.general#3) now states: > _Qualified name lookup_ in a class, namespace, or enumeration performs a search of the scope associated with it except as specified below. Unless otherwise specified, a qualified name undergoes qualified name lookup in its lookup context from the point where it appears unless the lookup context either is dependent and is not the current instantiation or is not a class or class template. If nothing is found by qualified lookup for a member-qualified name that is the terminal name of a _nested-name-specifier_ and is not dependent, it undergoes unqualified lookup. In non-standardese terms, these two paragraphs essentially state the following: - A name that immediately follows `.` or `->` in a class member access expression is a member-qualified name - A member-qualified name will be first looked up in the type of the object expression `T` unless `T` is a dependent type that is _not_ the current instantiation, e.g. ```cpp templatestruct A { void f(T* t) { this->x; // type of the object expression is 'A '. although 'A ' is dependent, it is the // current instantiation so we look up 'x' in the template definition context. t->y; // type of the object expression is 'T' ('->' is transformed to '.' per [expr.ref]). // 'T' is dependent and is *not* the current instantiation, so we lookup 'y' in the // template instantiation context. } }; ``` - If the first lookup finds nothing and: - the member-qualified name is the first component of a _nested-name-specifier_ (which could be an _identifier_ or a _simple-template-id_), and either: - the type of the object expression is the current instantiation and it has no dependent base classes, or - the type of the object expression is not dependent then we lookup the name again, this time via unqualified lookup. Although the second (unqualified) lookup is stated not to occur when the member-qualified name is dependent, a dependent name will _not_ be dependent once the template is instantiated, so the second lookup must "occur" during instantiation if qualified lookup does not find anything. This means that we must perform the second (unqualified) lookup during parsing even when the type of the object expression is dependent, but those results are _not_ used to determine whether a `<` token is the start of a _template-argument_list_; they are stored so we can replicate the second lookup during instantiation. In even simpler terms (paraphrasing the [meeting minutes from the review of P1787](https://wiki.edg.com/bin/view/Wg21summer2020/P1787%28Lookup%29Review2020-06-15Through2020-06-18)): - Unqualified lookup always happens for the first name in a _nested-name-specifier_ that follows `.` or `->` - The result of that lookup is only used to determine whether `<` is the start of a _template-argument-list_
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
https://github.com/sdkrystian ready_for_review https://github.com/llvm/llvm-project/pull/92957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
sdkrystian wrote:
I added support for unqualified lookup finding multiple declarations in the
most recent commits, fixing the crash the currently happens on clang assertions
trunk for the following ([godbolt link](https://godbolt.org/z/rEzo76qr5)):
```cpp
inline namespace N
{
struct A { };
}
struct A { };
template
void f(T& t)
{
t.A::x; // currently causes a crash
}
```
https://github.com/llvm/llvm-project/pull/92957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
https://github.com/sdkrystian updated
https://github.com/llvm/llvm-project/pull/92957
>From 616b2cf138f9b4a1f3a23db404f77c0603ad61e1 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski
Date: Tue, 21 May 2024 13:15:24 -0400
Subject: [PATCH 1/2] [WIP][Clang] Implement resolution for CWG1835
---
clang/include/clang/Sema/DeclSpec.h | 8 +++
clang/include/clang/Sema/Sema.h | 2 +-
clang/lib/Parse/ParseDeclCXX.cpp | 2 +-
clang/lib/Parse/ParseExpr.cpp | 9 +--
clang/lib/Sema/SemaCXXScopeSpec.cpp | 60 +--
clang/lib/Sema/SemaExprMember.cpp | 10 +---
clang/lib/Sema/SemaPseudoObject.cpp | 16 ++---
clang/lib/Sema/SemaTemplate.cpp | 22 ---
clang/lib/Sema/TreeTransform.h| 15 +
.../basic.lookup.classref/p1-cxx11.cpp| 16 +++--
.../basic.lookup/basic.lookup.classref/p1.cpp | 16 +++--
.../class.derived/class.member.lookup/p8.cpp | 4 +-
clang/test/CXX/drs/cwg1xx.cpp | 7 ++-
clang/test/SemaCXX/static-assert-cxx17.cpp| 2 +-
.../SemaTemplate/temp_arg_nontype_cxx20.cpp | 2 +-
15 files changed, 127 insertions(+), 64 deletions(-)
diff --git a/clang/include/clang/Sema/DeclSpec.h
b/clang/include/clang/Sema/DeclSpec.h
index 23bc780e04979..c6d87ca1683a8 100644
--- a/clang/include/clang/Sema/DeclSpec.h
+++ b/clang/include/clang/Sema/DeclSpec.h
@@ -75,6 +75,7 @@ class CXXScopeSpec {
SourceRange Range;
NestedNameSpecifierLocBuilder Builder;
ArrayRef TemplateParamLists;
+ NamedDecl *FoundFirstQualifierInScope;
public:
SourceRange getRange() const { return Range; }
@@ -91,6 +92,13 @@ class CXXScopeSpec {
return TemplateParamLists;
}
+ void setFoundFirstQualifierInScope(NamedDecl *Found) {
+FoundFirstQualifierInScope = Found;
+ }
+ NamedDecl *getFirstQualifierFoundInScope() const {
+return FoundFirstQualifierInScope;
+ }
+
/// Retrieve the representation of the nested-name-specifier.
NestedNameSpecifier *getScopeRep() const {
return Builder.getRepresentation();
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index e6296868000c5..08e32d92b69a2 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -6920,7 +6920,7 @@ class Sema final : public SemaBase {
const TemplateArgumentListInfo *TemplateArgs);
ExprResult ActOnMemberAccessExpr(Scope *S, Expr *Base, SourceLocation OpLoc,
- tok::TokenKind OpKind, CXXScopeSpec &SS,
+ bool IsArrow, CXXScopeSpec &SS,
SourceLocation TemplateKWLoc,
UnqualifiedId &Member, Decl *ObjCImpDecl);
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 9a4a777f575b2..884e7ea8ee2de 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -720,7 +720,7 @@ Parser::DeclGroupPtrTy Parser::ParseUsingDeclaration(
return nullptr;
}
CXXScopeSpec SS;
-if (ParseOptionalCXXScopeSpecifier(SS, /*ParsedType=*/nullptr,
+if (ParseOptionalCXXScopeSpecifier(SS, /*ObjectType=*/nullptr,
/*ObectHasErrors=*/false,
/*EnteringConttext=*/false,
/*MayBePseudoDestructor=*/nullptr,
diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index eb7447fa038e4..b3c25f88b403d 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -2254,6 +2254,7 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
}
break;
}
+
ParseOptionalCXXScopeSpecifier(
SS, ObjectType, LHS.get() && LHS.get()->containsErrors(),
/*EnteringContext=*/false, &MayBePseudoDestructor);
@@ -2328,10 +2329,10 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
}
if (!LHS.isInvalid())
-LHS = Actions.ActOnMemberAccessExpr(getCurScope(), LHS.get(), OpLoc,
-OpKind, SS, TemplateKWLoc, Name,
- CurParsedObjCImpl ? CurParsedObjCImpl->Dcl
- : nullptr);
+LHS = Actions.ActOnMemberAccessExpr(
+getCurScope(), LHS.get(), OpLoc, OpKind == tok::arrow, SS,
+TemplateKWLoc, Name,
+CurParsedObjCImpl ? CurParsedObjCImpl->Dcl : nullptr);
if (!LHS.isInvalid()) {
if (Tok.is(tok::less))
checkPotentialAngleBracket(LHS);
diff --git a/clang/lib/Sema/SemaCXXScopeSpec.cpp
b/clang/lib/Sema/SemaCXXScopeSpec.cpp
index c405fbc0aa421..6efa8925d1446 100644
--- a/clang/lib/Sema/SemaCXXScopeSpec.cpp
+++ b/clang/lib/Sema/SemaCXXScopeSpec.cpp
@@ -397,11 +397,19 @@ NamedDecl *Sema::FindFirstQualifierInScope(Scope *S,
NestedNameSpecifie
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
@@ -55,15 +55,21 @@ namespace PR11856 {
template T *end(T*);
- class X { };
+ struct X { };
+ struct Y {
+int end;
+ };
template
void Foo2() {
T it1;
-if (it1->end < it1->end) {
-}
+if (it1->end < it1->end) { }
X *x;
-if (x->end < 7) { // expected-error{{no member named 'end' in
'PR11856::X'}}
-}
+if (x->end < 7) { } // expected-error{{expected '>'}}
+// expected-note@-1{{to match this '<'}}
+// expected-error@-2{{expected unqualified-id}}
mizvekov wrote:
I see.
Still, error recovery is not great here.
I think this looks sufficiently different from a template argument list that we
should just error out and commit to the interpretation that this is a
comparison.
We already have this capability to look ahead and tell if something looks like
a template argument list, see:
https://github.com/llvm/llvm-project/blob/d07362f7a9fc06e2445f5c4bc62c10a339bf68a5/clang/include/clang/Parse/Parser.h#L2734
https://github.com/llvm/llvm-project/pull/92957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
@@ -55,15 +55,21 @@ namespace PR11856 {
template T *end(T*);
- class X { };
+ struct X { };
+ struct Y {
+int end;
+ };
template
void Foo2() {
T it1;
-if (it1->end < it1->end) {
-}
+if (it1->end < it1->end) { }
X *x;
-if (x->end < 7) { // expected-error{{no member named 'end' in
'PR11856::X'}}
-}
+if (x->end < 7) { } // expected-error{{expected '>'}}
+// expected-note@-1{{to match this '<'}}
+// expected-error@-2{{expected unqualified-id}}
sdkrystian wrote:
This actually is expected -- see the final note in the comments of the PR.
Since we didn't find anything via qualified lookup, we "lock into" the
nested-name-specifier interpretation (because it would otherwise be an invalid
class member access since such a member doesn't exist). So, we do unqualified
lookup, find the template, and try parse `<` as a template argument list.
https://github.com/llvm/llvm-project/pull/92957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
@@ -618,7 +618,6 @@ namespace cwg141 { // cwg141: 3.1
// FIXME: we issue a useful diagnostic first, then some bogus ones.
mizvekov wrote:
It looks like this FIXME is fixed as per change below.
https://github.com/llvm/llvm-project/pull/92957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
@@ -2893,6 +2893,8 @@ class TreeTransform {
CXXScopeSpec SS;
SS.Adopt(QualifierLoc);
+if (FirstQualifierInScope)
+ SS.setFoundFirstQualifierInScope(FirstQualifierInScope);
mizvekov wrote:
It looks like adding 'FirstQualifierInScope' as a property to SS makes it so
keeping and passing both around is redundant.
https://github.com/llvm/llvm-project/pull/92957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
@@ -55,15 +55,21 @@ namespace PR11856 {
template T *end(T*);
- class X { };
+ struct X { };
+ struct Y {
+int end;
+ };
template
void Foo2() {
T it1;
-if (it1->end < it1->end) {
-}
+if (it1->end < it1->end) { }
X *x;
-if (x->end < 7) { // expected-error{{no member named 'end' in
'PR11856::X'}}
-}
+if (x->end < 7) { } // expected-error{{expected '>'}}
+// expected-note@-1{{to match this '<'}}
+// expected-error@-2{{expected unqualified-id}}
mizvekov wrote:
This doesn't look expected to me, what is going on?
https://github.com/llvm/llvm-project/pull/92957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
@@ -397,22 +397,32 @@ NamedDecl *Sema::FindFirstQualifierInScope(Scope *S,
NestedNameSpecifier *NNS) {
while (NNS->getPrefix())
NNS = NNS->getPrefix();
- if (NNS->getKind() != NestedNameSpecifier::Identifier)
-return nullptr;
-
- LookupResult Found(*this, NNS->getAsIdentifier(), SourceLocation(),
- LookupNestedNameSpecifierName);
+ // FIXME: This is a rather nasty hack! Ideally we should get the results
+ // from LookupTemplateName/BuildCXXNestedNameSpecifier.
+ const IdentifierInfo *II = NNS->getAsIdentifier();
+ if (!II) {
+if (const auto *DTST =
+dyn_cast_if_present(
+NNS->getAsType()))
+ II = DTST->getIdentifier();
+else
+ return nullptr;
+ }
mizvekov wrote:
I am not sure why you think this is a nasty hack, this looks legitimate to me.
I think this looks ugly because of the lack of a NNS kind for an Identifier
with template arguments, so we abuse a type kind storing a
DependentTemplateSpecializationType with no qualifier.
https://github.com/llvm/llvm-project/pull/92957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
https://github.com/mizvekov edited https://github.com/llvm/llvm-project/pull/92957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
@@ -720,7 +720,7 @@ Parser::DeclGroupPtrTy Parser::ParseUsingDeclaration( return nullptr; } CXXScopeSpec SS; -if (ParseOptionalCXXScopeSpecifier(SS, /*ParsedType=*/nullptr, +if (ParseOptionalCXXScopeSpecifier(SS, /*ObjectType=*/nullptr, mizvekov wrote: Land this cleanup separately as well. https://github.com/llvm/llvm-project/pull/92957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
@@ -6891,7 +6891,7 @@ class Sema final : public SemaBase {
const TemplateArgumentListInfo *TemplateArgs);
ExprResult ActOnMemberAccessExpr(Scope *S, Expr *Base, SourceLocation OpLoc,
- tok::TokenKind OpKind, CXXScopeSpec &SS,
+ bool IsArrow, CXXScopeSpec &SS,
mizvekov wrote:
You could land this refactoring straight away on an NFC commit, to clean up for
the review a bit.
https://github.com/llvm/llvm-project/pull/92957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
@@ -47,8 +47,8 @@ template
void DerivedT::Inner() {
Derived1T::Foo();
Derived2T::Member = 42;
- this->Derived1T::Foo();
- this->Derived2T::Member = 42;
+ this->Derived1T::Foo(); // expected-error{{use 'template' keyword to
treat 'Derived1T' as a dependent template name}}
+ this->Derived2T::Member = 42; // expected-error{{use 'template' keyword
to treat 'Derived2T' as a dependent template name}}
mizvekov wrote:
This is an access control test:
```suggestion
this->template Derived1T::Foo();
this->template Derived2T::Member = 42;
```
https://github.com/llvm/llvm-project/pull/92957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
https://github.com/mizvekov commented: I think overall this looks like the right direction to me. https://github.com/llvm/llvm-project/pull/92957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
sdkrystian wrote: @Endilll I'm still in the process of writing tests, but I'll add several soon. I'm quite confident about my interpretation being correct but I just want to be 100% sure :). At the very least this patch gets [[basic.lookup.qual.general] example 3](http://eel.is/c++draft/basic.lookup.qual.general#example-3) right, in addition to the examples I provided in the PR. https://github.com/llvm/llvm-project/pull/92957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
Endilll wrote: It was a pure accident I opened this PR. I think draft status does a good job at silencing notifications. I spend a decent amount of time in P1787R6, so I might be able to provide good feedback. I'll take a look tomorrow. It would be nice to see a test for CWG1835 among the changes, even if you're not sure about your interpretation. https://github.com/llvm/llvm-project/pull/92957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
https://github.com/sdkrystian updated
https://github.com/llvm/llvm-project/pull/92957
>From 4524db5ae7f3133b51328fbabd1f6188d7d3707b Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski
Date: Tue, 21 May 2024 13:15:24 -0400
Subject: [PATCH 1/2] [WIP][Clang] Implement resolution for CWG1835
---
clang/include/clang/Sema/DeclSpec.h | 8 +++
clang/include/clang/Sema/Sema.h | 2 +-
clang/lib/Parse/ParseDeclCXX.cpp | 2 +-
clang/lib/Parse/ParseExpr.cpp | 9 +--
clang/lib/Sema/SemaCXXScopeSpec.cpp | 60 +--
clang/lib/Sema/SemaExprMember.cpp | 10 +---
clang/lib/Sema/SemaPseudoObject.cpp | 16 ++---
clang/lib/Sema/SemaTemplate.cpp | 22 ---
clang/lib/Sema/TreeTransform.h| 15 +
.../basic.lookup.classref/p1-cxx11.cpp| 16 +++--
.../basic.lookup/basic.lookup.classref/p1.cpp | 16 +++--
.../class.derived/class.member.lookup/p8.cpp | 4 +-
clang/test/CXX/drs/cwg1xx.cpp | 9 +--
clang/test/SemaCXX/static-assert-cxx17.cpp| 2 +-
.../SemaTemplate/temp_arg_nontype_cxx20.cpp | 2 +-
15 files changed, 128 insertions(+), 65 deletions(-)
diff --git a/clang/include/clang/Sema/DeclSpec.h
b/clang/include/clang/Sema/DeclSpec.h
index 23bc780e04979..c6d87ca1683a8 100644
--- a/clang/include/clang/Sema/DeclSpec.h
+++ b/clang/include/clang/Sema/DeclSpec.h
@@ -75,6 +75,7 @@ class CXXScopeSpec {
SourceRange Range;
NestedNameSpecifierLocBuilder Builder;
ArrayRef TemplateParamLists;
+ NamedDecl *FoundFirstQualifierInScope;
public:
SourceRange getRange() const { return Range; }
@@ -91,6 +92,13 @@ class CXXScopeSpec {
return TemplateParamLists;
}
+ void setFoundFirstQualifierInScope(NamedDecl *Found) {
+FoundFirstQualifierInScope = Found;
+ }
+ NamedDecl *getFirstQualifierFoundInScope() const {
+return FoundFirstQualifierInScope;
+ }
+
/// Retrieve the representation of the nested-name-specifier.
NestedNameSpecifier *getScopeRep() const {
return Builder.getRepresentation();
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 5894239664c15..805d36fd10544 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -6891,7 +6891,7 @@ class Sema final : public SemaBase {
const TemplateArgumentListInfo *TemplateArgs);
ExprResult ActOnMemberAccessExpr(Scope *S, Expr *Base, SourceLocation OpLoc,
- tok::TokenKind OpKind, CXXScopeSpec &SS,
+ bool IsArrow, CXXScopeSpec &SS,
SourceLocation TemplateKWLoc,
UnqualifiedId &Member, Decl *ObjCImpDecl);
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 5eaec2b621e6f..b2fa6da002f98 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -720,7 +720,7 @@ Parser::DeclGroupPtrTy Parser::ParseUsingDeclaration(
return nullptr;
}
CXXScopeSpec SS;
-if (ParseOptionalCXXScopeSpecifier(SS, /*ParsedType=*/nullptr,
+if (ParseOptionalCXXScopeSpecifier(SS, /*ObjectType=*/nullptr,
/*ObectHasErrors=*/false,
/*EnteringConttext=*/false,
/*MayBePseudoDestructor=*/nullptr,
diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index eb7447fa038e4..b3c25f88b403d 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -2254,6 +2254,7 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
}
break;
}
+
ParseOptionalCXXScopeSpecifier(
SS, ObjectType, LHS.get() && LHS.get()->containsErrors(),
/*EnteringContext=*/false, &MayBePseudoDestructor);
@@ -2328,10 +2329,10 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
}
if (!LHS.isInvalid())
-LHS = Actions.ActOnMemberAccessExpr(getCurScope(), LHS.get(), OpLoc,
-OpKind, SS, TemplateKWLoc, Name,
- CurParsedObjCImpl ? CurParsedObjCImpl->Dcl
- : nullptr);
+LHS = Actions.ActOnMemberAccessExpr(
+getCurScope(), LHS.get(), OpLoc, OpKind == tok::arrow, SS,
+TemplateKWLoc, Name,
+CurParsedObjCImpl ? CurParsedObjCImpl->Dcl : nullptr);
if (!LHS.isInvalid()) {
if (Tok.is(tok::less))
checkPotentialAngleBracket(LHS);
diff --git a/clang/lib/Sema/SemaCXXScopeSpec.cpp
b/clang/lib/Sema/SemaCXXScopeSpec.cpp
index c405fbc0aa421..6efa8925d1446 100644
--- a/clang/lib/Sema/SemaCXXScopeSpec.cpp
+++ b/clang/lib/Sema/SemaCXXScopeSpec.cpp
@@ -397,11 +397,19 @@ NamedDecl *Sema::FindFirstQualifierInScope(Scope *S,
NestedNameSpecifie
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
sdkrystian wrote: (this PR is by no means complete, but I would like feedback regarding the validity of my interpretation of the DR resolution and perhaps some guidance with how to best go about implementing it. The currently included implementation _works_, but I'm uncertain whether it's the best way of going about it). https://github.com/llvm/llvm-project/pull/92957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Implement resolution for CWG1835 (PR #92957)
https://github.com/sdkrystian created
https://github.com/llvm/llvm-project/pull/92957
[CWG1835](https://cplusplus.github.io/CWG/issues/1835.html) was one of the many
core issues resolved by [P1787R6: Declarations and where to find
them](http://wg21.link/p1787r6). Its resolution changes how [member-qualified
names](http://eel.is/c++draft/basic.lookup.qual.general#2) are looked up. This
patch is a draft implementation of that resolution.
Previously, an _identifier_ following `.` or `->` would be first looked up in
the type of the object expression (i.e. qualified lookup), and then in the
context of the _postfix-expression_ (i.e. unqualified lookup) if nothing was
found; the result of the second lookup was required to name a class template.
Notably, this second lookup would occur even when the object expression was
dependent, and its result would be used to determine whether a `<` token is the
start of a _template-argument_list_.
The new wording in [[basic.lookup.qual.general]
p2](eel.is/c++draft/basic.lookup.qual.general#2) states:
> A member-qualified name is the (unique) component name, if any, of
> - an _unqualified-id_ or
> - a _nested-name-specifier_ of the form _`type-name ::`_ or _`namespace-name
> ::`_
>
> in the id-expression of a class member access expression. A ***qualified
> name*** is
> - a member-qualified name or
> - the terminal name of
> - a _qualified-id_,
> - a _using-declarator_,
> - a _typename-specifier_,
> - a _qualified-namespace-specifier_, or
> - a _nested-name-specifier_, _elaborated-type-specifier_, or
> _class-or-decltype_ that has a _nested-name-specifier_.
>
> The _lookup context_ of a member-qualified name is the type of its
> associated object expression (considered dependent if the object expression
> is type-dependent). The lookup context of any other qualified name is the
> type, template, or namespace nominated by the preceding
> _nested-name-specifier_.
And [[basic.lookup.qual.general]
p3](eel.is/c++draft/basic.lookup.qual.general#3) now states:
> _Qualified name lookup_ in a class, namespace, or enumeration performs a
> search of the scope associated with it except as specified below. Unless
> otherwise specified, a qualified name undergoes qualified name lookup in its
> lookup context from the point where it appears unless the lookup context
> either is dependent and is not the current instantiation or is not a class or
> class template. If nothing is found by qualified lookup for a
> member-qualified name that is the terminal name of a _nested-name-specifier_
> and is not dependent, it undergoes unqualified lookup.
In non-standardese terms, these two paragraphs essentially state the following:
- A name that immediately follows `.` or `->` in a class member access
expression is a member-qualified name
- A member-qualified name will be first looked up in the type of the object
expression `T` unless `T` is a dependent type that is _not_ the current
instantiation, e.g.
```cpp
template
struct A
{
void f(T* t)
{
this->x; // type of the object expression is 'A'. although 'A' is
dependent, it is the
// current instantiation so we look up 'x' in the template
definition context.
t->y; // type of the object expression is 'T' ('->' is transformed to
'.' per [expr.ref]).
// 'T' is dependent and is *not* the current instantiation, so we
lookup 'y' in the
// template instantiation context.
}
};
```
- If the first lookup finds nothing and:
- the member-qualified name is the first component of a
_nested-name-specifier_ (which could be an _identifier_ or a
_simple-template-id_), and either:
- the type of the object expression is the current instantiation and it
has no dependent base classes, or
- the type of the object expression is not dependent
then we lookup the name again, this time via unqualified lookup.
Although the second (unqualified) lookup is stated not to occur when the
member-qualified name is dependent, a dependent name will _not_ be dependent
once the template is instantiated, so the second lookup must "occur" during
instantiation if qualified lookup does not find anything. This means that we
must perform the second (unqualified) lookup during parsing even when the type
of the object expression is dependent, but those results are _not_ used to
determine whether a `<` token is the start of a _template-argument_list_; they
are stored so we can replicate the second lookup during instantiation.
In even simpler terms (paraphrasing the [meeting minutes from the review of
P1787](https://wiki.edg.com/bin/view/Wg21summer2020/P1787%28Lookup%29Review2020-06-15Through2020-06-18)):
- Unqualified lookup always happens for the first name in a
_nested-name-specifier_ that follows `.` or `->`
- The result of that lookup is only used to determine whether `<` is the start
of a _template-argument-list_ if t
