[Clang] Do not assume a perfect match is a better match than a non-template non-perfect match#149504

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

cor3ntin merged 2 commits intollvm:mainfromcor3ntin:corentin/overload_resolution

Jul 18, 2025

Merged

[Clang] Do not assume a perfect match is a better match than a non-template non-perfect match#149504

cor3ntin merged 2 commits intollvm:mainfromcor3ntin:corentin/overload_resolution

Jul 18, 2025

+34 −45

Conversation

Copy link

Contributor

cor3ntin commentedJul 18, 2025

This fixes a regression introduced by the "perfect match" overload resolution mechanism introduced in8c5a307.

This does regress the performance noticeably (-0.7% for a stage 2 build), 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

[Clang] Do not assume a perfect match is a better match than a non-te…

f34d340

…mplatenon-perfect match.This fixes a regression introduced by the "perfect match" overloadresolution mechanism introduced in8c5a307.Fixesllvm#147374

llvmbot added clang

Clang issues not falling into any other category

clang:frontendLanguage frontend issues, e.g. anything involving "Sema" labels

Jul 18, 2025

cor3ntin requested review fromerichkeane andAaronBallman

July 18, 2025 12:12

Copy link

Member

llvmbot commentedJul 18, 2025

@llvm/pr-subscribers-clang

Author: Corentin Jabot (cor3ntin)

Changes

This fixes a regression introduced by the "perfect match" overload resolution mechanism introduced in8c5a307.

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 --git a/clang/include/clang/Sema/Overload.h b/clang/include/clang/Sema/Overload.hindex 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.cppindex 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.cppindex 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')}}+}++}