llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Corentin Jabot (cor3ntin) <details> <summary>Changes</summary> This fixes a regression introduced by the "perfect match" overload resolution mechanism introduced in 8c5a307. [This does regress the performance noticeably (-0.7% for a stage 2 build)](https://llvm-compile-time-tracker.com/compare.php?from=42d2ae1034b287eb60563c370dbf52c59b66db20&to=82303bbc3e003c937ded498ac9f94f49a3fc3d90&stat=instructions:u), however, the original patch had a +4% performance impact, so we are only losing some of the gain, and this has the benefit of being correct and more robust. Fixes #<!-- -->147374 --- Full diff: https://github.com/llvm/llvm-project/pull/149504.diff 3 Files Affected: - (modified) clang/include/clang/Sema/Overload.h (-2) - (modified) clang/lib/Sema/SemaOverload.cpp (+7-44) - (modified) clang/test/SemaCXX/overload-resolution-deferred-templates.cpp (+28) ``````````diff diff --git a/clang/include/clang/Sema/Overload.h b/clang/include/clang/Sema/Overload.h index a70335bef9dd4..d34a4146ddbd6 100644 --- a/clang/include/clang/Sema/Overload.h +++ b/clang/include/clang/Sema/Overload.h @@ -1491,8 +1491,6 @@ class Sema; OverloadingResult BestViableFunctionImpl(Sema &S, SourceLocation Loc, OverloadCandidateSet::iterator &Best); - void PerfectViableFunction(Sema &S, SourceLocation Loc, - OverloadCandidateSet::iterator &Best); }; bool isBetterOverloadCandidate(Sema &S, const OverloadCandidate &Cand1, diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index 1b54628c5e564..cf9bd1b71ba63 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -11353,56 +11353,19 @@ OverloadingResult OverloadCandidateSet::BestViableFunction(Sema &S, bool TwoPhaseResolution = DeferredCandidatesCount != 0 && !ResolutionByPerfectCandidateIsDisabled; - if (TwoPhaseResolution) { - - PerfectViableFunction(S, Loc, Best); - if (Best != end()) - return ResultForBestCandidate(Best); + if(TwoPhaseResolution) { + OverloadingResult Res = BestViableFunctionImpl(S, Loc, Best); + if (Best != end() && Best->isPerfectMatch(S.Context)) { + if(!(HasDeferredTemplateConstructors && + isa_and_nonnull<CXXConversionDecl>(Best->Function))) + return Res; + } } InjectNonDeducedTemplateCandidates(S); return BestViableFunctionImpl(S, Loc, Best); } -void OverloadCandidateSet::PerfectViableFunction( - Sema &S, SourceLocation Loc, OverloadCandidateSet::iterator &Best) { - - Best = end(); - for (auto It = Candidates.begin(); It != Candidates.end(); ++It) { - - if (!It->isPerfectMatch(S.getASTContext())) - continue; - - // We found a suitable conversion function - // but if there is a template constructor in the target class - // we might prefer that instead. - if (HasDeferredTemplateConstructors && - isa_and_nonnull<CXXConversionDecl>(It->Function)) { - Best = end(); - break; - } - - if (Best == end()) { - Best = It; - continue; - } - if (Best->Function && It->Function) { - FunctionDecl *D = - S.getMoreConstrainedFunction(Best->Function, It->Function); - if (D == nullptr) { - Best = end(); - break; - } - if (D == It->Function) - Best = It; - continue; - } - // ambiguous - Best = end(); - break; - } -} - OverloadingResult OverloadCandidateSet::BestViableFunctionImpl( Sema &S, SourceLocation Loc, OverloadCandidateSet::iterator &Best) { diff --git a/clang/test/SemaCXX/overload-resolution-deferred-templates.cpp b/clang/test/SemaCXX/overload-resolution-deferred-templates.cpp index 46c3670848529..135865c8450f5 100644 --- a/clang/test/SemaCXX/overload-resolution-deferred-templates.cpp +++ b/clang/test/SemaCXX/overload-resolution-deferred-templates.cpp @@ -283,3 +283,31 @@ void f() { } #endif + +namespace GH147374 { + +struct String {}; +template <typename T> void operator+(T, String &&) = delete; + +struct Bar { + void operator+(String) const; // expected-note {{candidate function}} + friend void operator+(Bar, String) {}; // expected-note {{candidate function}} +}; + +struct Baz { + void operator+(String); // expected-note {{candidate function}} + friend void operator+(Baz, String) {}; // expected-note {{candidate function}} +}; + +void test() { + Bar a; + String b; + a + b; + //expected-error@-1 {{use of overloaded operator '+' is ambiguous (with operand types 'Bar' and 'String')}} + + Baz z; + z + b; + //expected-error@-1 {{use of overloaded operator '+' is ambiguous (with operand types 'Baz' and 'String')}} +} + +} `````````` </details> https://github.com/llvm/llvm-project/pull/149504 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
