From 90648944f6e27de2b150f33887798c221bbb19b4 Mon Sep 17 00:00:00 2001 From: Corentin Jabot Date: Tue, 8 Jul 2025 11:53:21 +0200 Subject: [PATCH 1/3] [Clang] Better handle overload of explicit object member functions ```cpp struct A { constexpr int f() const { return 1; } // #1 }; struct B : A { using A::f; constexpr int f(this A) { return 2; } }; static_assert(B{}.f() == 1); ``` In the above scenario, we failed to realize the second `f` was an overload. This was caused by a few compounding deficiences in how we handle reference qualified member objects (and non-qualified implicit object member functions). We also failed to consider that `#1` has a better conversion sequence. Fixes #99902 --- clang/docs/ReleaseNotes.rst | 1 + clang/lib/Sema/SemaOverload.cpp | 23 ++++++++---- clang/test/SemaCXX/cxx2b-deducing-this.cpp | 43 ++++++++++++++++++++++ 3 files changed, 60 insertions(+), 7 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index a6be59f1d6bd7..ba0c9cdcd11a6 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -903,6 +903,7 @@ Bug Fixes to C++ Support - Fix a bug where private access specifier of overloaded function not respected. (#GH107629) - Correctly handle allocations in the condition of a ``if constexpr``.(#GH120197) (#GH134820) - Fixed a crash when handling invalid member using-declaration in C++20+ mode. (#GH63254) +- Fix the handling of overload sets mixing explicit and implicit member functions. (#GH99902) Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index 34f512b13d6a8..c859268afa3b2 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -1483,8 +1483,8 @@ static bool IsOverloadOrOverrideImpl(Sema &SemaRef, FunctionDecl *New, if (BS.Ty != DS.Ty) return false; - if (Base->isLValueReferenceType()) - return D->isLValueReferenceType(); + if (Base->isLValueReferenceType() || D->isLValueReferenceType()) + return Base->isLValueReferenceType() == D->isLValueReferenceType(); return Base->isRValueReferenceType() == D->isRValueReferenceType(); }; @@ -1536,10 +1536,19 @@ static bool IsOverloadOrOverrideImpl(Sema &SemaRef, FunctionDecl *New, !F->isExplicitObjectMemberFunction(); }; - if (IsImplicitWithNoRefQual(Old) != IsImplicitWithNoRefQual(New) && - CompareType(OldObjectType.getNonReferenceType(), - NewObjectType.getNonReferenceType())) - return true; + if ((Old->getRefQualifier() != RQ_None || + Old->isExplicitObjectMemberFunction()) && + IsImplicitWithNoRefQual(New) && + OldObjectType->isRValueReferenceType()) + NewObjectType = + SemaRef.getASTContext().getRValueReferenceType(NewObjectType); + else if ((New->getRefQualifier() != RQ_None || + New->isExplicitObjectMemberFunction()) && + IsImplicitWithNoRefQual(Old) && + NewObjectType->isRValueReferenceType()) + OldObjectType = + SemaRef.getASTContext().getRValueReferenceType(OldObjectType); + return CompareType(OldObjectType, NewObjectType); }(OldMethod, NewMethod); @@ -5922,7 +5931,7 @@ static ImplicitConversionSequence TryObjectArgumentInitialization( if (Method->isExplicitObjectMemberFunction()) { if (ExplicitParameterType.isNull()) ExplicitParameterType = Method->getFunctionObjectParameterReferenceType(); - OpaqueValueExpr TmpExpr(Loc, FromType.getNonReferenceType(), + OpaqueValueExpr TmpExpr(Loc, FromType, ValueKindFromClassification(FromClassification)); ImplicitConversionSequence ICS = TryCopyInitialization( S, &TmpExpr, ExplicitParameterType, SuppressUserConversion, diff --git a/clang/test/SemaCXX/cxx2b-deducing-this.cpp b/clang/test/SemaCXX/cxx2b-deducing-this.cpp index 2286da8d1c0e5..a4a5224782f5e 100644 --- a/clang/test/SemaCXX/cxx2b-deducing-this.cpp +++ b/clang/test/SemaCXX/cxx2b-deducing-this.cpp @@ -1168,3 +1168,46 @@ struct S { bool g() { return f(); } // expected-error {{no viable conversion from returned value of type 'S' to function return type 'bool'}} }; } + +namespace GH99902 { +struct A { + constexpr int f() const { return 1; } + constexpr int g() const & { return 1; } + constexpr int h() const && { return 1; } + + constexpr int i() { return 1; } + constexpr int j() & { return 1; } + constexpr int k() && { return 1; } + +}; + +struct B : A { + using A::f; + using A::g; + using A::h; + using A::i; + using A::j; + using A::k; + constexpr int f(this A) { return 2; } + constexpr int g(this A) { return 2; } + constexpr int h(this A) { return 2; } + constexpr int i(this A) { return 2; } + constexpr int j(this A) { return 2; } + constexpr int k(this A) { return 2; } +}; + +constexpr B b; +static_assert(B{}.f() == 1); +static_assert(B{}.g() == 1); +static_assert(B{}.h() == 1); +static_assert(B{}.i() == 1); +static_assert(B{}.j() == 2); +static_assert(B{}.k() == 1); +static_assert(b.f() == 1); +static_assert(b.g() == 1); +static_assert(b.h() == 2); +static_assert(b.i() == 2); +static_assert(b.j() == 2); +static_assert(b.k() == 2); + +} From 9f1e263b2db19dc28750c3f254e46d93752a4b77 Mon Sep 17 00:00:00 2001 From: Corentin Jabot Date: Tue, 8 Jul 2025 16:01:53 +0200 Subject: [PATCH 2/3] simplify the code a bit --- clang/lib/Sema/SemaOverload.cpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index c859268afa3b2..fcbed7d1949be 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -1535,16 +1535,11 @@ static bool IsOverloadOrOverrideImpl(Sema &SemaRef, FunctionDecl *New, return F->getRefQualifier() == RQ_None && !F->isExplicitObjectMemberFunction(); }; - - if ((Old->getRefQualifier() != RQ_None || - Old->isExplicitObjectMemberFunction()) && - IsImplicitWithNoRefQual(New) && + if (!IsImplicitWithNoRefQual(Old) && IsImplicitWithNoRefQual(New) && OldObjectType->isRValueReferenceType()) NewObjectType = SemaRef.getASTContext().getRValueReferenceType(NewObjectType); - else if ((New->getRefQualifier() != RQ_None || - New->isExplicitObjectMemberFunction()) && - IsImplicitWithNoRefQual(Old) && + else if (!IsImplicitWithNoRefQual(New) && IsImplicitWithNoRefQual(Old) && NewObjectType->isRValueReferenceType()) OldObjectType = SemaRef.getASTContext().getRValueReferenceType(OldObjectType); From 1e01ff6e09c31ffc21d7693385dd74c10cd96f98 Mon Sep 17 00:00:00 2001 From: Corentin Jabot Date: Wed, 9 Jul 2025 19:30:01 +0200 Subject: [PATCH 3/3] add comment --- clang/lib/Sema/SemaOverload.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index fcbed7d1949be..79ad2be21702f 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -1535,6 +1535,10 @@ static bool IsOverloadOrOverrideImpl(Sema &SemaRef, FunctionDecl *New, return F->getRefQualifier() == RQ_None && !F->isExplicitObjectMemberFunction(); }; + + // If one of the two function has no ref qualifier and the other one + // has a rvalue reference object, we need to adjust their types + // so that both functions are rvalue-reference qualified. if (!IsImplicitWithNoRefQual(Old) && IsImplicitWithNoRefQual(New) && OldObjectType->isRValueReferenceType()) NewObjectType =