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

Corentin Jabot via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 18 05:12:33 PDT 2025


https://github.com/cor3ntin created https://github.com/llvm/llvm-project/pull/149504

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

>From f34d340f59e742f6e75397043b0bed8faed7d94a Mon Sep 17 00:00:00 2001
From: Corentin Jabot <corentinjabot at gmail.com>
Date: Fri, 18 Jul 2025 10:27:44 +0200
Subject: [PATCH] [Clang] Do not assume a perfect match is a better match than
 a non-template non-perfect match.

This fixes a regression introduced by the "perfect match" overload
resolution mechanism introduced in 8c5a307.

Fixes #147374
---
 clang/include/clang/Sema/Overload.h           |  2 -
 clang/lib/Sema/SemaOverload.cpp               | 51 +++----------------
 ...overload-resolution-deferred-templates.cpp | 28 ++++++++++
 3 files changed, 35 insertions(+), 46 deletions(-)

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 at -1 {{use of overloaded operator '+' is ambiguous (with operand types 'Bar' and 'String')}}
+
+    Baz z;
+    z + b;
+    //expected-error at -1 {{use of overloaded operator '+' is ambiguous (with operand types 'Baz' and 'String')}}
+}
+
+}



More information about the cfe-commits mailing list