[clang] [Clang][Sema] Ensure that the selected candidate for a member function explicit specialization is more constrained than all others (PR #101721)
Krystian Stasiowski via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 6 08:15:31 PDT 2024
https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/101721
>From dd1c7b5fe3e1c4bca73cc5b4162ae73c3d7783fb Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Fri, 2 Aug 2024 11:32:49 -0400
Subject: [PATCH 1/5] [Clang][Sema] Ensure that the selected candidate for a
member function explicit specialization is more constrained than all others
---
clang/lib/Sema/SemaTemplate.cpp | 80 +++++++++++++++++++++++----------
1 file changed, 56 insertions(+), 24 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 1346a4a3f0012..c96925ba73ae2 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -9062,51 +9062,83 @@ Sema::CheckMemberSpecialization(NamedDecl *Member, LookupResult &Previous) {
if (Previous.empty()) {
// Nowhere to look anyway.
} else if (FunctionDecl *Function = dyn_cast<FunctionDecl>(Member)) {
- SmallVector<FunctionDecl *> Candidates;
- bool Ambiguous = false;
- for (LookupResult::iterator I = Previous.begin(), E = Previous.end();
- I != E; ++I) {
- CXXMethodDecl *Method =
- dyn_cast<CXXMethodDecl>((*I)->getUnderlyingDecl());
- if (!Method)
+ LookupResult::Filter Filter = Previous.makeFilter();
+ while (Filter.hasNext()) {
+ auto *Method =
+ dyn_cast<CXXMethodDecl>(Filter.next()->getUnderlyingDecl());
+ // Discard any candidates that aren't member functions.
+ if (!Method) {
+ Filter.erase();
continue;
+ }
+
QualType Adjusted = Function->getType();
if (!hasExplicitCallingConv(Adjusted))
Adjusted = adjustCCAndNoReturn(Adjusted, Method->getType());
+ // Discard any candidates with the wrong type.
// This doesn't handle deduced return types, but both function
// declarations should be undeduced at this point.
- if (!Context.hasSameType(Adjusted, Method->getType()))
+ if (!Context.hasSameType(Adjusted, Method->getType())) {
+ Filter.erase();
continue;
+ }
+
+ // Discard any candidates with unsatisfied constraints.
if (ConstraintSatisfaction Satisfaction;
Method->getTrailingRequiresClause() &&
(CheckFunctionConstraints(Method, Satisfaction,
/*UsageLoc=*/Member->getLocation(),
/*ForOverloadResolution=*/true) ||
- !Satisfaction.IsSatisfied))
- continue;
- Candidates.push_back(Method);
- FunctionDecl *MoreConstrained =
- Instantiation ? getMoreConstrainedFunction(
- Method, cast<FunctionDecl>(Instantiation))
- : Method;
- if (!MoreConstrained) {
- Ambiguous = true;
+ !Satisfaction.IsSatisfied)) {
+ Filter.erase();
continue;
}
- if (MoreConstrained == Method) {
- Ambiguous = false;
- FoundInstantiation = *I;
- Instantiation = Method;
- InstantiatedFrom = Method->getInstantiatedFromMemberFunction();
- MSInfo = Method->getMemberSpecializationInfo();
+ }
+ Filter.done();
+
+ // If we have no candidates left after filtering, we are done.
+ if (Previous.empty())
+ return false;
+
+ // Find the function that is more constrained than every other function it
+ // has been compared to.
+ UnresolvedSetIterator Best = Previous.begin();
+ CXXMethodDecl *BestMethod = nullptr;
+ for (UnresolvedSetIterator I = Previous.begin(), E = Previous.end(); I != E;
+ ++I) {
+ auto *Method = cast<CXXMethodDecl>(I->getUnderlyingDecl());
+ if (I == Best ||
+ getMoreConstrainedFunction(Method, BestMethod) == Method) {
+ Best = I;
+ BestMethod = Method;
+ }
+ }
+
+ FoundInstantiation = *Best;
+ Instantiation = BestMethod;
+ InstantiatedFrom = BestMethod->getInstantiatedFromMemberFunction();
+ MSInfo = BestMethod->getMemberSpecializationInfo();
+
+ // Make sure the best candidate is more specialized than all of the others.
+ bool Ambiguous = false;
+ for (UnresolvedSetIterator I = Previous.begin(), E = Previous.end(); I != E;
+ ++I) {
+ auto *Method = cast<CXXMethodDecl>(I->getUnderlyingDecl());
+ if (I != Best &&
+ getMoreConstrainedFunction(Method, BestMethod) != BestMethod) {
+ Ambiguous = true;
+ break;
}
}
+
if (Ambiguous) {
Diag(Member->getLocation(), diag::err_function_member_spec_ambiguous)
<< Member << (InstantiatedFrom ? InstantiatedFrom : Instantiation);
- for (FunctionDecl *Candidate : Candidates)
+ for (NamedDecl *Candidate : Previous) {
+ Candidate = Candidate->getUnderlyingDecl();
Diag(Candidate->getLocation(), diag::note_function_member_spec_matched)
<< Candidate;
+ }
return true;
}
} else if (isa<VarDecl>(Member)) {
>From 8d9a4f0fc1da8b376a4c1e5cbf5e868ee20c790a Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Fri, 2 Aug 2024 12:51:22 -0400
Subject: [PATCH 2/5] [FOLD] add test
---
.../temp/temp.spec/temp.expl.spec/p14-23.cpp | 116 +++++++++++-------
1 file changed, 73 insertions(+), 43 deletions(-)
diff --git a/clang/test/CXX/temp/temp.spec/temp.expl.spec/p14-23.cpp b/clang/test/CXX/temp/temp.spec/temp.expl.spec/p14-23.cpp
index dc17cea99d435..a023bf84137d7 100644
--- a/clang/test/CXX/temp/temp.spec/temp.expl.spec/p14-23.cpp
+++ b/clang/test/CXX/temp/temp.spec/temp.expl.spec/p14-23.cpp
@@ -1,60 +1,90 @@
// RUN: %clang_cc1 -std=c++20 -verify %s
-template<int I>
-concept C = I >= 4;
+namespace N0 {
+ template<int I>
+ concept C = I >= 4;
-template<int I>
-concept D = I < 8;
+ template<int I>
+ concept D = I < 8;
-template<int I>
-struct A {
- constexpr static int f() { return 0; }
- constexpr static int f() requires C<I> && D<I> { return 1; }
- constexpr static int f() requires C<I> { return 2; }
+ template<int I>
+ struct A {
+ constexpr static int f() { return 0; }
+ constexpr static int f() requires C<I> && D<I> { return 1; }
+ constexpr static int f() requires C<I> { return 2; }
- constexpr static int g() requires C<I> { return 0; } // #candidate-0
- constexpr static int g() requires D<I> { return 1; } // #candidate-1
+ constexpr static int g() requires C<I> { return 0; } // #candidate-0
+ constexpr static int g() requires D<I> { return 1; } // #candidate-1
- constexpr static int h() requires C<I> { return 0; } // expected-note {{member declaration nearly matches}}
-};
+ constexpr static int h() requires C<I> { return 0; } // expected-note {{member declaration nearly matches}}
+ };
-template<>
-constexpr int A<2>::f() { return 3; }
+ template<>
+ constexpr int A<2>::f() { return 3; }
-template<>
-constexpr int A<4>::f() { return 4; }
+ template<>
+ constexpr int A<4>::f() { return 4; }
-template<>
-constexpr int A<8>::f() { return 5; }
+ template<>
+ constexpr int A<8>::f() { return 5; }
-static_assert(A<3>::f() == 0);
-static_assert(A<5>::f() == 1);
-static_assert(A<9>::f() == 2);
-static_assert(A<2>::f() == 3);
-static_assert(A<4>::f() == 4);
-static_assert(A<8>::f() == 5);
+ static_assert(A<3>::f() == 0);
+ static_assert(A<5>::f() == 1);
+ static_assert(A<9>::f() == 2);
+ static_assert(A<2>::f() == 3);
+ static_assert(A<4>::f() == 4);
+ static_assert(A<8>::f() == 5);
-template<>
-constexpr int A<0>::g() { return 2; }
+ template<>
+ constexpr int A<0>::g() { return 2; }
-template<>
-constexpr int A<8>::g() { return 3; }
+ template<>
+ constexpr int A<8>::g() { return 3; }
-template<>
-constexpr int A<6>::g() { return 4; } // expected-error {{ambiguous member function specialization 'A<6>::g' of 'A::g'}}
- // expected-note@#candidate-0 {{member function specialization matches 'g'}}
- // expected-note@#candidate-1 {{member function specialization matches 'g'}}
+ template<>
+ constexpr int A<6>::g() { return 4; } // expected-error {{ambiguous member function specialization 'N0::A<6>::g' of 'N0::A::g'}}
+ // expected-note@#candidate-0 {{member function specialization matches 'g'}}
+ // expected-note@#candidate-1 {{member function specialization matches 'g'}}
-static_assert(A<9>::g() == 0);
-static_assert(A<1>::g() == 1);
-static_assert(A<0>::g() == 2);
-static_assert(A<8>::g() == 3);
+ static_assert(A<9>::g() == 0);
+ static_assert(A<1>::g() == 1);
+ static_assert(A<0>::g() == 2);
+ static_assert(A<8>::g() == 3);
-template<>
-constexpr int A<4>::h() { return 1; }
+ template<>
+ constexpr int A<4>::h() { return 1; }
-template<>
-constexpr int A<0>::h() { return 2; } // expected-error {{out-of-line definition of 'h' does not match any declaration in 'A<0>'}}
+ template<>
+ constexpr int A<0>::h() { return 2; } // expected-error {{out-of-line definition of 'h' does not match any declaration in 'N0::A<0>'}}
-static_assert(A<5>::h() == 0);
-static_assert(A<4>::h() == 1);
+ static_assert(A<5>::h() == 0);
+ static_assert(A<4>::h() == 1);
+} // namespace N0
+
+namespace N1 {
+ template<int I>
+ concept C = I > 0;
+
+ template<int I>
+ concept D = I > 1;
+
+ template<int I>
+ concept E = I > 2;
+
+ template<int I>
+ struct A {
+ void f() requires C<I> && D<I>; // expected-note {{member function specialization matches 'f'}}
+ void f() requires C<I> && E<I>; // expected-note {{member function specialization matches 'f'}}
+ void f() requires C<I> && D<I> && true; // expected-note {{member function specialization matches 'f'}}
+
+ void g() requires C<I> && E<I>; // expected-note {{member function specialization matches 'g'}}
+ void g() requires C<I> && D<I>; // expected-note {{member function specialization matches 'g'}}
+ void g() requires C<I> && D<I> && true; // expected-note {{member function specialization matches 'g'}}
+ };
+
+ template<>
+ void A<3>::f(); // expected-error {{ambiguous member function specialization 'N1::A<3>::f' of 'N1::A::f'}}
+
+ template<>
+ void A<3>::g(); // expected-error {{ambiguous member function specialization 'N1::A<3>::g' of 'N1::A::g'}}
+} // namespace N1
>From a3cd1dc9293529bfbc2e6a842c83043f0eef98b2 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Fri, 2 Aug 2024 13:33:38 -0400
Subject: [PATCH 3/5] [FOLD] use local UnresolvedSet to store candidates
---
clang/lib/Sema/SemaDecl.cpp | 8 ++++--
clang/lib/Sema/SemaTemplate.cpp | 51 ++++++++++++++++-----------------
2 files changed, 29 insertions(+), 30 deletions(-)
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 1f4bfa247b544..117278e4fa827 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -7964,8 +7964,9 @@ NamedDecl *Sema::ActOnVariableDeclarator(
D.setRedeclaration(CheckVariableDeclaration(NewVD, Previous));
} else {
// If this is an explicit specialization of a static data member, check it.
- if (IsMemberSpecialization && !IsVariableTemplateSpecialization &&
- !NewVD->isInvalidDecl() && CheckMemberSpecialization(NewVD, Previous))
+ if (IsMemberSpecialization && !IsVariableTemplate &&
+ !IsVariableTemplateSpecialization && !NewVD->isInvalidDecl() &&
+ CheckMemberSpecialization(NewVD, Previous))
NewVD->setInvalidDecl();
// Merge the decl with the existing one if appropriate.
@@ -10466,7 +10467,8 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
Previous))
NewFD->setInvalidDecl();
}
- } else if (isMemberSpecialization && isa<CXXMethodDecl>(NewFD)) {
+ } else if (isMemberSpecialization && !FunctionTemplate &&
+ isa<CXXMethodDecl>(NewFD)) {
if (CheckMemberSpecialization(NewFD, Previous))
NewFD->setInvalidDecl();
}
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index c96925ba73ae2..6e2cb59b2b3db 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -9051,7 +9051,8 @@ bool Sema::CheckFunctionTemplateSpecialization(
bool
Sema::CheckMemberSpecialization(NamedDecl *Member, LookupResult &Previous) {
- assert(!isa<TemplateDecl>(Member) && "Only for non-template members");
+ assert(!Member->isTemplateDecl() && !Member->getDescribedTemplate() &&
+ "Only for non-template members");
// Try to find the member we are instantiating.
NamedDecl *FoundInstantiation = nullptr;
@@ -9062,50 +9063,46 @@ Sema::CheckMemberSpecialization(NamedDecl *Member, LookupResult &Previous) {
if (Previous.empty()) {
// Nowhere to look anyway.
} else if (FunctionDecl *Function = dyn_cast<FunctionDecl>(Member)) {
- LookupResult::Filter Filter = Previous.makeFilter();
- while (Filter.hasNext()) {
- auto *Method =
- dyn_cast<CXXMethodDecl>(Filter.next()->getUnderlyingDecl());
- // Discard any candidates that aren't member functions.
- if (!Method) {
- Filter.erase();
+ UnresolvedSet<8> Candidates;
+ for (NamedDecl *Candidate : Previous) {
+ auto *Method = dyn_cast<CXXMethodDecl>(Candidate->getUnderlyingDecl());
+ // Ignore any candidates that aren't member functions.
+ if (!Method)
continue;
- }
QualType Adjusted = Function->getType();
if (!hasExplicitCallingConv(Adjusted))
Adjusted = adjustCCAndNoReturn(Adjusted, Method->getType());
- // Discard any candidates with the wrong type.
+ // Ignore any candidates with the wrong type.
// This doesn't handle deduced return types, but both function
// declarations should be undeduced at this point.
- if (!Context.hasSameType(Adjusted, Method->getType())) {
- Filter.erase();
+ // FIXME: The exception specification should probably be ignored when
+ // comparing the types.
+ if (!Context.hasSameType(Adjusted, Method->getType()))
continue;
- }
- // Discard any candidates with unsatisfied constraints.
+ // Ignore any candidates with unsatisfied constraints.
if (ConstraintSatisfaction Satisfaction;
Method->getTrailingRequiresClause() &&
(CheckFunctionConstraints(Method, Satisfaction,
/*UsageLoc=*/Member->getLocation(),
/*ForOverloadResolution=*/true) ||
- !Satisfaction.IsSatisfied)) {
- Filter.erase();
+ !Satisfaction.IsSatisfied))
continue;
- }
+
+ Candidates.addDecl(Candidate);
}
- Filter.done();
- // If we have no candidates left after filtering, we are done.
- if (Previous.empty())
+ // If we have no viable candidates left after filtering, we are done.
+ if (Candidates.empty())
return false;
// Find the function that is more constrained than every other function it
// has been compared to.
- UnresolvedSetIterator Best = Previous.begin();
+ UnresolvedSetIterator Best = Candidates.begin();
CXXMethodDecl *BestMethod = nullptr;
- for (UnresolvedSetIterator I = Previous.begin(), E = Previous.end(); I != E;
- ++I) {
+ for (UnresolvedSetIterator I = Candidates.begin(), E = Candidates.end();
+ I != E; ++I) {
auto *Method = cast<CXXMethodDecl>(I->getUnderlyingDecl());
if (I == Best ||
getMoreConstrainedFunction(Method, BestMethod) == Method) {
@@ -9119,10 +9116,10 @@ Sema::CheckMemberSpecialization(NamedDecl *Member, LookupResult &Previous) {
InstantiatedFrom = BestMethod->getInstantiatedFromMemberFunction();
MSInfo = BestMethod->getMemberSpecializationInfo();
- // Make sure the best candidate is more specialized than all of the others.
+ // Make sure the best candidate is more constrained than all of the others.
bool Ambiguous = false;
- for (UnresolvedSetIterator I = Previous.begin(), E = Previous.end(); I != E;
- ++I) {
+ for (UnresolvedSetIterator I = Candidates.begin(), E = Candidates.end();
+ I != E; ++I) {
auto *Method = cast<CXXMethodDecl>(I->getUnderlyingDecl());
if (I != Best &&
getMoreConstrainedFunction(Method, BestMethod) != BestMethod) {
@@ -9134,7 +9131,7 @@ Sema::CheckMemberSpecialization(NamedDecl *Member, LookupResult &Previous) {
if (Ambiguous) {
Diag(Member->getLocation(), diag::err_function_member_spec_ambiguous)
<< Member << (InstantiatedFrom ? InstantiatedFrom : Instantiation);
- for (NamedDecl *Candidate : Previous) {
+ for (NamedDecl *Candidate : Candidates) {
Candidate = Candidate->getUnderlyingDecl();
Diag(Candidate->getLocation(), diag::note_function_member_spec_matched)
<< Candidate;
>From eaaf40916f6326d67b309b6396cd6f038713683c Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Fri, 2 Aug 2024 13:41:47 -0400
Subject: [PATCH 4/5] [FOLD] remove useless condition
---
clang/lib/Sema/SemaDecl.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 117278e4fa827..bab32b99b75be 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -10467,8 +10467,7 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
Previous))
NewFD->setInvalidDecl();
}
- } else if (isMemberSpecialization && !FunctionTemplate &&
- isa<CXXMethodDecl>(NewFD)) {
+ } else if (isMemberSpecialization && !FunctionTemplate) {
if (CheckMemberSpecialization(NewFD, Previous))
NewFD->setInvalidDecl();
}
>From 05cd241a1a9621cc128cc923f87a84f9b60f21fa Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Tue, 6 Aug 2024 11:13:11 -0400
Subject: [PATCH 5/5] [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 68c3c3b685675..d322da81723a5 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -190,6 +190,7 @@ Bug Fixes to C++ Support
- Fix init-capture packs having a size of one before being instantiated. (#GH63677)
- Clang now preserves the unexpanded flag in a lambda transform used for pack expansion. (#GH56852), (#GH85667),
(#GH99877).
+- Fixed a bug when diagnosing ambiguous explicit specializations of constrained member functions.
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
More information about the cfe-commits
mailing list