[clang] [clang] Fix overload resolution ranking of inherited constructors (PR #132830)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: None (offsetof)

<details>
<summary>Changes</summary>

Take parameter types of candidate constructors into account when deciding whether to apply the tiebreaker.

Fixes #<!-- -->121331

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


3 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+3) 
- (modified) clang/lib/Sema/SemaOverload.cpp (+8-9) 
- (modified) clang/test/CXX/drs/cwg22xx.cpp (+14) 


``````````diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f919b66dd0e41..a3d882312056c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -358,6 +358,9 @@ Bug Fixes to C++ Support
 - Fixed a Clang regression in C++20 mode where unresolved dependent call expressions were created inside non-dependent contexts (#GH122892)
 - Clang now emits the ``-Wunused-variable`` warning when some structured bindings are unused
   and the ``[[maybe_unused]]`` attribute is not applied. (#GH125810)
+- Overload resolution tiebreaker for inherited constructors is now only
+  applied if their parameters have the same type, as required by the C++
+  standard. (#GH121331)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 6d8006b35dcf4..f35d272b470cb 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -40,6 +40,7 @@
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/Casting.h"
 #include <algorithm>
 #include <cassert>
 #include <cstddef>
@@ -10724,15 +10725,13 @@ bool clang::isBetterOverloadCandidate(
   //   -- F1 is a constructor for a class D, F2 is a constructor for a base
   //      class B of D, and for all arguments the corresponding parameters of
   //      F1 and F2 have the same type.
-  // FIXME: Implement the "all parameters have the same type" check.
-  bool Cand1IsInherited =
-      isa_and_nonnull<ConstructorUsingShadowDecl>(Cand1.FoundDecl.getDecl());
-  bool Cand2IsInherited =
-      isa_and_nonnull<ConstructorUsingShadowDecl>(Cand2.FoundDecl.getDecl());
-  if (Cand1IsInherited != Cand2IsInherited)
-    return Cand2IsInherited;
-  else if (Cand1IsInherited) {
-    assert(Cand2IsInherited);
+  if (isa_and_nonnull<CXXConstructorDecl>(Cand1.Function) &&
+      isa_and_nonnull<CXXConstructorDecl>(Cand2.Function) &&
+      llvm::equal(Cand1.Function->parameters().take_front(NumArgs),
+                  Cand2.Function->parameters().take_front(NumArgs),
+                  [&](ParmVarDecl *P1, ParmVarDecl *P2) {
+                    return S.Context.hasSameUnqualifiedType(P1->getType(), P2->getType());
+                  })) {
     auto *Cand1Class = cast<CXXRecordDecl>(Cand1.Function->getDeclContext());
     auto *Cand2Class = cast<CXXRecordDecl>(Cand2.Function->getDeclContext());
     if (Cand1Class->isDerivedFrom(Cand2Class))
diff --git a/clang/test/CXX/drs/cwg22xx.cpp b/clang/test/CXX/drs/cwg22xx.cpp
index 8c8ad9f7f74ee..43bab3da171e4 100644
--- a/clang/test/CXX/drs/cwg22xx.cpp
+++ b/clang/test/CXX/drs/cwg22xx.cpp
@@ -169,6 +169,20 @@ B b;
 // since-cxx11-error at -1 {{call to implicitly-deleted default constructor of 'B'}}
 //   since-cxx11-note@#cwg2273-B {{default constructor of 'B' is implicitly deleted because base class 'A' has a deleted default constructor}}
 //   since-cxx11-note@#cwg2273-A {{'A' has been explicitly marked deleted here}}
+
+struct X {
+  X(float); // since-cxx11-note {{candidate inherited constructor}}
+  X(void*, int = 0) = delete;
+};
+
+struct Y : X {
+  using X::X; // since-cxx11-note {{constructor from base class 'X' inherited here}}
+  Y(double); // since-cxx11-note {{candidate constructor}}
+  Y(void* const, long = 1);
+};
+
+Y y = 1; // since-cxx11-error {{conversion from 'int' to 'Y' is ambiguous}}
+Y z = nullptr;
 #endif
 } // namespace cwg2273
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/132830