[Sema] Compare canonical conversion function (#154158)

With lazy template loading, it is possible to find non-canonical
FunctionDecls, depending on when redecl chains are completed. This
is a problem for templated conversion operators that would allow to
call either the copy assignment or the move assignment operator.
This ambiguity is resolved by isBetterReferenceBindingKind (called
from CompareStandardConversionSequences) ranking rvalue refs over
lvalue refs.
    
Unfortunately, this fix is hard to test in isolation without the
changes in #133057 that
make lazy template loading more likely to complete redecl chains
at "inconvenient" times. The added reproducer passes before and
after this commit, but would have failed with the proposed changes
of the linked PR.
    
Kudos to Maksim Ivanov for providing an initial version of the
reproducer that I further simplified.

Author: hahnjo

clang/lib/Sema/SemaOverload.cpp

@@ -4413,14 +4413,23 @@ CompareImplicitConversionSequences(Sema &S, SourceLocation Loc,
     Result = CompareStandardConversionSequences(S, Loc,
                                                 ICS1.Standard, ICS2.Standard);
   else if (ICS1.isUserDefined()) {
+    // With lazy template loading, it is possible to find non-canonical
+    // FunctionDecls, depending on when redecl chains are completed. Make sure
+    // to compare the canonical decls of conversion functions. This avoids
+    // ambiguity problems for templated conversion operators.
+    const FunctionDecl *ConvFunc1 = ICS1.UserDefined.ConversionFunction;
+    if (ConvFunc1)
+      ConvFunc1 = ConvFunc1->getCanonicalDecl();
+    const FunctionDecl *ConvFunc2 = ICS2.UserDefined.ConversionFunction;
+    if (ConvFunc2)
+      ConvFunc2 = ConvFunc2->getCanonicalDecl();
     // User-defined conversion sequence U1 is a better conversion
     // sequence than another user-defined conversion sequence U2 if
     // they contain the same user-defined conversion function or
     // constructor and if the second standard conversion sequence of
     // U1 is better than the second standard conversion sequence of
     // U2 (C++ 13.3.3.2p3).
-    if (ICS1.UserDefined.ConversionFunction ==
-          ICS2.UserDefined.ConversionFunction)
+    if (ConvFunc1 == ConvFunc2)
       Result = CompareStandardConversionSequences(S, Loc,
                                                   ICS1.UserDefined.After,
                                                   ICS2.UserDefined.After);

clang/test/Modules/pr133057.cpp

@@ -0,0 +1,143 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -xc++ -std=c++20 -emit-module -fmodule-name=hf -fno-cxx-modules -fmodules -fno-implicit-modules %t/CMO.cppmap -o %t/WI9.pcm
+// RUN: %clang_cc1 -xc++ -std=c++20 -emit-module -fmodule-name=g -fno-cxx-modules -fmodules -fno-implicit-modules -fmodule-file=%t/WI9.pcm %t/E6H.cppmap -o %t/4BK.pcm
+// RUN: %clang_cc1 -xc++ -std=c++20 -emit-module -fmodule-name=r -fno-cxx-modules -fmodules -fno-implicit-modules -fmodule-file=%t/WI9.pcm %t/HMT.cppmap -o %t/LUM.pcm
+// RUN: %clang_cc1 -xc++ -std=c++20 -emit-module -fmodule-name=q -fno-cxx-modules -fmodules -fno-implicit-modules -fmodule-file=%t/LUM.pcm -fmodule-file=%t/4BK.pcm %t/JOV.cppmap -o %t/9VX.pcm
+// RUN: %clang_cc1 -xc++ -std=c++20 -verify -fsyntax-only -fno-cxx-modules -fmodules -fno-implicit-modules -fmodule-file=%t/9VX.pcm %t/XFD.cc
+
+//--- 2OT.h
+#include "LQ1.h"
+
+namespace ciy {
+namespace xqk {
+template <typename>
+class vum {
+ public:
+  using sc = std::C::wmd;
+  friend bool operator==(vum, vum);
+};
+template <typename>
+class me {
+ public:
+  using vbh = vum<me>;
+  using sc = std::C::vy<vbh>::sc;
+  template <typename db>
+  operator db() { return {}; }
+};
+}  // namespace xqk
+template <typename vus>
+xqk::me<vus> uvo(std::C::wmd, vus);
+}  // namespace ciy
+
+class ua {
+  std::C::wmd kij() {
+    ciy::uvo(kij(), '-');
+    return {};
+  }
+};
+
+//--- 9KF.h
+#include "LQ1.h"
+#include "2OT.h"
+namespace {
+void al(std::C::wmd lou) { std::C::jv<std::C::wmd> yt = ciy::uvo(lou, '/'); }
+}  // namespace
+
+//--- CMO.cppmap
+module "hf" {
+header "LQ1.h"
+}
+
+
+//--- E6H.cppmap
+module "g" {
+export *
+header "2OT.h"
+}
+
+
+//--- HMT.cppmap
+module "r" {
+header "2OT.h"
+}
+
+
+//--- JOV.cppmap
+module "q" {
+header "9KF.h"
+}
+
+
+//--- LQ1.h
+namespace std {
+namespace C {
+template <class zd>
+struct vy : zd {};
+template <class ub>
+struct vy<ub*> {
+  typedef ub jz;
+};
+struct wmd {};
+template <class uo, class zt>
+void sk(uo k, zt gf) {
+  (void)(k != gf);
+}
+template <class uo>
+class fm {
+ public:
+  fm(uo);
+};
+template <class kj, class kju>
+bool operator==(kj, kju);
+template <class epn>
+void afm(epn) {
+  using yp = vy<epn>;
+  if (__is_trivially_copyable(yp)) {
+    sk(fm(epn()), nullptr);
+  }
+}
+template <class ub>
+class jv {
+ public:
+  constexpr void gq();
+  ub *nef;
+};
+template <class ub>
+constexpr void jv<ub>::gq() {
+    afm(nef);
+}
+}  // namespace C
+}  // namespace std
+namespace ciy {
+}  // namespace ciy
+
+//--- XFD.cc
+// expected-no-diagnostics
+#include "LQ1.h"
+#include "2OT.h"
+class wiy {
+ public:
+  std::C::wmd eyb();
+};
+template <typename wpa>
+void i(wpa fg) {
+  std::C::jv<std::C::wmd> zs;
+  zs = ciy::uvo(fg.eyb(), '\n');
+}
+namespace ciy {
+namespace xqk {
+struct sbv;
+std::C::jv<sbv> ns() {
+  std::C::jv<sbv> ubs;
+  ubs.gq();
+  return ubs;
+}
+}  // namespace xqk
+}  // namespace ciy
+void s() {
+  wiy fg;
+  i(fg);
+}