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 <sdkryst...@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 <sdkryst...@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 <sdkryst...@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 <sdkryst...@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 <sdkryst...@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 ^^^^^^^^^^^^^^^^^^^^^^^^^ _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits