[Clang] Bypass TAD during overload resolution if a perfect match exists #136018

cor3ntin · 2025-04-16T20:20:27Z

This implements the same overload resolution behavior as GCC,
as described in https://wg21.link/p3606 (section 1-2, not 3)

If during overload resolution, there is a non-template candidate
that would be always be picked - because each of the argument
is a perfect match (ie the source and target types are the same),
we do not perform deduction for any template candidate
that might exists.

The goal is to be able to merge #122423 without being too disruptive.

This change means that the selection of the best viable candidate and
template argument deduction become interleaved.

To avoid rewriting half of Clang we store in OverloadCandidateSet
enough information to be able to deduce template candidates from
OverloadCandidateSet::BestViableFunction. Which means
the lifetime of any object used by template argument must outlive
a call to Add*Template*Candidate.

This two phase resolution is not performed for some initialization
as there are cases where template candidate are better match
in these cases per the standard. It's also bypassed for code completion.

The change has a nice impact on compile times
https://llvm-compile-time-tracker.com/compare.php?from=719b029c16eeb1035da522fd641dfcc4cee6be74&to=bf7041045c9408490c395230047c5461de72fc39&stat=instructions%3Au

Fixes #62096
Fixes #74581

Reapplies #133426

…match exists This implements the same overload resolution behavior as GCC, as described in https://wg21.link/p3606 (section 1-2, not 3) If during overload resolution, there is a non-template candidate that would be always be picked - because each of the argument is a perfect match (ie the source and target types are the same), we do not perform deduction for any template candidate that might exists. The goal is to be able to merge llvm#122423 without being too disruptive. This change means that the selection of the best viable candidate and template argument deduction become interleaved. To avoid rewriting half of Clang we store in `OverloadCandidateSet` enough information to be able to deduce template candidates from `OverloadCandidateSet::BestViableFunction`. Which means the lifetime of any object used by template argument must outlive a call to `Add*Template*Candidate`. This two phase resolution is not performed for some initialization as there are cases where template candidate are better match in these cases per the standard. It's also bypassed for code completion. The change has a nice impact on compile times https://llvm-compile-time-tracker.com/compare.php?from=719b029c16eeb1035da522fd641dfcc4cee6be74&to=bf7041045c9408490c395230047c5461de72fc39&stat=instructions%3Au Fixes llvm#62096 Fixes llvm#74581

… include a conversion

…tion in non-member calls

* Prefer a constructor template over a non-template conversion function

- We find a template conversion candidate - A constructor with a dependent explicit specifier

erichkeane

1 suggestion, 1 question.

clang/include/clang/Sema/Overload.h

erichkeane · 2025-04-16T20:26:14Z

clang/include/clang/Sema/Overload.h

@@ -1005,14 +1005,14 @@ class Sema;

    // An overload is a perfect match if the conversion
    // sequences for each argument are perfect.
-    bool isPerfectMatch(const ASTContext &Ctx) const {
+    bool isPerfectMatch(const ASTContext &Ctx, bool ForConversion) const {


Vastly prefer this take the Kind, then line 1015 describe why conversions are ok when CSK_InitByUserDefinedConversion.

erichkeane

Discussed elsewhere, but in a touch of "lets leave it better than we found it", can we make an assert(HasFinalConversion) everywhere else we access FinalConversion?

erichkeane

Accepted once you get the asserts requested.

erichkeane · 2025-04-16T21:04:36Z

clang/lib/Sema/SemaOverload.cpp

@@ -5175,6 +5178,7 @@ FindConversionForRefInit(Sema &S, ImplicitConversionSequence &ICS,
    if (!Best->FinalConversion.DirectBinding)


What about this 'if' here? Assert probably needs to be above this.

h-vetinari · 2025-04-16T23:35:15Z

clang/docs/ReleaseNotes.rst

@@ -96,6 +96,12 @@ C++ Language Changes
      asm((std::string_view("nop")) ::: (std::string_view("memory")));
    }

+- Clang now implements the changes to overload resolution proposed by section 1 and 2 of
+  `P3606 <https://wg21.link/P3606R0>`_. If a non-template candidate exists in an overload set that is
+  a perfect match (all conversion sequences are identity conversions) template candiates are not instantiated.


Suggested change

a perfect match (all conversion sequences are identity conversions) template candiates are not instantiated.

a perfect match (all conversion sequences are identity conversions) template candidates are not instantiated.

…tch exists" (#136113) Reverts #136018 Still some bots failing https://lab.llvm.org/buildbot/#/builders/52/builds/7643

… perfect match exists" (#136113) Reverts llvm/llvm-project#136018 Still some bots failing https://lab.llvm.org/buildbot/#/builders/52/builds/7643

…ts (llvm#136018) This implements the same overload resolution behavior as GCC, as described in https://wg21.link/p3606 (section 1-2, not 3) If during overload resolution, there is a non-template candidate that would be always be picked - because each of the argument is a perfect match (ie the source and target types are the same), we do not perform deduction for any template candidate that might exists. The goal is to be able to merge llvm#122423 without being too disruptive. This change means that the selection of the best viable candidate and template argument deduction become interleaved. To avoid rewriting half of Clang we store in `OverloadCandidateSet` enough information to be able to deduce template candidates from `OverloadCandidateSet::BestViableFunction`. Which means the lifetime of any object used by template argument must outlive a call to `Add*Template*Candidate`. This two phase resolution is not performed for some initialization as there are cases where template candidate are better match in these cases per the standard. It's also bypassed for code completion. The change has a nice impact on compile times https://llvm-compile-time-tracker.com/compare.php?from=719b029c16eeb1035da522fd641dfcc4cee6be74&to=bf7041045c9408490c395230047c5461de72fc39&stat=instructions%3Au Fixes llvm#62096 Fixes llvm#74581 Reapplies llvm#133426

…tch exists" (llvm#136113) Reverts llvm#136018 Still some bots failing https://lab.llvm.org/buildbot/#/builders/52/builds/7643

fmayer · 2025-04-29T18:24:03Z

clang now fails an assertion on this

class a {              template < typename b > void c();                 void c(int (a::*)(int, int));                 template < typename b > b d(b , b    ) {
  c(&::d);
  }
};

clang-21: [...]/llvm-project/clang/include/clang/Sema/Overload.h:427: bool clang::StandardConversionSequence::isP
erfect(const ASTContext &) const: Assertion `C.hasSameUnqualifiedType(Decay(getFromType()), Decay(getToType(2)))' failed.                      

[...]

 #0 0x000055bc6cac8e58 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /usr/local/google/home/fmayer/large/llvm-project/llvm/lib/Support/Un
ix/Signals.inc:804:13                                                                                                                          
 #1 0x000055bc6cac6f60 llvm::sys::RunSignalHandlers() /usr/local/google/home/fmayer/large/llvm-project/llvm/lib/Support/Signals.cpp:106:18     
 #2 0x000055bc6cac94d1 SignalHandler(int, siginfo_t*, void*) /usr/local/google/home/fmayer/large/llvm-project/llvm/lib/Support/Unix/Signals.inc
:0:3                                                                   
 #3 0x00007f739a849df0 (/lib/x86_64-linux-gnu/libc.so.6+0x3fdf0)
 #4 0x00007f739a89e95c __pthread_kill_implementation ./nptl/pthread_kill.c:44:76
 #5 0x00007f739a849cc2 raise ./signal/../sysdeps/posix/raise.c:27:6
 #6 0x00007f739a8324ac abort ./stdlib/abort.c:81:3
 #7 0x00007f739a832420 __assert_perror_fail ./assert/assert-perr.c:31:1 
 #8 0x000055bc6f432869 clang::StandardConversionSequence::getToType(unsigned int) const /usr/local/google/home/fmayer/large/llvm-project/clang/
include/clang/Sema/Overload.h:400:41
 #9 0x000055bc6f432869 clang::StandardConversionSequence::isPerfect(clang::ASTContext const&) const /usr/local/google/home/fmayer/large/llvm-pr
oject/clang/include/clang/Sema/Overload.h:431:41
#10 0x000055bc6f414f89 clang::OverloadCandidate::isPerfectMatch(clang::ASTContext const&) const /usr/local/google/home/fmayer/large/llvm-projec
t/clang/include/clang/Sema/Overload.h:1026:13
#11 0x000055bc6f414f89 clang::OverloadCandidateSet::PerfectViableFunction(clang::Sema&, clang::SourceLocation, clang::OverloadCandidate*&) /usr
/local/google/home/fmayer/large/llvm-project/clang/lib/Sema/SemaOverload.cpp:11242:14
#12 0x000055bc6f40487f clang::OverloadCandidateSet::BestViableFunction(clang::Sema&, clang::SourceLocation, clang::OverloadCandidate*&) /usr/lo
cal/google/home/fmayer/large/llvm-project/clang/lib/Sema/SemaOverload.cpp:11228:9
#13 0x000055bc6f424779 clang::Sema::BuildCallToMemberFunction(clang::Scope*, clang::Expr*, clang::SourceLocation, llvm::MutableArrayRef<clang::
Expr*>, clang::SourceLocation, clang::Expr*, bool, bool) /usr/local/google/home/fmayer/large/llvm-project/clang/lib/Sema/SemaOverload.cpp:16053
:5
#14 0x000055bc6f07adcb clang::Sema::ActOnCallExpr(clang::Scope*, clang::Expr*, clang::SourceLocation, llvm::MutableArrayRef<clang::Expr*>, clan
g::SourceLocation, clang::Expr*) /usr/local/google/home/fmayer/large/llvm-project/clang/lib/Sema/SemaExpr.cpp:6507:7
#15 0x000055bc6f1e2d3c clang::TreeTransform<(anonymous namespace)::TransformTypos>::RebuildCallExpr(clang::Expr*, clang::SourceLocation, llvm::
MutableArrayRef<clang::Expr*>, clang::SourceLocation, clang::Expr*) /usr/local/google/home/fmayer/large/llvm-project/clang/lib/Sema/TreeTransfo
rm.h:2869:22

@pirama-arumuga-nainar CC

cor3ntin added 25 commits April 16, 2025 17:03

avoid comparing types

d35cfbb

Fix logic

297109c

optimize

f1b0454

optimize again

31ad232

Cleanups, fix tests

f57291b

Fix templight test

bade63f

format

a134656

do not skip templates when initializing by constructors as that might…

057b935

… include a conversion

move and document the special handling of implicit object member func…

721f26c

…tion in non-member calls

Fix typos and cuda tests

a21bc4d

remove redundant check

b205be1

More code simplification

af22b19

fix cuda, add comments

42e8795

add tests

03c157a

* Fix handling of explicit specifiers

0709939

* Prefer a constructor template over a non-template conversion function

slab allocate template candidates

a02299e

fix rvalue to lvalue binding

8ee938c

Disable resolution by perfect match when

bcf08f5

- We find a template conversion candidate - A constructor with a dependent explicit specifier

address Erich's, format, add release notes

8aee255

address more of Erich's feedback

6edcb65

Clarify changelog

7b0c9c6

fix assertion

c621548

cleanups

cfecf5a

fix assertion

8004549

cor3ntin requested a review from erichkeane April 16, 2025 20:20

erichkeane reviewed Apr 16, 2025

View reviewed changes

cor3ntin added 2 commits April 16, 2025 22:46

Cleaner approach

14e0515

Add comment

d4db7e6

erichkeane reviewed Apr 16, 2025

View reviewed changes

erichkeane approved these changes Apr 16, 2025

View reviewed changes

Add asserts

02db83a

erichkeane reviewed Apr 16, 2025

View reviewed changes

fix missplaced assert

17cbd62

h-vetinari reviewed Apr 16, 2025

View reviewed changes

typo

b1ce5e7

cor3ntin merged commit 377ec36 into llvm:main Apr 17, 2025
9 of 12 checks passed

cor3ntin mentioned this pull request Apr 17, 2025

Revert "[Clang] Bypass TAD during overload resolution if a perfect match exists" #136113

Merged

cor3ntin added a commit that referenced this pull request Apr 17, 2025

Revert "[Clang] Bypass TAD during overload resolution if a perfect ma…

2a91d04

…tch exists" (#136113) Reverts #136018 Still some bots failing https://lab.llvm.org/buildbot/#/builders/52/builds/7643

cor3ntin added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label May 12, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Clang] Bypass TAD during overload resolution if a perfect match exists #136018

[Clang] Bypass TAD during overload resolution if a perfect match exists #136018

Uh oh!

cor3ntin commented Apr 16, 2025

Uh oh!

erichkeane left a comment

Uh oh!

Uh oh!

erichkeane Apr 16, 2025

Uh oh!

erichkeane left a comment

Uh oh!

erichkeane left a comment

Uh oh!

erichkeane Apr 16, 2025

Uh oh!

h-vetinari Apr 16, 2025

Uh oh!

Uh oh!

fmayer commented Apr 29, 2025 •

edited

Loading

Uh oh!

Uh oh!

		@@ -5175,6 +5178,7 @@ FindConversionForRefInit(Sema &S, ImplicitConversionSequence &ICS,
		if (!Best->FinalConversion.DirectBinding)

	a perfect match (all conversion sequences are identity conversions) template candiates are not instantiated.
	a perfect match (all conversion sequences are identity conversions) template candidates are not instantiated.

[Clang] Bypass TAD during overload resolution if a perfect match exists #136018

[Clang] Bypass TAD during overload resolution if a perfect match exists #136018

Uh oh!

Conversation

cor3ntin commented Apr 16, 2025

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

erichkeane Apr 16, 2025

Choose a reason for hiding this comment

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

erichkeane Apr 16, 2025

Choose a reason for hiding this comment

Uh oh!

h-vetinari Apr 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fmayer commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

fmayer commented Apr 29, 2025 •

edited

Loading