Skip to content

Commit

Permalink
[C++20] Implement P2113R0: Changes to the Partial Ordering of Constra…
Browse files Browse the repository at this point in the history
…ined Functions

This implementation matches GCC behavior in that [[ https://eel.is/c++draft/temp.func.order#6.2.1 | temp.func.order p6.2.1 ]] is not implemented [1]. I reached out to the GCC author to confirm that some changes elsewhere to overload resolution are probably needed, but no solution has been developed sufficiently [3].

Most of the wordings are implemented straightforwardly. However,
for [[ https://eel.is/c++draft/temp.func.order#6.2.2 | temp.func.order p6.2.2 ]] "... or if the function parameters that positionally correspond between the two templates are not of the same type", the "same type" is not very clear ([2] is a bug related to this). Here is a quick example
```
template <C T, C U>        int f(T, U);
template <typename T, C U> int f(U, T);

int x = f(0, 0);
```
Is the `U` and `T` from different `f`s the "same type"? The answer is NO even though both `U` and `T` are deduced to be `int` in this case. The reason is that `U` and `T` are dependent types, according to [[ https://eel.is/c++draft/temp.over.link#3 |  temp.over.link p3 ]], they can not be the "same type".

To check if two function parameters are the "same type":
* For //function template//: compare the function parameter canonical types and return type between two function templates.
* For //class template/partial specialization//: by [[ https://eel.is/c++draft/temp.spec.partial.order#1.2 | temp.spec.partial.order p1.2 ]], compare the injected template arguments between two templates using hashing(TemplateArgument::Profile) is enough.

[1] https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=57b4daf8dc4ed7b669cc70638866ddb00f5b7746
[2] #49308
[3] https://lists.isocpp.org/core/2020/06/index.php#msg9392

Fixes #54039
Fixes #49308 (PR49964)

Reviewed By: royjacobson, #clang-language-wg, mizvekov

Differential Revision: https://reviews.llvm.org/D128750
  • Loading branch information
Yuanfang Chen committed Oct 18, 2022
1 parent 9f8340e commit 340eac0
Show file tree
Hide file tree
Showing 11 changed files with 385 additions and 144 deletions.
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Expand Up @@ -534,6 +534,9 @@ C++20 Feature Support
which removes the requirement for the ``typename`` keyword in certain contexts.
- Implemented The Equality Operator You Are Looking For (`P2468 <http://wg21.link/p2468r2>`_).

- Implemented `P2113R0: Proposed resolution for 2019 comment CA 112 <https://wg21.link/P2113R0>`_
([temp.func.order]p6.2.1 is not implemented, matching GCC).

C++2b Feature Support
^^^^^^^^^^^^^^^^^^^^^

Expand Down
35 changes: 17 additions & 18 deletions clang/include/clang/AST/DeclTemplate.h
Expand Up @@ -847,6 +847,15 @@ class RedeclarableTemplateDecl : public TemplateDecl,
/// The first value in the array is the number of specializations/partial
/// specializations that follow.
uint32_t *LazySpecializations = nullptr;

/// The set of "injected" template arguments used within this
/// template.
///
/// This pointer refers to the template arguments (there are as
/// many template arguments as template parameaters) for the
/// template, and is allocated lazily, since most templates do not
/// require the use of this information.
TemplateArgument *InjectedArgs = nullptr;
};

/// Pointer to the common data shared by all declarations of this
Expand Down Expand Up @@ -954,6 +963,14 @@ class RedeclarableTemplateDecl : public TemplateDecl,
getCommonPtr()->InstantiatedFromMember.setPointer(TD);
}

/// Retrieve the "injected" template arguments that correspond to the
/// template parameters of this template.
///
/// Although the C++ standard has no notion of the "injected" template
/// arguments for a template, the notion is convenient when
/// we need to perform substitutions inside the definition of a template.
ArrayRef<TemplateArgument> getInjectedTemplateArgs();

using redecl_range = redeclarable_base::redecl_range;
using redecl_iterator = redeclarable_base::redecl_iterator;

Expand Down Expand Up @@ -998,15 +1015,6 @@ class FunctionTemplateDecl : public RedeclarableTemplateDecl {
/// template, including explicit specializations and instantiations.
llvm::FoldingSetVector<FunctionTemplateSpecializationInfo> Specializations;

/// The set of "injected" template arguments used within this
/// function template.
///
/// This pointer refers to the template arguments (there are as
/// many template arguments as template parameaters) for the function
/// template, and is allocated lazily, since most function templates do not
/// require the use of this information.
TemplateArgument *InjectedArgs = nullptr;

Common() = default;
};

Expand Down Expand Up @@ -1106,15 +1114,6 @@ class FunctionTemplateDecl : public RedeclarableTemplateDecl {
return makeSpecIterator(getSpecializations(), true);
}

/// Retrieve the "injected" template arguments that correspond to the
/// template parameters of this function template.
///
/// Although the C++ standard has no notion of the "injected" template
/// arguments for a function template, the notion is convenient when
/// we need to perform substitutions inside the definition of a function
/// template.
ArrayRef<TemplateArgument> getInjectedTemplateArgs();

/// Return whether this function template is an abbreviated function template,
/// e.g. `void foo(auto x)` or `template<typename T> void foo(auto x)`
bool isAbbreviated() const {
Expand Down
12 changes: 7 additions & 5 deletions clang/include/clang/Sema/Sema.h
Expand Up @@ -8329,14 +8329,17 @@ class Sema final {
const NamedDecl *NewInstFrom, TemplateParameterList *New,
const NamedDecl *OldInstFrom, TemplateParameterList *Old, bool Complain,
TemplateParameterListEqualKind Kind,
SourceLocation TemplateArgLoc = SourceLocation());
SourceLocation TemplateArgLoc = SourceLocation(),
bool PartialOrdering = false);

bool TemplateParameterListsAreEqual(
TemplateParameterList *New, TemplateParameterList *Old, bool Complain,
TemplateParameterListEqualKind Kind,
SourceLocation TemplateArgLoc = SourceLocation()) {
SourceLocation TemplateArgLoc = SourceLocation(),
bool PartialOrdering = false) {
return TemplateParameterListsAreEqual(nullptr, New, nullptr, Old, Complain,
Kind, TemplateArgLoc);
Kind, TemplateArgLoc,
PartialOrdering);
}

bool CheckTemplateDeclScope(Scope *S, TemplateParameterList *TemplateParams);
Expand Down Expand Up @@ -9021,8 +9024,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,
Expand Down
32 changes: 16 additions & 16 deletions clang/lib/AST/DeclTemplate.cpp
Expand Up @@ -353,6 +353,22 @@ void RedeclarableTemplateDecl::addSpecializationImpl(
SETraits::getDecl(Entry));
}

ArrayRef<TemplateArgument> RedeclarableTemplateDecl::getInjectedTemplateArgs() {
TemplateParameterList *Params = getTemplateParameters();
auto *CommonPtr = getCommonPtr();
if (!CommonPtr->InjectedArgs) {
auto &Context = getASTContext();
SmallVector<TemplateArgument, 16> TemplateArgs;
Context.getInjectedTemplateArgs(Params, TemplateArgs);
CommonPtr->InjectedArgs =
new (Context) TemplateArgument[TemplateArgs.size()];
std::copy(TemplateArgs.begin(), TemplateArgs.end(),
CommonPtr->InjectedArgs);
}

return llvm::makeArrayRef(CommonPtr->InjectedArgs, Params->size());
}

//===----------------------------------------------------------------------===//
// FunctionTemplateDecl Implementation
//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -403,22 +419,6 @@ void FunctionTemplateDecl::addSpecialization(
InsertPos);
}

ArrayRef<TemplateArgument> FunctionTemplateDecl::getInjectedTemplateArgs() {
TemplateParameterList *Params = getTemplateParameters();
Common *CommonPtr = getCommonPtr();
if (!CommonPtr->InjectedArgs) {
auto &Context = getASTContext();
SmallVector<TemplateArgument, 16> TemplateArgs;
Context.getInjectedTemplateArgs(Params, TemplateArgs);
CommonPtr->InjectedArgs =
new (Context) TemplateArgument[TemplateArgs.size()];
std::copy(TemplateArgs.begin(), TemplateArgs.end(),
CommonPtr->InjectedArgs);
}

return llvm::makeArrayRef(CommonPtr->InjectedArgs, Params->size());
}

void FunctionTemplateDecl::mergePrevDecl(FunctionTemplateDecl *Prev) {
using Base = RedeclarableTemplateDecl;

Expand Down
12 changes: 12 additions & 0 deletions clang/lib/Sema/SemaConcept.cpp
Expand Up @@ -1104,6 +1104,18 @@ NormalizedConstraint::fromConstraintExpr(Sema &S, NamedDecl *D, const Expr *E) {
// - The normal form of an expression (E) is the normal form of E.
// [...]
E = E->IgnoreParenImpCasts();

// C++2a [temp.param]p4:
// [...] If T is not a pack, then E is E', otherwise E is (E' && ...).
//
// Using the pattern suffices because the partial ordering rules guarantee
// the template paramaters are equivalent.
if (auto *FoldE = dyn_cast<const CXXFoldExpr>(E)) {
assert(FoldE->isRightFold() && FoldE->getOperator() == BO_LAnd);
assert(E->IgnoreParenImpCasts() == E);
E = FoldE->getPattern();
}

if (LogicalBinOp BO = E) {
auto LHS = fromConstraintExpr(S, D, BO.getLHS());
if (!LHS)
Expand Down
15 changes: 4 additions & 11 deletions clang/lib/Sema/SemaOverload.cpp
Expand Up @@ -9776,19 +9776,13 @@ static bool haveSameParameterTypes(ASTContext &Context, const FunctionDecl *F1,

/// 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,
static bool sameFunctionParameterTypeLists(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()) {
if (Cand1.Function && Cand2.Function) {
auto *PT1 = cast<FunctionProtoType>(Cand1.Function->getFunctionType());
auto *PT2 = cast<FunctionProtoType>(Cand2.Function->getFunctionType());
if (PT1->getNumParams() == PT2->getNumParams() &&
Expand Down Expand Up @@ -10031,15 +10025,14 @@ 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)) {
sameFunctionParameterTypeLists(S, Cand1, Cand2)) {
Expr *RC1 = Cand1.Function->getTrailingRequiresClause();
Expr *RC2 = Cand2.Function->getTrailingRequiresClause();
if (RC1 && RC2) {
Expand Down
22 changes: 12 additions & 10 deletions clang/lib/Sema/SemaTemplate.cpp
Expand Up @@ -7652,10 +7652,10 @@ bool Sema::CheckTemplateTemplateArgument(TemplateTemplateParmDecl *Param,

if (isTemplateTemplateParameterAtLeastAsSpecializedAs(Params, Template,
Arg.getLocation())) {
// C++2a[temp.func.order]p2
// P2113
// C++20[temp.func.order]p2
// [...] If both deductions succeed, the partial ordering selects the
// more constrained template as described by the rules in
// [temp.constr.order].
// more constrained template (if one exists) as determined below.
SmallVector<const Expr *, 3> ParamsAC, TemplateAC;
Params->getAssociatedConstraints(ParamsAC);
// C++2a[temp.arg.template]p3
Expand Down Expand Up @@ -7864,7 +7864,8 @@ Sema::BuildExpressionFromIntegralTemplateArgument(const TemplateArgument &Arg,
static bool MatchTemplateParameterKind(
Sema &S, NamedDecl *New, const NamedDecl *NewInstFrom, NamedDecl *Old,
const NamedDecl *OldInstFrom, bool Complain,
Sema::TemplateParameterListEqualKind Kind, SourceLocation TemplateArgLoc) {
Sema::TemplateParameterListEqualKind Kind, SourceLocation TemplateArgLoc,
bool PartialOrdering) {
// Check the actual kind (type, non-type, template).
if (Old->getKind() != New->getKind()) {
if (Complain) {
Expand Down Expand Up @@ -7952,11 +7953,11 @@ static bool MatchTemplateParameterKind(
(Kind == Sema::TPL_TemplateMatch
? Sema::TPL_TemplateTemplateParmMatch
: Kind),
TemplateArgLoc))
TemplateArgLoc, PartialOrdering))
return false;
}

if (Kind != Sema::TPL_TemplateTemplateArgumentMatch &&
if (!PartialOrdering && Kind != Sema::TPL_TemplateTemplateArgumentMatch &&
!isa<TemplateTemplateParmDecl>(Old)) {
const Expr *NewC = nullptr, *OldC = nullptr;

Expand Down Expand Up @@ -8049,7 +8050,8 @@ void DiagnoseTemplateParameterListArityMismatch(Sema &S,
bool Sema::TemplateParameterListsAreEqual(
const NamedDecl *NewInstFrom, TemplateParameterList *New,
const NamedDecl *OldInstFrom, TemplateParameterList *Old, bool Complain,
TemplateParameterListEqualKind Kind, SourceLocation TemplateArgLoc) {
TemplateParameterListEqualKind Kind, SourceLocation TemplateArgLoc,
bool PartialOrdering) {
if (Old->size() != New->size() && Kind != TPL_TemplateTemplateArgumentMatch) {
if (Complain)
DiagnoseTemplateParameterListArityMismatch(*this, New, Old, Kind,
Expand Down Expand Up @@ -8081,7 +8083,7 @@ bool Sema::TemplateParameterListsAreEqual(

if (!MatchTemplateParameterKind(*this, *NewParm, NewInstFrom, *OldParm,
OldInstFrom, Complain, Kind,
TemplateArgLoc))
TemplateArgLoc, PartialOrdering))
return false;

++NewParm;
Expand All @@ -8098,7 +8100,7 @@ bool Sema::TemplateParameterListsAreEqual(
for (; NewParm != NewParmEnd; ++NewParm) {
if (!MatchTemplateParameterKind(*this, *NewParm, NewInstFrom, *OldParm,
OldInstFrom, Complain, Kind,
TemplateArgLoc))
TemplateArgLoc, PartialOrdering))
return false;
}
}
Expand All @@ -8112,7 +8114,7 @@ bool Sema::TemplateParameterListsAreEqual(
return false;
}

if (Kind != TPL_TemplateTemplateArgumentMatch) {
if (!PartialOrdering && Kind != TPL_TemplateTemplateArgumentMatch) {
const Expr *NewRC = New->getRequiresClause();
const Expr *OldRC = Old->getRequiresClause();

Expand Down

0 comments on commit 340eac0

Please sign in to comment.