[clang] Fix CodeComplete crash involving CWG1432 (#129436)

This skips the provisional resolution of CWG1432 just when ordering the
candidates for function call code completion, as otherwise this breaks
some assumptions the implementation makes about how closely related the
candidates are.

As a drive-by, deduplicate the implementation with the one used for
class template partial ordering, and strenghten an assertion which was
previosuly dependent on the order of candidates.

Also add a test for the fix for CWG1432 when partial ordering function
templates, which was otherwise untested.

Fixes #125500

clang/docs/ReleaseNotes.rst

@@ -165,7 +165,7 @@ related warnings within the method body.
       print_status("%s (%#08x)\n");
       // order of %s and %x is swapped but there is no diagnostic
     }
-  
+
   Before the introducion of ``format_matches``, this code cannot be verified
   at compile-time. ``format_matches`` plugs that hole:
 
@@ -233,6 +233,8 @@ Bug Fixes in This Version
   signed char values (#GH102798).
 - Fixed rejects-valid problem when #embed appears in std::initializer_list or
   when it can affect template argument deduction (#GH122306).
+- Fix crash on code completion of function calls involving partial order of function templates
+  (#GH125500).
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

clang/include/clang/Sema/Overload.h

@@ -1265,11 +1265,11 @@ class Sema;
 
   };
 
-  bool isBetterOverloadCandidate(Sema &S,
-                                 const OverloadCandidate &Cand1,
+  bool isBetterOverloadCandidate(Sema &S, const OverloadCandidate &Cand1,
                                  const OverloadCandidate &Cand2,
                                  SourceLocation Loc,
-                                 OverloadCandidateSet::CandidateSetKind Kind);
+                                 OverloadCandidateSet::CandidateSetKind Kind,
+                                 bool PartialOverloading = false);
 
   struct ConstructorInfo {
     DeclAccessPair FoundDecl;

clang/include/clang/Sema/Sema.h

@@ -12617,7 +12617,8 @@ public:
   FunctionTemplateDecl *getMoreSpecializedTemplate(
       FunctionTemplateDecl *FT1, FunctionTemplateDecl *FT2, SourceLocation Loc,
       TemplatePartialOrderingContext TPOC, unsigned NumCallArguments1,
-      QualType RawObj1Ty = {}, QualType RawObj2Ty = {}, bool Reversed = false);
+      QualType RawObj1Ty = {}, QualType RawObj2Ty = {}, bool Reversed = false,
+      bool PartialOverloading = false);
 
   /// Retrieve the most specialized of the given function template
   /// specializations.

clang/lib/Sema/SemaCodeComplete.cpp

@@ -6229,8 +6229,8 @@ static void mergeCandidatesWithResults(
   // Sort the overload candidate set by placing the best overloads first.
   llvm::stable_sort(CandidateSet, [&](const OverloadCandidate &X,
                                       const OverloadCandidate &Y) {
-    return isBetterOverloadCandidate(SemaRef, X, Y, Loc,
-                                     CandidateSet.getKind());
+    return isBetterOverloadCandidate(SemaRef, X, Y, Loc, CandidateSet.getKind(),
+                                     /*PartialOverloading=*/true);
   });
 
   // Add the remaining viable overload candidates as code-completion results.

clang/lib/Sema/SemaOverload.cpp

@@ -10429,7 +10429,8 @@ getMorePartialOrderingConstrained(Sema &S, FunctionDecl *Fn1, FunctionDecl *Fn2,
 /// candidate is a better candidate than the second (C++ 13.3.3p1).
 bool clang::isBetterOverloadCandidate(
     Sema &S, const OverloadCandidate &Cand1, const OverloadCandidate &Cand2,
-    SourceLocation Loc, OverloadCandidateSet::CandidateSetKind Kind) {
+    SourceLocation Loc, OverloadCandidateSet::CandidateSetKind Kind,
+    bool PartialOverloading) {
   // Define viable functions to be better candidates than non-viable
   // functions.
   if (!Cand2.Viable)
@@ -10666,7 +10667,7 @@ bool clang::isBetterOverloadCandidate(
                         : QualType{},
             Obj2Context ? QualType(Obj2Context->getTypeForDecl(), 0)
                         : QualType{},
-            Cand1.isReversed() ^ Cand2.isReversed())) {
+            Cand1.isReversed() ^ Cand2.isReversed(), PartialOverloading)) {
       return BetterTemplate == Cand1.Function->getPrimaryTemplate();
     }
   }

clang/lib/Sema/SemaTemplateDeduction.cpp

@@ -5862,10 +5862,42 @@ static bool isAtLeastAsSpecializedAs(
   return true;
 }
 
+enum class MoreSpecializedTrailingPackTieBreakerResult { Equal, Less, More };
+
+// This a speculative fix for CWG1432 (Similar to the fix for CWG1395) that
+// there is no wording or even resolution for this issue.
+static MoreSpecializedTrailingPackTieBreakerResult
+getMoreSpecializedTrailingPackTieBreaker(
+    const TemplateSpecializationType *TST1,
+    const TemplateSpecializationType *TST2) {
+  ArrayRef<TemplateArgument> As1 = TST1->template_arguments(),
+                             As2 = TST2->template_arguments();
+  const TemplateArgument &TA1 = As1.back(), &TA2 = As2.back();
+  bool IsPack = TA1.getKind() == TemplateArgument::Pack;
+  assert(IsPack == (TA2.getKind() == TemplateArgument::Pack));
+  if (!IsPack)
+    return MoreSpecializedTrailingPackTieBreakerResult::Equal;
+  assert(As1.size() == As2.size());
+
+  unsigned PackSize1 = TA1.pack_size(), PackSize2 = TA2.pack_size();
+  bool IsPackExpansion1 =
+      PackSize1 && TA1.pack_elements().back().isPackExpansion();
+  bool IsPackExpansion2 =
+      PackSize2 && TA2.pack_elements().back().isPackExpansion();
+  if (PackSize1 == PackSize2 && IsPackExpansion1 == IsPackExpansion2)
+    return MoreSpecializedTrailingPackTieBreakerResult::Equal;
+  if (PackSize1 > PackSize2 && IsPackExpansion1)
+    return MoreSpecializedTrailingPackTieBreakerResult::More;
+  if (PackSize1 < PackSize2 && IsPackExpansion2)
+    return MoreSpecializedTrailingPackTieBreakerResult::Less;
+  return MoreSpecializedTrailingPackTieBreakerResult::Equal;
+}
+
 FunctionTemplateDecl *Sema::getMoreSpecializedTemplate(
     FunctionTemplateDecl *FT1, FunctionTemplateDecl *FT2, SourceLocation Loc,
     TemplatePartialOrderingContext TPOC, unsigned NumCallArguments1,
-    QualType RawObj1Ty, QualType RawObj2Ty, bool Reversed) {
+    QualType RawObj1Ty, QualType RawObj2Ty, bool Reversed,
+    bool PartialOverloading) {
   SmallVector<QualType> Args1;
   SmallVector<QualType> Args2;
   const FunctionDecl *FD1 = FT1->getTemplatedDecl();
@@ -6001,34 +6033,27 @@ FunctionTemplateDecl *Sema::getMoreSpecializedTemplate(
       return FT1;
   }
 
-  // This a speculative fix for CWG1432 (Similar to the fix for CWG1395) that
-  // there is no wording or even resolution for this issue.
-  for (int i = 0, e = std::min(NumParams1, NumParams2); i < e; ++i) {
+  // Skip this tie breaker if we are performing overload resolution with partial
+  // arguments, as this breaks some assumptions about how closely related the
+  // candidates are.
+  for (int i = 0, e = std::min(NumParams1, NumParams2);
+       !PartialOverloading && i < e; ++i) {
     QualType T1 = Param1[i].getCanonicalType();
     QualType T2 = Param2[i].getCanonicalType();
     auto *TST1 = dyn_cast<TemplateSpecializationType>(T1);
     auto *TST2 = dyn_cast<TemplateSpecializationType>(T2);
     if (!TST1 || !TST2)
       continue;
-    const TemplateArgument &TA1 = TST1->template_arguments().back();
-    if (TA1.getKind() == TemplateArgument::Pack) {
-      assert(TST1->template_arguments().size() ==
-             TST2->template_arguments().size());
-      const TemplateArgument &TA2 = TST2->template_arguments().back();
-      assert(TA2.getKind() == TemplateArgument::Pack);
-      unsigned PackSize1 = TA1.pack_size();
-      unsigned PackSize2 = TA2.pack_size();
-      bool IsPackExpansion1 =
-          PackSize1 && TA1.pack_elements().back().isPackExpansion();
-      bool IsPackExpansion2 =
-          PackSize2 && TA2.pack_elements().back().isPackExpansion();
-      if (PackSize1 != PackSize2 && IsPackExpansion1 != IsPackExpansion2) {
-        if (PackSize1 > PackSize2 && IsPackExpansion1)
-          return FT2;
-        if (PackSize1 < PackSize2 && IsPackExpansion2)
-          return FT1;
-      }
+    switch (getMoreSpecializedTrailingPackTieBreaker(TST1, TST2)) {
+    case MoreSpecializedTrailingPackTieBreakerResult::Less:
+      return FT1;
+    case MoreSpecializedTrailingPackTieBreakerResult::More:
+      return FT2;
+    case MoreSpecializedTrailingPackTieBreakerResult::Equal:
+      continue;
     }
+    llvm_unreachable(
+        "unknown MoreSpecializedTrailingPackTieBreakerResult value");
   }
 
   if (!Context.getLangOpts().CPlusPlus20)
@@ -6375,28 +6400,15 @@ getMoreSpecialized(Sema &S, QualType T1, QualType T2, TemplateLikeDecl *P1,
   if (!Better1 && !Better2)
     return nullptr;
 
-  // This a speculative fix for CWG1432 (Similar to the fix for CWG1395) that
-  // there is no wording or even resolution for this issue.
-  auto *TST1 = cast<TemplateSpecializationType>(T1);
-  auto *TST2 = cast<TemplateSpecializationType>(T2);
-  const TemplateArgument &TA1 = TST1->template_arguments().back();
-  if (TA1.getKind() == TemplateArgument::Pack) {
-    assert(TST1->template_arguments().size() ==
-           TST2->template_arguments().size());
-    const TemplateArgument &TA2 = TST2->template_arguments().back();
-    assert(TA2.getKind() == TemplateArgument::Pack);
-    unsigned PackSize1 = TA1.pack_size();
-    unsigned PackSize2 = TA2.pack_size();
-    bool IsPackExpansion1 =
-        PackSize1 && TA1.pack_elements().back().isPackExpansion();
-    bool IsPackExpansion2 =
-        PackSize2 && TA2.pack_elements().back().isPackExpansion();
-    if (PackSize1 != PackSize2 && IsPackExpansion1 != IsPackExpansion2) {
-      if (PackSize1 > PackSize2 && IsPackExpansion1)
-        return GetP2()(P1, P2);
-      if (PackSize1 < PackSize2 && IsPackExpansion2)
-        return P1;
-    }
+  switch (getMoreSpecializedTrailingPackTieBreaker(
+      cast<TemplateSpecializationType>(T1),
+      cast<TemplateSpecializationType>(T2))) {
+  case MoreSpecializedTrailingPackTieBreakerResult::Less:
+    return P1;
+  case MoreSpecializedTrailingPackTieBreakerResult::More:
+    return GetP2()(P1, P2);
+  case MoreSpecializedTrailingPackTieBreakerResult::Equal:
+    break;
   }
 
   if (!S.Context.getLangOpts().CPlusPlus20)

clang/test/CXX/drs/cwg14xx.cpp

@@ -59,22 +59,34 @@ namespace cwg1423 { // cwg1423: 11
 
 namespace cwg1432 { // cwg1432: 16
 #if __cplusplus >= 201103L
-  template<typename T> T declval();
+  namespace class_template_partial_spec {
+    template<typename T> T declval();
 
-  template <class... T>
-  struct common_type;
+    template <class... T>
+    struct common_type;
 
-  template <class T, class U>
-  struct common_type<T, U> {
-   typedef decltype(true ? declval<T>() : declval<U>()) type;
-  };
+    template <class T, class U>
+    struct common_type<T, U> {
+     typedef decltype(true ? declval<T>() : declval<U>()) type;
+    };
 
-  template <class T, class U, class... V>
-  struct common_type<T, U, V...> {
-   typedef typename common_type<typename common_type<T, U>::type, V...>::type type;
-  };
+    template <class T, class U, class... V>
+    struct common_type<T, U, V...> {
+     typedef typename common_type<typename common_type<T, U>::type, V...>::type type;
+    };
 
-  template struct common_type<int, double>;
+    template struct common_type<int, double>;
+  } // namespace class_template_partial_spec
+  namespace function_template {
+    template <int I, class... Ts> struct A {};
+
+    template <int I, class... Ts> void f(A<I, Ts...>) = delete;
+    template <int I> void f(A<I>);
+
+    void test() {
+      f(A<0>());
+    }
+  } // namespace function_template
 #endif
 } // namespace cwg1432
 

clang/test/CodeCompletion/GH125500.cpp

@@ -0,0 +1,38 @@
+// Note: the run lines follow their respective tests, since line/column
+// matter in this test.
+
+template <int...> struct B {};
+template <int> class C;
+
+namespace method {
+  struct S {
+    template <int Z>
+    void waldo(C<Z>);
+
+    template <int... Is, int Z>
+    void waldo(B<Is...>, C<Z>);
+  };
+
+  void foo() {
+    S().waldo(/*invoke completion here*/);
+    // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:%(line-1):15 %s -o - | FileCheck -check-prefix=CHECK-METHOD %s
+    // CHECK-METHOD-LABEL: OPENING_PAREN_LOC:
+    // CHECK-METHOD-NEXT: OVERLOAD: [#void#]waldo(<#C<Z>#>)
+    // CHECK-METHOD-NEXT: OVERLOAD: [#void#]waldo(<#B<>#>, C<Z>)
+  }
+} // namespace method
+namespace function {
+  template <int Z>
+  void waldo(C<Z>);
+
+  template <int... Is, int Z>
+  void waldo(B<Is...>, C<Z>);
+
+  void foo() {
+    waldo(/*invoke completion here*/);
+    // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:%(line-1):11 %s -o - | FileCheck -check-prefix=CHECK-FUNC %s
+    // CHECK-FUNC-LABEL: OPENING_PAREN_LOC:
+    // CHECK-FUNC-NEXT: OVERLOAD: [#void#]waldo(<#B<>#>, C<Z>)
+    // CHECK-FUNC-NEXT: OVERLOAD: [#void#]waldo(<#C<Z>#>)
+  }
+} // namespace function