[clang] 454d1df - [Concepts] Fix overload resolution bug with constrained candidates

Roy Jacobson via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 19 01:45:34 PDT 2022


Author: Roy Jacobson
Date: 2022-04-19T04:45:28-04:00
New Revision: 454d1df9423c95e54c3a2f5cb58d864096032d09

URL: https://github.com/llvm/llvm-project/commit/454d1df9423c95e54c3a2f5cb58d864096032d09
DIFF: https://github.com/llvm/llvm-project/commit/454d1df9423c95e54c3a2f5cb58d864096032d09.diff

LOG: [Concepts] Fix overload resolution bug with constrained candidates

When doing overload resolution, we have to check that candidates' parameter types are equal before trying to find a better candidate through checking which candidate is more constrained.
This revision adds this missing check and makes us diagnose those cases as ambiguous calls when the types are not equal.

Fixes GitHub issue https://github.com/llvm/llvm-project/issues/53640

Reviewed By: erichkeane

Differential Revision: https://reviews.llvm.org/D123182

Added: 
    clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/include/clang/Sema/Sema.h
    clang/lib/Sema/SemaOverload.cpp
    clang/lib/Sema/SemaTemplateDeduction.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c396322f2de57..a7a6fab5fe535 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -123,6 +123,11 @@ Bug Fixes
   a lambda expression that shares the name of a variable in a containing
   if/while/for/switch init statement as a redeclaration.
   This fixes `Issue 54913 <https://github.com/llvm/llvm-project/issues/54913>`_.
+- Overload resolution for constrained function templates could use the partial
+  order of constraints to select an overload, even if the parameter types of
+  the functions were 
diff erent. It now diagnoses this case correctly as an
+  ambiguous call and an error. Fixes
+  `Issue 53640 <https://github.com/llvm/llvm-project/issues/53640>`_.
 
 Improvements to Clang's diagnostics
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index e2ae02ea9f76f..fd9433cb1b786 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3590,7 +3590,8 @@ class Sema final {
                                 QualType& ConvertedType);
   bool FunctionParamTypesAreEqual(const FunctionProtoType *OldType,
                                   const FunctionProtoType *NewType,
-                                  unsigned *ArgPos = nullptr);
+                                  unsigned *ArgPos = nullptr,
+                                  bool Reversed = false);
   void HandleFunctionTypeMismatch(PartialDiagnostic &PDiag,
                                   QualType FromType, QualType ToType);
 
@@ -8768,7 +8769,8 @@ class Sema final {
   FunctionTemplateDecl *getMoreSpecializedTemplate(
       FunctionTemplateDecl *FT1, FunctionTemplateDecl *FT2, SourceLocation Loc,
       TemplatePartialOrderingContext TPOC, unsigned NumCallArguments1,
-      unsigned NumCallArguments2, bool Reversed = false);
+      unsigned NumCallArguments2, bool Reversed = false,
+      bool AllowOrderingByConstraints = true);
   UnresolvedSetIterator
   getMostSpecialized(UnresolvedSetIterator SBegin, UnresolvedSetIterator SEnd,
                      TemplateSpecCandidateSet &FailedCandidates,

diff  --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 1add971a81a02..6e45fdfbeb089 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -2945,24 +2945,30 @@ void Sema::HandleFunctionTypeMismatch(PartialDiagnostic &PDiag,
 }
 
 /// FunctionParamTypesAreEqual - This routine checks two function proto types
-/// for equality of their argument types. Caller has already checked that
-/// they have same number of arguments.  If the parameters are 
diff erent,
+/// for equality of their parameter types. Caller has already checked that
+/// they have same number of parameters.  If the parameters are 
diff erent,
 /// ArgPos will have the parameter index of the first 
diff erent parameter.
+/// If `Reversed` is true, the parameters of `NewType` will be compared in
+/// reverse order. That's useful if one of the functions is being used as a C++20
+/// synthesized operator overload with a reversed parameter order.
 bool Sema::FunctionParamTypesAreEqual(const FunctionProtoType *OldType,
                                       const FunctionProtoType *NewType,
-                                      unsigned *ArgPos) {
-  for (FunctionProtoType::param_type_iterator O = OldType->param_type_begin(),
-                                              N = NewType->param_type_begin(),
-                                              E = OldType->param_type_end();
-       O && (O != E); ++O, ++N) {
+                                      unsigned *ArgPos, bool Reversed) {
+  assert(OldType->getNumParams() == NewType->getNumParams() &&
+         "Can't compare parameters of functions with 
diff erent number of "
+         "parameters!");
+  for (size_t I = 0; I < OldType->getNumParams(); I++) {
+    // Reverse iterate over the parameters of `OldType` if `Reversed` is true.
+    size_t J = Reversed ? (OldType->getNumParams() - I - 1) : I;
+
     // Ignore address spaces in pointee type. This is to disallow overloading
     // on __ptr32/__ptr64 address spaces.
-    QualType Old = Context.removePtrSizeAddrSpace(O->getUnqualifiedType());
-    QualType New = Context.removePtrSizeAddrSpace(N->getUnqualifiedType());
+    QualType Old = Context.removePtrSizeAddrSpace(OldType->getParamType(I).getUnqualifiedType());
+    QualType New = Context.removePtrSizeAddrSpace(NewType->getParamType(J).getUnqualifiedType());
 
     if (!Context.hasSameType(Old, New)) {
       if (ArgPos)
-        *ArgPos = O - OldType->param_type_begin();
+        *ArgPos = I;
       return false;
     }
   }
@@ -9584,6 +9590,32 @@ static bool haveSameParameterTypes(ASTContext &Context, const FunctionDecl *F1,
   return true;
 }
 
+/// We're allowed to use constraints partial ordering only if the candidates
+/// have the same parameter types:
+/// [temp.func.order]p6.2.2 [...] or if the function parameters that
+/// positionally correspond between the two templates are not of the same type,
+/// neither template is more specialized than the other.
+/// [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 [...]
+static bool canCompareFunctionConstraints(Sema &S,
+                                          const OverloadCandidate &Cand1,
+                                          const OverloadCandidate &Cand2) {
+  // FIXME: Per P2113R0 we also need to compare the template parameter lists
+  // when comparing template functions. 
+  if (Cand1.Function && Cand2.Function && Cand1.Function->hasPrototype() &&
+      Cand2.Function->hasPrototype()) {
+    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;
+  }
+  return false;
+}
+
 /// isBetterOverloadCandidate - Determines whether the first overload
 /// candidate is a better candidate than the second (C++ 13.3.3p1).
 bool clang::isBetterOverloadCandidate(
@@ -9815,34 +9847,28 @@ bool clang::isBetterOverloadCandidate(
             isa<CXXConversionDecl>(Cand1.Function) ? TPOC_Conversion
                                                    : TPOC_Call,
             Cand1.ExplicitCallArguments, Cand2.ExplicitCallArguments,
-            Cand1.isReversed() ^ Cand2.isReversed()))
+            Cand1.isReversed() ^ Cand2.isReversed(),
+            canCompareFunctionConstraints(S, Cand1, Cand2)))
       return BetterTemplate == Cand1.Function->getPrimaryTemplate();
   }
 
   //   -— F1 and F2 are non-template functions with the same
   //      parameter-type-lists, and F1 is more constrained than F2 [...],
-  if (Cand1.Function && Cand2.Function && !Cand1IsSpecialization &&
-      !Cand2IsSpecialization && Cand1.Function->hasPrototype() &&
-      Cand2.Function->hasPrototype()) {
-    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)) {
-      Expr *RC1 = Cand1.Function->getTrailingRequiresClause();
-      Expr *RC2 = Cand2.Function->getTrailingRequiresClause();
-      if (RC1 && RC2) {
-        bool AtLeastAsConstrained1, AtLeastAsConstrained2;
-        if (S.IsAtLeastAsConstrained(Cand1.Function, {RC1}, Cand2.Function,
-                                     {RC2}, AtLeastAsConstrained1) ||
-            S.IsAtLeastAsConstrained(Cand2.Function, {RC2}, Cand1.Function,
-                                     {RC1}, AtLeastAsConstrained2))
-          return false;
-        if (AtLeastAsConstrained1 != AtLeastAsConstrained2)
-          return AtLeastAsConstrained1;
-      } else if (RC1 || RC2) {
-        return RC1 != nullptr;
-      }
+  if (!Cand1IsSpecialization && !Cand2IsSpecialization &&
+      canCompareFunctionConstraints(S, Cand1, Cand2)) {
+    Expr *RC1 = Cand1.Function->getTrailingRequiresClause();
+    Expr *RC2 = Cand2.Function->getTrailingRequiresClause();
+    if (RC1 && RC2) {
+      bool AtLeastAsConstrained1, AtLeastAsConstrained2;
+      if (S.IsAtLeastAsConstrained(Cand1.Function, {RC1}, Cand2.Function, {RC2},
+                                   AtLeastAsConstrained1) ||
+          S.IsAtLeastAsConstrained(Cand2.Function, {RC2}, Cand1.Function, {RC1},
+                                   AtLeastAsConstrained2))
+        return false;
+      if (AtLeastAsConstrained1 != AtLeastAsConstrained2)
+        return AtLeastAsConstrained1;
+    } else if (RC1 || RC2) {
+      return RC1 != nullptr;
     }
   }
 

diff  --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index cb273aa285906..0662dd7602236 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -5143,18 +5143,20 @@ static bool isVariadicFunctionTemplate(FunctionTemplateDecl *FunTmpl) {
 /// candidate with a reversed parameter order. In this case, the corresponding
 /// P/A pairs between FT1 and FT2 are reversed.
 ///
+/// \param AllowOrderingByConstraints If \c is false, don't check whether one
+/// of the templates is more constrained than the other. Default is true.
+///
 /// \returns the more specialized function template. If neither
 /// template is more specialized, returns NULL.
-FunctionTemplateDecl *
-Sema::getMoreSpecializedTemplate(FunctionTemplateDecl *FT1,
-                                 FunctionTemplateDecl *FT2,
-                                 SourceLocation Loc,
-                                 TemplatePartialOrderingContext TPOC,
-                                 unsigned NumCallArguments1,
-                                 unsigned NumCallArguments2,
-                                 bool Reversed) {
-
-  auto JudgeByConstraints = [&] () -> FunctionTemplateDecl * {
+FunctionTemplateDecl *Sema::getMoreSpecializedTemplate(
+    FunctionTemplateDecl *FT1, FunctionTemplateDecl *FT2, SourceLocation Loc,
+    TemplatePartialOrderingContext TPOC, unsigned NumCallArguments1,
+    unsigned NumCallArguments2, bool Reversed,
+    bool AllowOrderingByConstraints) {
+
+  auto JudgeByConstraints = [&]() -> FunctionTemplateDecl * {
+    if (!AllowOrderingByConstraints)
+      return nullptr;
     llvm::SmallVector<const Expr *, 3> AC1, AC2;
     FT1->getAssociatedConstraints(AC1);
     FT2->getAssociatedConstraints(AC2);

diff  --git a/clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp b/clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
new file mode 100644
index 0000000000000..d2c7ec91c3fc8
--- /dev/null
+++ b/clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
+
+struct A;
+struct B;
+
+template <typename> constexpr bool True = true;
+template <typename T> concept C = True<T>;
+
+void f(C auto &, auto &) = delete;
+template <C Q> void f(Q &, C auto &);
+
+void g(struct A *ap, struct B *bp) {
+  f(*ap, *bp);
+}
+
+template <typename T, typename U> struct X {};
+
+template <typename T, C U, typename V> bool operator==(X<T, U>, V) = delete;
+template <C T, C U, C V>               bool operator==(T, X<U, V>);
+
+bool h() {
+  return X<void *, int>{} == 0;
+}
+
+namespace PR53640 {
+
+template <typename T>
+concept C = true;
+
+template <C T>
+void f(T t) {} // expected-note {{candidate function [with T = int]}}
+
+template <typename T>
+void f(const T &t) {} // expected-note {{candidate function [with T = int]}}
+
+int g() {
+  f(0); // expected-error {{call to 'f' is ambiguous}}
+}
+
+struct S {
+  template <typename T> explicit S(T) noexcept requires C<T> {} // expected-note {{candidate constructor}}
+  template <typename T> explicit S(const T &) noexcept {}       // expected-note {{candidate constructor}}
+};
+
+int h() {
+  S s(4); // expected-error-re {{call to constructor of {{.*}} is ambiguous}}
+}
+
+}


        


More information about the cfe-commits mailing list