This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG807e418413a0: [Concepts] Fix overload resolution bug with constrained candidates (authored by royjacobson).
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123182/new/ https://reviews.llvm.org/D123182 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaOverload.cpp clang/lib/Sema/SemaTemplateDeduction.cpp clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
Index: clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp =================================================================== --- /dev/null +++ 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}} +} + +} Index: clang/lib/Sema/SemaTemplateDeduction.cpp =================================================================== --- clang/lib/Sema/SemaTemplateDeduction.cpp +++ clang/lib/Sema/SemaTemplateDeduction.cpp @@ -5143,18 +5143,20 @@ /// 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); Index: clang/lib/Sema/SemaOverload.cpp =================================================================== --- clang/lib/Sema/SemaOverload.cpp +++ clang/lib/Sema/SemaOverload.cpp @@ -2945,24 +2945,30 @@ } /// 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 different, +/// for equality of their parameter types. Caller has already checked that +/// they have same number of parameters. If the parameters are different, /// ArgPos will have the parameter index of the first different 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 different 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 @@ 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 @@ 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; } } Index: clang/include/clang/Sema/Sema.h =================================================================== --- clang/include/clang/Sema/Sema.h +++ clang/include/clang/Sema/Sema.h @@ -3580,7 +3580,8 @@ 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); @@ -8749,7 +8750,8 @@ 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, Index: clang/docs/ReleaseNotes.rst =================================================================== --- clang/docs/ReleaseNotes.rst +++ clang/docs/ReleaseNotes.rst @@ -123,6 +123,11 @@ 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 different. 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 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits