[clang] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 93831c73ea51dcf4dc1832a4ea5616b819d36f31
babdee51c4a40a5053ed47846ff57bae2858a21b --extensions 'h,,cpp,c' --
clang/test/CXX/basic/basic.lookup/basic.lookup.qual/basic.lookup.qual.general/p3-example3.cpp
clang/test/CXX/basic/basic.lookup/basic.lookup.qual/basic.lookup.qual.general/p3.cpp
clang/test/CXX/temp/temp.names/p3-23.cpp clang/include/clang/AST/ExprCXX.h
clang/include/clang/AST/Stmt.h clang/include/clang/AST/UnresolvedSet.h
clang/include/clang/Parse/Parser.h clang/include/clang/Sema/DeclSpec.h
clang/include/clang/Sema/Lookup.h clang/include/clang/Sema/Sema.h
clang/lib/AST/ASTImporter.cpp clang/lib/AST/ExprCXX.cpp
clang/lib/AST/ItaniumMangle.cpp clang/lib/Parse/ParseExprCXX.cpp
clang/lib/Sema/SemaCXXScopeSpec.cpp clang/lib/Sema/SemaCoroutine.cpp
clang/lib/Sema/SemaDeclCXX.cpp clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprMember.cpp clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaStmtAsm.cpp clang/lib/Sema/SemaTemplate.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp clang/lib/Sema/TreeTransform.h
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriterStmt.cpp
clang/test/CXX/basic/basic.lookup/basic.lookup.classref/p1-cxx11.cpp
clang/test/CXX/basic/basic.lookup/basic.lookup.classref/p1.cpp
clang/test/CXX/class.derived/class.member.lookup/p8.cpp
clang/test/CXX/drs/cwg1xx.cpp clang/test/CXX/drs/cwg3xx.cpp
clang/test/CXX/drs/cwg4xx.cpp clang/test/CXX/temp/temp.res/p3.cpp
clang/test/FixIt/fixit.cpp clang/test/Misc/warning-flags.c
clang/test/Parser/cxx2a-concepts-requires-expr.cpp
clang/test/SemaCXX/pseudo-destructors.cpp
clang/test/SemaCXX/static-assert-cxx17.cpp
clang/test/SemaTemplate/dependent-base-classes.cpp
clang/test/SemaTemplate/dependent-template-recover.cpp
clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
clang/test/SemaTemplate/template-id-expr.cpp
clang/test/SemaTemplate/typename-specifier-3.cpp libcxx/include/regex
``
View the diff from clang-format here.
``diff
diff --git a/clang/lib/Sema/SemaCXXScopeSpec.cpp
b/clang/lib/Sema/SemaCXXScopeSpec.cpp
index ae20849193..84d0baf80e 100644
--- a/clang/lib/Sema/SemaCXXScopeSpec.cpp
+++ b/clang/lib/Sema/SemaCXXScopeSpec.cpp
@@ -483,7 +483,7 @@ bool Sema::BuildCXXNestedNameSpecifier(Scope *S,
NestedNameSpecInfo ,
bool ErrorRecoveryLookup,
bool *IsCorrectedToColon,
bool OnlyNamespace) {
- #if 0
+#if 0
if (IdInfo.Identifier->isEditorPlaceholder())
return true;
LookupResult Found(*this, IdInfo.Identifier, IdInfo.IdentifierLoc,
@@ -579,7 +579,6 @@ bool Sema::BuildCXXNestedNameSpecifier(Scope *S,
NestedNameSpecInfo ,
#endif
#endif
-
if (IdInfo.Identifier->isEditorPlaceholder())
return true;
if (IsCorrectedToColon)
@@ -609,8 +608,7 @@ bool Sema::BuildCXXNestedNameSpecifier(Scope *S,
NestedNameSpecInfo ,
}
bool ObjectTypeSearchedInScope = false;
- bool LookupFirstQualifierInScope =
- Found.empty() && !ObjectType.isNull();
+ bool LookupFirstQualifierInScope = Found.empty() && !ObjectType.isNull();
if (LookupFirstQualifierInScope) {
if (S) {
@@ -658,9 +656,10 @@ bool Sema::BuildCXXNestedNameSpecifier(Scope *S,
NestedNameSpecInfo ,
}
}
- DeclContext *LookupCtx = SS.isSet()
- ? computeDeclContext(SS, EnteringContext)
- : (!ObjectType.isNull() ? computeDeclContext(ObjectType) : nullptr);
+ DeclContext *LookupCtx =
+ SS.isSet()
+ ? computeDeclContext(SS, EnteringContext)
+ : (!ObjectType.isNull() ? computeDeclContext(ObjectType) : nullptr);
if (Found.empty() && !ErrorRecoveryLookup && !getLangOpts().MSVCCompat) {
// We haven't found anything, and we're not recovering from a
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 1349ee2ea2..5caa8eff30 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -400,8 +400,8 @@ bool Sema::LookupTemplateName(LookupResult , Scope
*S, CXXScopeSpec ,
// vector component name, and look up a template name if not. And similarly
// for lookups into Objective-C class and object types, where the same
// problem can arise.
- if (!ObjectType.isNull() &&
- (ObjectType->isVectorType() ||
ObjectType->isObjCObjectOrInterfaceType())) {
+ if (!ObjectType.isNull() && (ObjectType->isVectorType() ||
+ ObjectType->isObjCObjectOrInterfaceType())) {
Found.clear();
return false;
}
@@ -412,7 +412,7 @@ bool Sema::LookupTemplateName(LookupResult , Scope
*S, CXXScopeSpec ,
if (Found.wasNotFoundInCurrentInstantiation())
return false;
- #if 0
+#if 0
// Determine where to perform name lookup
[clang] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)
@@ -390,29 +390,37 @@ bool Sema::isAcceptableNestedNameSpecifier(const
NamedDecl *SD,
/// (e.g., Base::), perform name lookup for that identifier as a
/// nested-name-specifier within the given scope, and return the result of that
/// name lookup.
-NamedDecl *Sema::FindFirstQualifierInScope(Scope *S, NestedNameSpecifier *NNS)
{
- if (!S || !NNS)
-return nullptr;
+bool Sema::LookupFirstQualifierInScope(Scope *S, NestedNameSpecifier *NNS,
+ UnresolvedSetImpl ) {
+ if (!S)
+return false;
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
mizvekov wrote:
I don't think this is a hack per se, I think this is just a consequence of not
having a special NNS kind for this situation, and representing it with a DTST.
The alternative that I see is to implement a new NNS prefix which is composed
by an identifier followed by template arguments.
It can still be represented internally with the type, if that's cheaper, but
having a special accessor, and making `NNS->getAsIdentifier()` work for it,
would be nicer.
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] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)
@@ -548,6 +575,7 @@ bool Sema::BuildCXXNestedNameSpecifier(Scope *S, NestedNameSpecInfo , // Perform unqualified name lookup in the current scope. LookupName(Found, S); } +#endif mizvekov wrote: A left-over. 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] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)
https://github.com/mizvekov approved this pull request. LGTM save a few minor cleanups needed. I think this patch has an incredible amount of noise due to amount of cosmetic changes caused by reformatting, specially due to changing parameters and such. This is fine for now and I am not suggesting you go back and do it for this patch, but I think it would have been helpful to pre-clang-format the file, and offload more of the minor changes into a previous patch. 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] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)
@@ -2923,10 +2920,9 @@ class TreeTransform {
}
return getSema().BuildMemberReferenceExpr(Base, BaseType, OpLoc, isArrow,
- SS, TemplateKWLoc,
- FirstQualifierInScope,
- R, ExplicitTemplateArgs,
- /*S*/nullptr);
+ SS, TemplateKWLoc, R,
+ ExplicitTemplateArgs,
+ /*S*/ nullptr);
mizvekov wrote:
```suggestion
/*S=*/nullptr);
```
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] [libcxx] [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] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)
@@ -1442,19 +1442,27 @@ SourceLocation
CXXUnresolvedConstructExpr::getBeginLoc() const {
CXXDependentScopeMemberExpr::CXXDependentScopeMemberExpr(
const ASTContext , Expr *Base, QualType BaseType, bool IsArrow,
SourceLocation OperatorLoc, NestedNameSpecifierLoc QualifierLoc,
-SourceLocation TemplateKWLoc, NamedDecl *FirstQualifierFoundInScope,
+SourceLocation TemplateKWLoc, ArrayRef UnqualifiedLookups,
DeclarationNameInfo MemberNameInfo,
const TemplateArgumentListInfo *TemplateArgs)
: Expr(CXXDependentScopeMemberExprClass, Ctx.DependentTy, VK_LValue,
OK_Ordinary),
- Base(Base), BaseType(BaseType), QualifierLoc(QualifierLoc),
- MemberNameInfo(MemberNameInfo) {
+ Base(Base), BaseType(BaseType), MemberNameInfo(MemberNameInfo),
+ OperatorLoc(OperatorLoc) {
CXXDependentScopeMemberExprBits.IsArrow = IsArrow;
+ CXXDependentScopeMemberExprBits.HasQualifier = QualifierLoc.hasQualifier();
+ CXXDependentScopeMemberExprBits.NumUnqualifiedLookups =
+ UnqualifiedLookups.size();
CXXDependentScopeMemberExprBits.HasTemplateKWAndArgsInfo =
(TemplateArgs != nullptr) || TemplateKWLoc.isValid();
- CXXDependentScopeMemberExprBits.HasFirstQualifierFoundInScope =
- FirstQualifierFoundInScope != nullptr;
- CXXDependentScopeMemberExprBits.OperatorLoc = OperatorLoc;
+
+ if (hasQualifier())
+new (getTrailingObjects())
+NestedNameSpecifierLoc(QualifierLoc);
+
+ std::uninitialized_copy_n(UnqualifiedLookups.data(),
+UnqualifiedLookups.size(),
sdkrystian wrote:
That's left over from a point in time when I had implemented the storage of
multiple lookup results in `CXXDependentScopeMemberExpr`, but the constructor
still had a single `NamedDecl*` parameter. I'll change this to use
`std::uninitialized_copy`.
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] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)
@@ -1790,24 +1782,25 @@ ExprResult Sema::ActOnMemberAccessExpr(Scope *S, Expr *Base, const TemplateArgumentListInfo *TemplateArgs; DecomposeUnqualifiedId(Id, TemplateArgsBuffer, NameInfo, TemplateArgs); - - bool IsArrow = (OpKind == tok::arrow); + bool IsArrow = OpKind == tok::arrow; erichkeane wrote: Mild preference to keep the old spelling here. 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] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)
@@ -390,29 +390,37 @@ bool Sema::isAcceptableNestedNameSpecifier(const
NamedDecl *SD,
/// (e.g., Base::), perform name lookup for that identifier as a
/// nested-name-specifier within the given scope, and return the result of that
/// name lookup.
-NamedDecl *Sema::FindFirstQualifierInScope(Scope *S, NestedNameSpecifier *NNS)
{
- if (!S || !NNS)
-return nullptr;
+bool Sema::LookupFirstQualifierInScope(Scope *S, NestedNameSpecifier *NNS,
+ UnresolvedSetImpl ) {
+ if (!S)
+return false;
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
erichkeane wrote:
I sadly don't have a good alternative here...
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] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)
https://github.com/erichkeane 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] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)
https://github.com/erichkeane approved this pull request. Sorry this took so long! Apparently I'd left some comments without finishing my review. Did another look through, everything looks good, just some minor nits. Apply/merge at your convenience. 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] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)
@@ -1442,19 +1442,27 @@ SourceLocation
CXXUnresolvedConstructExpr::getBeginLoc() const {
CXXDependentScopeMemberExpr::CXXDependentScopeMemberExpr(
const ASTContext , Expr *Base, QualType BaseType, bool IsArrow,
SourceLocation OperatorLoc, NestedNameSpecifierLoc QualifierLoc,
-SourceLocation TemplateKWLoc, NamedDecl *FirstQualifierFoundInScope,
+SourceLocation TemplateKWLoc, ArrayRef UnqualifiedLookups,
DeclarationNameInfo MemberNameInfo,
const TemplateArgumentListInfo *TemplateArgs)
: Expr(CXXDependentScopeMemberExprClass, Ctx.DependentTy, VK_LValue,
OK_Ordinary),
- Base(Base), BaseType(BaseType), QualifierLoc(QualifierLoc),
- MemberNameInfo(MemberNameInfo) {
+ Base(Base), BaseType(BaseType), MemberNameInfo(MemberNameInfo),
+ OperatorLoc(OperatorLoc) {
CXXDependentScopeMemberExprBits.IsArrow = IsArrow;
+ CXXDependentScopeMemberExprBits.HasQualifier = QualifierLoc.hasQualifier();
+ CXXDependentScopeMemberExprBits.NumUnqualifiedLookups =
+ UnqualifiedLookups.size();
CXXDependentScopeMemberExprBits.HasTemplateKWAndArgsInfo =
(TemplateArgs != nullptr) || TemplateKWLoc.isValid();
- CXXDependentScopeMemberExprBits.HasFirstQualifierFoundInScope =
- FirstQualifierFoundInScope != nullptr;
- CXXDependentScopeMemberExprBits.OperatorLoc = OperatorLoc;
+
+ if (hasQualifier())
+new (getTrailingObjects())
+NestedNameSpecifierLoc(QualifierLoc);
+
+ std::uninitialized_copy_n(UnqualifiedLookups.data(),
+UnqualifiedLookups.size(),
erichkeane wrote:
Any reason for `std::uninitialized_copy_n` vs `std::uninitialized_copy`? Don't
mind, mostly curious.
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] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)
sdkrystian wrote: @erichkeane Hoping you can review this before your break! 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] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)
https://github.com/ldionne approved this pull request. This patch isn't blocked by libc++, thanks for fixing the incorrect usage. This isn't a review of the patch itself, just unblocking for libcxx-reviewers. 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] [libcxx] [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:
Yes, this looks great now, thanks!
I still have to finish review of the rest of this huge patch, this is going to
take a while :)
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] [libcxx] [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:
@mizvekov I implemented this in the ~5 most recent commits, if you'd like to
take a second look
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] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)
@@ -1442,19 +1442,27 @@ SourceLocation
CXXUnresolvedConstructExpr::getBeginLoc() const {
CXXDependentScopeMemberExpr::CXXDependentScopeMemberExpr(
const ASTContext , Expr *Base, QualType BaseType, bool IsArrow,
SourceLocation OperatorLoc, NestedNameSpecifierLoc QualifierLoc,
-SourceLocation TemplateKWLoc, NamedDecl *FirstQualifierFoundInScope,
+SourceLocation TemplateKWLoc, ArrayRef UnqualifiedLookups,
DeclarationNameInfo MemberNameInfo,
const TemplateArgumentListInfo *TemplateArgs)
: Expr(CXXDependentScopeMemberExprClass, Ctx.DependentTy, VK_LValue,
OK_Ordinary),
- Base(Base), BaseType(BaseType), QualifierLoc(QualifierLoc),
+ Base(Base), BaseType(BaseType), OperatorLoc(OperatorLoc),
jcsxky wrote:
There is a warning here. `field 'OperatorLoc' will be initialized after field
'MemberNameInfo'`
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
