[Clang] Do not treat Foo -> const Foo conversion sequences as perfect

[cor3ntin]

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

Note that GCC allows the ambiguity between a const and non-const candidate to be resolved. But this patch focuses on restoring the Clang 20 behavior, and to fix the cases which we did resolve incorrectly.

Fixes #147374

[llvmbot]

@llvm/pr-subscribers-clang

Author: Corentin Jabot (cor3ntin)

Changes

For implicit object arguments.
This fixes a regression introduced by the "perfect match" overload resolusion mechanism introduced in #8c5a307.

Note that GCC allows the ambiguity between a const and non-const candidate to be resolved. But this patch focuses on restoring the Clang 20 behavior, and to fix the cases which we did resolve incorrectly.

Fixes #147374

---

Full diff: https://github.com/llvm/llvm-project/pull/148613.diff

3 Files Affected:

• (modified) clang/include/clang/Sema/Overload.h (+9-3)
• (modified) clang/lib/Sema/SemaOverload.cpp (+17-5)
• (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.h index a70335bef9dd4..bc42e1684ba99 100644 --- a/clang/include/clang/Sema/Overload.h +++ b/clang/include/clang/Sema/Overload.h @@ -350,6 +350,11 @@ class Sema; LLVM_PREFERRED_TYPE(bool) unsigned BindsToRvalue : 1; + /// Whether this was an identity conversion with qualification + /// conversion for the implicit object argument. + LLVM_PREFERRED_TYPE(bool) + unsigned IsImplicitObjectArgumentQualificationConversion : 1; + /// Whether this binds an implicit object argument to a /// non-static member function without a ref-qualifier. LLVM_PREFERRED_TYPE(bool) @@ -448,11 +453,12 @@ class Sema; #endif return true; } - if (!C.hasSameType(getFromType(), getToType(2))) - return false; if (BindsToRvalue && IsLvalueReference) return false; - return true; + if (IsImplicitObjectArgumentQualificationConversion) { + return C.hasSameUnqualifiedType(getFromType(), getToType(2)); + } + return C.hasSameType(getFromType(), getToType(2)); } ImplicitConversionRank getRank() const; diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index 7af3acacb5ba6..cf6e7ed15a46a 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -245,6 +245,7 @@ void StandardConversionSequence::setAsIdentityConversion() { IsLvalueReference = true; BindsToFunctionLvalue = false; BindsToRvalue = false; + IsImplicitObjectArgumentQualificationConversion = false; BindsImplicitObjectArgumentWithoutRefQualifier = false; ObjCLifetimeConversionBinding = false; FromBracedInitList = false; @@ -5305,10 +5306,10 @@ TryReferenceInit(Sema &S, Expr *Init, QualType DeclType, // FIXME: As a speculative fix to a defect introduced by CWG2352, we rank // a reference binding that performs a non-top-level qualification // conversion as a qualification conversion, not as an identity conversion. - ICS.Standard.Third = (RefConv & - Sema::ReferenceConversions::NestedQualification) - ? ICK_Qualification - : ICK_Identity; + ICS.Standard.Third = + (RefConv & Sema::ReferenceConversions::NestedQualification) + ? ICK_Qualification + : ICK_Identity; ICS.Standard.setFromType(T2); ICS.Standard.setToType(0, T2); ICS.Standard.setToType(1, T1); @@ -5317,6 +5318,7 @@ TryReferenceInit(Sema &S, Expr *Init, QualType DeclType, ICS.Standard.DirectBinding = BindsDirectly; ICS.Standard.IsLvalueReference = !isRValRef; ICS.Standard.BindsToFunctionLvalue = T2->isFunctionType(); + ICS.Standard.IsImplicitObjectArgumentQualificationConversion = false; ICS.Standard.BindsToRvalue = InitCategory.isRValue(); ICS.Standard.BindsImplicitObjectArgumentWithoutRefQualifier = false; ICS.Standard.ObjCLifetimeConversionBinding = @@ -5496,6 +5498,7 @@ TryReferenceInit(Sema &S, Expr *Init, QualType DeclType, ICS.Standard.IsLvalueReference = !isRValRef; ICS.Standard.BindsToFunctionLvalue = false; ICS.Standard.BindsToRvalue = true; + ICS.Standard.IsImplicitObjectArgumentQualificationConversion = false; ICS.Standard.BindsImplicitObjectArgumentWithoutRefQualifier = false; ICS.Standard.ObjCLifetimeConversionBinding = false; } else if (ICS.isUserDefined()) { @@ -5518,6 +5521,8 @@ TryReferenceInit(Sema &S, Expr *Init, QualType DeclType, ICS.UserDefined.After.IsLvalueReference = !isRValRef; ICS.UserDefined.After.BindsToFunctionLvalue = false; ICS.UserDefined.After.BindsToRvalue = !LValRefType; + ICS.UserDefined.After.IsImplicitObjectArgumentQualificationConversion = + false; ICS.UserDefined.After.BindsImplicitObjectArgumentWithoutRefQualifier = false; ICS.UserDefined.After.ObjCLifetimeConversionBinding = false; ICS.UserDefined.After.FromBracedInitList = false; @@ -5802,6 +5807,7 @@ TryListConversion(Sema &S, InitListExpr *From, QualType ToType, StandardConversionSequence &SCS = Result.isStandard() ? Result.Standard : Result.UserDefined.After; SCS.ReferenceBinding = true; + SCS.IsImplicitObjectArgumentQualificationConversion = false; SCS.IsLvalueReference = ToType->isLValueReferenceType(); SCS.BindsToRvalue = true; SCS.BindsToFunctionLvalue = false; @@ -5999,8 +6005,12 @@ static ImplicitConversionSequence TryObjectArgumentInitialization( // affects the conversion rank. QualType ClassTypeCanon = S.Context.getCanonicalType(ClassType); ImplicitConversionKind SecondKind; - if (ClassTypeCanon == FromTypeCanon.getLocalUnqualifiedType()) { + bool IsQualificationConversion = false; + if (ImplicitParamType.getCanonicalType() == FromTypeCanon) { SecondKind = ICK_Identity; + } else if (ClassTypeCanon == FromTypeCanon.getLocalUnqualifiedType()) { + SecondKind = ICK_Identity; + IsQualificationConversion = true; } else if (S.IsDerivedFrom(Loc, FromType, ClassType)) { SecondKind = ICK_Derived_To_Base; } else if (!Method->isExplicitObjectMemberFunction()) { @@ -6041,6 +6051,8 @@ static ImplicitConversionSequence TryObjectArgumentInitialization( ICS.Standard.setFromType(FromType); ICS.Standard.setAllToTypes(ImplicitParamType); ICS.Standard.ReferenceBinding = true; + ICS.Standard.IsImplicitObjectArgumentQualificationConversion = + IsQualificationConversion; ICS.Standard.DirectBinding = true; ICS.Standard.IsLvalueReference = Method->getRefQualifier() != RQ_RValue; ICS.Standard.BindsToFunctionLvalue = false; 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')}} +} + +} 

[erichkeane]

2 nits, else LGTM.

[erichkeane]

Nit: No curleys here.

[erichkeane]

Unrelated change?

[MatzeB]

FYI: This change breaks common thrift-generated code. Reduced example here shows the problem and produces "expression is not assignable" after this change but is accepted by all released gcc and clang versions as far as I can tell. No idea if the code is valid/invalid:

#include <exception> struct TmsException : public std::exception { int _what; const char* what() const noexcept override { return ""; } template<typename..., typename T=int> T& what() & { return _what; } }; int main() { TmsException E; E.what() = 42; } 

[alexfh]

Heads up: this commit causes a huge fallout in our codebase. We'll try to come up with some specific examples tomorrow.

[cor3ntin]

Thanks for the heads up!

I'm looking into it.

Reduced to

struct TmsException { int _what; const char* what() const; template<typename T=int> int& what() & { return _what; } }; int main() { TmsException E; E.what() = 42; }

[bgra8]

Thanks for reverting @cor3ntin !

The fallout @alexfh refers too is caused by code that reduces to something similar to what @MatzeB reported.

[cor3ntin]

Sorry for the inconvenience. It was clearly not the right solution in hindsight. It may take me a while to figure out how to approach a fix to #147374