[clang] cfc2c59 - Revert "Revert "Revert "[Concepts] Fix overload resolution bug with constrained candidates"""

Nick Kreeger via cfe-commits cfe-commits at lists.llvm.org
Sat Apr 23 17:37:07 PDT 2022


Author: Nick Kreeger
Date: 2022-04-23T19:36:51-05:00
New Revision: cfc2c5905ec11a501cdfc9502f503ab494c200b6

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

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

This reverts commit a0636b5855f5ba1f7033670dc32c956d63baaa51.

Added: 
    

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

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


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e78167bad589e..3f1c86cdc3aca 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -123,11 +123,6 @@ 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 114725498c982..9093bb39e9e86 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3580,8 +3580,7 @@ class Sema final {
                                 QualType& ConvertedType);
   bool FunctionParamTypesAreEqual(const FunctionProtoType *OldType,
                                   const FunctionProtoType *NewType,
-                                  unsigned *ArgPos = nullptr,
-                                  bool Reversed = false);
+                                  unsigned *ArgPos = nullptr);
   void HandleFunctionTypeMismatch(PartialDiagnostic &PDiag,
                                   QualType FromType, QualType ToType);
 
@@ -8750,8 +8749,7 @@ class Sema final {
   FunctionTemplateDecl *getMoreSpecializedTemplate(
       FunctionTemplateDecl *FT1, FunctionTemplateDecl *FT2, SourceLocation Loc,
       TemplatePartialOrderingContext TPOC, unsigned NumCallArguments1,
-      unsigned NumCallArguments2, bool Reversed = false,
-      bool AllowOrderingByConstraints = true);
+      unsigned NumCallArguments2, bool Reversed = false);
   UnresolvedSetIterator
   getMostSpecialized(UnresolvedSetIterator SBegin, UnresolvedSetIterator SEnd,
                      TemplateSpecCandidateSet &FailedCandidates,

diff  --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 6e45fdfbeb089..1add971a81a02 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -2945,30 +2945,24 @@ void Sema::HandleFunctionTypeMismatch(PartialDiagnostic &PDiag,
 }
 
 /// FunctionParamTypesAreEqual - This routine checks two function proto types
-/// for equality of their parameter types. Caller has already checked that
-/// they have same number of parameters.  If the parameters are 
diff erent,
+/// for equality of their argument types. Caller has already checked that
+/// they have same number of arguments.  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, 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;
-
+                                      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) {
     // Ignore address spaces in pointee type. This is to disallow overloading
     // on __ptr32/__ptr64 address spaces.
-    QualType Old = Context.removePtrSizeAddrSpace(OldType->getParamType(I).getUnqualifiedType());
-    QualType New = Context.removePtrSizeAddrSpace(NewType->getParamType(J).getUnqualifiedType());
+    QualType Old = Context.removePtrSizeAddrSpace(O->getUnqualifiedType());
+    QualType New = Context.removePtrSizeAddrSpace(N->getUnqualifiedType());
 
     if (!Context.hasSameType(Old, New)) {
       if (ArgPos)
-        *ArgPos = I;
+        *ArgPos = O - OldType->param_type_begin();
       return false;
     }
   }
@@ -9590,32 +9584,6 @@ 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(
@@ -9847,28 +9815,34 @@ bool clang::isBetterOverloadCandidate(
             isa<CXXConversionDecl>(Cand1.Function) ? TPOC_Conversion
                                                    : TPOC_Call,
             Cand1.ExplicitCallArguments, Cand2.ExplicitCallArguments,
-            Cand1.isReversed() ^ Cand2.isReversed(),
-            canCompareFunctionConstraints(S, Cand1, Cand2)))
+            Cand1.isReversed() ^ Cand2.isReversed()))
       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 (!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;
+  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;
+      }
     }
   }
 

diff  --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index 0662dd7602236..cb273aa285906 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -5143,20 +5143,18 @@ 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,
-    bool AllowOrderingByConstraints) {
-
-  auto JudgeByConstraints = [&]() -> FunctionTemplateDecl * {
-    if (!AllowOrderingByConstraints)
-      return nullptr;
+FunctionTemplateDecl *
+Sema::getMoreSpecializedTemplate(FunctionTemplateDecl *FT1,
+                                 FunctionTemplateDecl *FT2,
+                                 SourceLocation Loc,
+                                 TemplatePartialOrderingContext TPOC,
+                                 unsigned NumCallArguments1,
+                                 unsigned NumCallArguments2,
+                                 bool Reversed) {
+
+  auto JudgeByConstraints = [&] () -> FunctionTemplateDecl * {
     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
deleted file mode 100644
index d2c7ec91c3fc8..0000000000000
--- a/clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
+++ /dev/null
@@ -1,49 +0,0 @@
-// 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