[Clang] CWG2789 Overload resolution with implicit and explicit object…

[cor3ntin]

… member functions

Implement the resolution to CWG2789 from
https://wiki.edg.com/pub/Wg21kona2023/StrawPolls/p3046r0.html

The DR page is not updated because the issue has not made it to a published list yet.

[llvmbot]

@llvm/pr-subscribers-clang

Author: cor3ntin (cor3ntin)

Changes

… member functions

Implement the resolution to CWG2789 from
https://wiki.edg.com/pub/Wg21kona2023/StrawPolls/p3046r0.html

The DR page is not updated because the issue has not made it to a published list yet.

---

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

3 Files Affected:

• (modified) clang/include/clang/Sema/Sema.h (+6)
• (modified) clang/lib/Sema/SemaOverload.cpp (+55-14)
• (added) clang/test/CXX/drs/dr27xx.cpp (+31)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index f7c9d0e2e6412b7..7579a3256bc37aa 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3849,6 +3849,12 @@ class Sema final {
                                   const FunctionProtoType *NewType,
                                   unsigned *ArgPos = nullptr,
                                   bool Reversed = false);
+
+  bool FunctionNonObjectParamTypesAreEqual(const FunctionDecl *OldFunction,
+                                           const FunctionDecl *NewFunction,
+                                           unsigned *ArgPos = nullptr,
+                                           bool Reversed = false);
+
   void HandleFunctionTypeMismatch(PartialDiagnostic &PDiag,
                                   QualType FromType, QualType ToType);
 
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 9800d7f1c9cfee9..cc69cd1f2862aae 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -3239,6 +3239,28 @@ bool Sema::FunctionParamTypesAreEqual(const FunctionProtoType *OldType,
                                     NewType->param_types(), ArgPos, Reversed);
 }
 
+bool Sema::FunctionNonObjectParamTypesAreEqual(const FunctionDecl *OldFunction,
+                                               const FunctionDecl *NewFunction,
+                                               unsigned *ArgPos,
+                                               bool Reversed) {
+
+  if (OldFunction->getNumNonObjectParams() !=
+      NewFunction->getNumNonObjectParams())
+    return false;
+
+  unsigned OldIgnore =
+      unsigned(OldFunction->hasCXXExplicitFunctionObjectParameter());
+  unsigned NewIgnore =
+      unsigned(NewFunction->hasCXXExplicitFunctionObjectParameter());
+
+  auto *OldPT = cast<FunctionProtoType>(OldFunction->getFunctionType());
+  auto *NewPT = cast<FunctionProtoType>(NewFunction->getFunctionType());
+
+  return FunctionParamTypesAreEqual(OldPT->param_types().slice(OldIgnore),
+                                    NewPT->param_types().slice(NewIgnore),
+                                    ArgPos, Reversed);
+}
+
 /// CheckPointerConversion - Check the pointer conversion from the
 /// expression From to the type ToType. This routine checks for
 /// ambiguous or inaccessible derived-to-base pointer
@@ -10121,22 +10143,41 @@ static bool haveSameParameterTypes(ASTContext &Context, const FunctionDecl *F1,
 
 /// We're allowed to use constraints partial ordering only if the candidates
 /// have the same parameter types:
-/// [over.match.best]p2.6
-/// F1 and F2 are non-template functions with the same parameter-type-lists,
-/// and F1 is more constrained than F2 [...]
+/// [over.match.best.general]p2.6
+/// F1 and F2 are non-template functions with the same
+/// non-object-parameter-type-lists, and F1 is more constrained than F2 [...]
 static bool sameFunctionParameterTypeLists(Sema &S,
-                                          const OverloadCandidate &Cand1,
-                                          const OverloadCandidate &Cand2) {
-  if (Cand1.Function && Cand2.Function) {
-    auto *PT1 = cast<FunctionProtoType>(Cand1.Function->getFunctionType());
-    auto *PT2 = cast<FunctionProtoType>(Cand2.Function->getFunctionType());
-    if (PT1->getNumParams() == PT2->getNumParams() &&
-        PT1->isVariadic() == PT2->isVariadic() &&
-        S.FunctionParamTypesAreEqual(PT1, PT2, nullptr,
-                                     Cand1.isReversed() ^ Cand2.isReversed()))
-      return true;
+                                           const OverloadCandidate &Cand1,
+                                           const OverloadCandidate &Cand2) {
+  if (!Cand1.Function || !Cand2.Function)
+    return false;
+
+  auto *Fn1 = Cand1.Function;
+  auto *Fn2 = Cand2.Function;
+
+  if (Fn1->isVariadic() != Fn1->isVariadic())
+    return false;
+
+  if (!S.FunctionNonObjectParamTypesAreEqual(
+          Fn1, Fn2, nullptr, Cand1.isReversed() ^ Cand2.isReversed()))
+    return false;
+
+  auto *Mem1 = dyn_cast<CXXMethodDecl>(Fn1);
+  auto *Mem2 = dyn_cast<CXXMethodDecl>(Fn2);
+  if (Mem1 && Mem2) {
+    // if they are member functions, both are direct members of the same class,
+    // and
+    if (Mem1->getParent() != Mem2->getParent())
+      return false;
+    // if both are non-static member functions, they have the same types for
+    // their object parameters
+    if (Mem1->isInstance() && Mem2->isInstance() &&
+        !S.getASTContext().hasSameType(
+            Mem1->getFunctionObjectParameterReferenceType(),
+            Mem1->getFunctionObjectParameterReferenceType()))
+      return false;
   }
-  return false;
+  return true;
 }
 
 /// isBetterOverloadCandidate - Determines whether the first overload
diff --git a/clang/test/CXX/drs/dr27xx.cpp b/clang/test/CXX/drs/dr27xx.cpp
new file mode 100644
index 000000000000000..20504aeac436109
--- /dev/null
+++ b/clang/test/CXX/drs/dr27xx.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -std=c++2c -verify %s
+
+
+namespace dr2789 { // dr2789: 18 open
+
+template <typename T = int>
+struct Base {
+    constexpr void g(); // expected-note {{candidate function}}
+};
+
+template <typename T = int>
+struct Base2 {
+    constexpr void g() requires true;  // expected-note {{candidate function}}
+};
+
+template <typename T = int>
+struct S : Base<T>, Base2<T> {
+    constexpr void f();                      // #1
+    constexpr void f(this S&) requires true{}; // #2
+
+    using Base<T>::g;
+    using Base2<T>::g;
+};
+
+void test() {
+    S<> s;
+    s.f();
+    s.g(); // expected-error {{call to member function 'g' is ambiguous}}
+}
+
+}

[Endilll]

I believe our current best practices on handling errors with notes in DR tests are showcased here:

llvm-project/clang/test/CXX/drs/dr13xx.cpp

Line 505 in fe3c421

// expected-error@#dr1397-struct-A {{default member initializer for 'p' needed within definition of enclosing class 'A' outside of member functions}}

Notes being scattered across the test makes it harder to match the test against compiler output on e.g. Compiler Explorer.

[AaronBallman]

Changes LGTM modulo some comments on the review.

[AaronBallman]

Please spell out the types explicitly as they're not spelled by the initializer.

[cor3ntin]

@Endilll I addressed your feedback!