Skip to content

Commit 724cfce

Browse files
authored
[Clang] Do not assume a perfect match is a better match than a non-template non-perfect match (#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
1 parent 151fffc commit 724cfce

File tree

3 files changed

+34
-45
lines changed

3 files changed

+34
-45
lines changed

clang/include/clang/Sema/Overload.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1491,8 +1491,6 @@ class Sema;
14911491
OverloadingResult
14921492
BestViableFunctionImpl(Sema &S, SourceLocation Loc,
14931493
OverloadCandidateSet::iterator &Best);
1494-
void PerfectViableFunction(Sema &S, SourceLocation Loc,
1495-
OverloadCandidateSet::iterator &Best);
14961494
};
14971495

14981496
bool isBetterOverloadCandidate(Sema &S, const OverloadCandidate &Cand1,

clang/lib/Sema/SemaOverload.cpp

Lines changed: 6 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -11354,55 +11354,18 @@ OverloadingResult OverloadCandidateSet::BestViableFunction(Sema &S,
1135411354
DeferredCandidatesCount != 0 && !ResolutionByPerfectCandidateIsDisabled;
1135511355

1135611356
if (TwoPhaseResolution) {
11357-
11358-
PerfectViableFunction(S, Loc, Best);
11359-
if (Best != end())
11360-
return ResultForBestCandidate(Best);
11357+
OverloadingResult Res = BestViableFunctionImpl(S, Loc, Best);
11358+
if (Best != end() && Best->isPerfectMatch(S.Context)) {
11359+
if (!(HasDeferredTemplateConstructors &&
11360+
isa_and_nonnull<CXXConversionDecl>(Best->Function)))
11361+
return Res;
11362+
}
1136111363
}
1136211364

1136311365
InjectNonDeducedTemplateCandidates(S);
1136411366
return BestViableFunctionImpl(S, Loc, Best);
1136511367
}
1136611368

11367-
void OverloadCandidateSet::PerfectViableFunction(
11368-
Sema &S, SourceLocation Loc, OverloadCandidateSet::iterator &Best) {
11369-
11370-
Best = end();
11371-
for (auto It = Candidates.begin(); It != Candidates.end(); ++It) {
11372-
11373-
if (!It->isPerfectMatch(S.getASTContext()))
11374-
continue;
11375-
11376-
// We found a suitable conversion function
11377-
// but if there is a template constructor in the target class
11378-
// we might prefer that instead.
11379-
if (HasDeferredTemplateConstructors &&
11380-
isa_and_nonnull<CXXConversionDecl>(It->Function)) {
11381-
Best = end();
11382-
break;
11383-
}
11384-
11385-
if (Best == end()) {
11386-
Best = It;
11387-
continue;
11388-
}
11389-
if (Best->Function && It->Function) {
11390-
FunctionDecl *D =
11391-
S.getMoreConstrainedFunction(Best->Function, It->Function);
11392-
if (D == nullptr) {
11393-
Best = end();
11394-
break;
11395-
}
11396-
if (D == It->Function)
11397-
Best = It;
11398-
continue;
11399-
}
11400-
// ambiguous
11401-
Best = end();
11402-
break;
11403-
}
11404-
}
11405-
1140611369
OverloadingResult OverloadCandidateSet::BestViableFunctionImpl(
1140711370
Sema &S, SourceLocation Loc, OverloadCandidateSet::iterator &Best) {
1140811371

clang/test/SemaCXX/overload-resolution-deferred-templates.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,3 +283,31 @@ void f() {
283283
}
284284

285285
#endif
286+
287+
namespace GH147374 {
288+
289+
struct String {};
290+
template <typename T> void operator+(T, String &&) = delete;
291+
292+
struct Bar {
293+
void operator+(String) const; // expected-note {{candidate function}}
294+
friend void operator+(Bar, String) {}; // expected-note {{candidate function}}
295+
};
296+
297+
struct Baz {
298+
void operator+(String); // expected-note {{candidate function}}
299+
friend void operator+(Baz, String) {}; // expected-note {{candidate function}}
300+
};
301+
302+
void test() {
303+
Bar a;
304+
String b;
305+
a + b;
306+
//expected-error@-1 {{use of overloaded operator '+' is ambiguous (with operand types 'Bar' and 'String')}}
307+
308+
Baz z;
309+
z + b;
310+
//expected-error@-1 {{use of overloaded operator '+' is ambiguous (with operand types 'Baz' and 'String')}}
311+
}
312+
313+
}

0 commit comments

Comments
 (0)