Skip to content

Commit

Permalink
[clang] Finish implementation of P0522
Browse files Browse the repository at this point in the history
This finishes the clang implementation of P0522, getting rid
of the fallback to the old, pre-P0522 rules.

Before this patch, when partial ordering template template parameters,
we would perform, in order:
* If the old rules would match, we would accept it. Otherwise, don't
  generate diagnostics yet.
* If the new rules would match, just accept it. Otherwise, don't
  generate any diagnostics yet again.
* Apply the old rules again, this time with diagnostics.

This situation was far from ideal, as we would sometimes:
* Accept some things we shouldn't.
* Reject some things we shouldn't.
* Only diagnose rejection in terms of the old rules.

With this patch, we apply the P0522 rules throughout.

This needed to extend template argument deduction in order
to accept the historial rule for TTP matching pack parameter to non-pack
arguments.
This change also makes us accept some combinations of historical and P0522
allowances we wouldn't before.

It also fixes a bunch of bugs that were documented in the test suite,
which I am not sure there are issues already created for them.

This causes a lot of changes to the way these failures are diagnosed,
with related test suite churn.

The problem here is that the old rules were very simple and
non-recursive, making it easy to provide customized diagnostics,
and to keep them consistent with each other.

The new rules are a lot more complex and rely on template argument
deduction, substitutions, and they are recursive.

The approach taken here is to mostly rely on existing diagnostics,
and create a new instantiation context that keeps track of this context.

So for example when a substitution failure occurs, we use the error
produced there unmodified, and just attach notes to it explaining
that it occurred in the context of partial ordering this template
argument against that template parameter.

This diverges from the old diagnostics, which would lead with an
error pointing to the template argument, explain the problem
in subsequent notes, and produce a final note pointing to the parameter.
  • Loading branch information
mizvekov committed Sep 5, 2024
1 parent 530f884 commit 958703d
Show file tree
Hide file tree
Showing 17 changed files with 652 additions and 266 deletions.
10 changes: 10 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,10 @@ C++23 Feature Support
C++20 Feature Support
^^^^^^^^^^^^^^^^^^^^^

C++17 Feature Support
^^^^^^^^^^^^^^^^^^^^^
- The implementation of the relaxed template template argument matching rules is
more complete and reliable, and should provide more accurate diagnostics.

Resolutions to C++ Defect Reports
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down Expand Up @@ -260,6 +264,10 @@ Improvements to Clang's diagnostics

- Clang now diagnoses when the result of a [[nodiscard]] function is discarded after being cast in C. Fixes #GH104391.

- Clang now properly explains the reason a template template argument failed to
match a template template parameter, in terms of the C++17 relaxed matching rules
instead of the old ones.

- Don't emit duplicated dangling diagnostics. (#GH93386).

- Improved diagnostic when trying to befriend a concept. (#GH45182).
Expand Down Expand Up @@ -341,6 +349,8 @@ Bug Fixes to C++ Support
- Correctly check constraints of explicit instantiations of member functions. (#GH46029)
- When performing partial ordering of function templates, clang now checks that
the deduction was consistent. Fixes (#GH18291).
- Fixes to several issues in partial ordering of template template parameters, which
were documented in the test suite.
- Fixed an assertion failure about a constraint of a friend function template references to a value with greater
template depth than the friend function template. (#GH98258)
- Clang now rebuilds the template parameters of out-of-line declarations and specializations in the context
Expand Down
7 changes: 7 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -5256,6 +5256,13 @@ def note_template_arg_refers_here_func : Note<
def err_template_arg_template_params_mismatch : Error<
"template template argument has different template parameters than its "
"corresponding template template parameter">;
def note_template_arg_template_params_mismatch : Note<
"template template argument has different template parameters than its "
"corresponding template template parameter">;
def err_non_deduced_mismatch : Error<
"could not match %diff{$ against $|types}0,1">;
def err_inconsistent_deduction : Error<
"conflicting deduction %diff{$ against $|types}0,1 for parameter">;
def err_template_arg_not_integral_or_enumeral : Error<
"non-type template argument of type %0 must have an integral or enumeration"
" type">;
Expand Down
14 changes: 12 additions & 2 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -12483,8 +12483,9 @@ class Sema final : public SemaBase {
sema::TemplateDeductionInfo &Info);

bool isTemplateTemplateParameterAtLeastAsSpecializedAs(
TemplateParameterList *PParam, TemplateDecl *AArg,
const DefaultArguments &DefaultArgs, SourceLocation Loc, bool IsDeduced);
TemplateParameterList *PParam, TemplateDecl *PArg, TemplateDecl *AArg,
const DefaultArguments &DefaultArgs, SourceLocation ArgLoc,
bool IsDeduced);

/// Mark which template parameters are used in a given expression.
///
Expand Down Expand Up @@ -12793,6 +12794,9 @@ class Sema final : public SemaBase {

/// We are instantiating a type alias template declaration.
TypeAliasTemplateInstantiation,

/// We are performing partial ordering for template template parameters.
PartialOrderingTTP,
} Kind;

/// Was the enclosing context a non-instantiation SFINAE context?
Expand Down Expand Up @@ -13014,6 +13018,12 @@ class Sema final : public SemaBase {
TemplateDecl *Entity, BuildingDeductionGuidesTag,
SourceRange InstantiationRange = SourceRange());

struct PartialOrderingTTP {};
/// \brief Note that we are partial ordering template template parameters.
InstantiatingTemplate(Sema &SemaRef, SourceLocation ArgLoc,
PartialOrderingTTP, TemplateDecl *PArg,
SourceRange InstantiationRange = SourceRange());

/// Note that we have finished instantiating this template.
void Clear();

Expand Down
2 changes: 2 additions & 0 deletions clang/lib/Frontend/FrontendActions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,8 @@ class DefaultTemplateInstCallback : public TemplateInstantiationCallback {
return "BuildingDeductionGuides";
case CodeSynthesisContext::TypeAliasTemplateInstantiation:
return "TypeAliasTemplateInstantiation";
case CodeSynthesisContext::PartialOrderingTTP:
return "PartialOrderingTTP";
}
return "";
}
Expand Down
94 changes: 38 additions & 56 deletions clang/lib/Sema/SemaTemplate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5341,8 +5341,7 @@ bool Sema::CheckTemplateArgumentList(
DefaultArgs && ParamIdx >= DefaultArgs.StartPos) {
// All written arguments should have been consumed by this point.
assert(ArgIdx == NumArgs && "bad default argument deduction");
// FIXME: Don't ignore parameter packs.
if (ParamIdx == DefaultArgs.StartPos && !(*Param)->isParameterPack()) {
if (ParamIdx == DefaultArgs.StartPos) {
assert(Param + DefaultArgs.Args.size() <= ParamEnd);
// Default arguments from a DeducedTemplateName are already converted.
for (const TemplateArgument &DefArg : DefaultArgs.Args) {
Expand Down Expand Up @@ -5586,8 +5585,9 @@ bool Sema::CheckTemplateArgumentList(
// pack expansions; they might be empty. This can happen even if
// PartialTemplateArgs is false (the list of arguments is complete but
// still dependent).
if (ArgIdx < NumArgs && CurrentInstantiationScope &&
CurrentInstantiationScope->getPartiallySubstitutedPack()) {
if (PartialOrderingTTP ||
(CurrentInstantiationScope &&
CurrentInstantiationScope->getPartiallySubstitutedPack())) {
while (ArgIdx < NumArgs &&
NewArgs[ArgIdx].getArgument().isPackExpansion()) {
const TemplateArgument &Arg = NewArgs[ArgIdx++].getArgument();
Expand Down Expand Up @@ -7185,64 +7185,46 @@ bool Sema::CheckTemplateTemplateArgument(TemplateTemplateParmDecl *Param,
<< Template;
}

if (!getLangOpts().RelaxedTemplateTemplateArgs)
return !TemplateParameterListsAreEqual(
Template->getTemplateParameters(), Params, /*Complain=*/true,
TPL_TemplateTemplateArgumentMatch, Arg.getLocation());

// C++1z [temp.arg.template]p3: (DR 150)
// A template-argument matches a template template-parameter P when P
// is at least as specialized as the template-argument A.
if (getLangOpts().RelaxedTemplateTemplateArgs) {
// Quick check for the common case:
// If P contains a parameter pack, then A [...] matches P if each of A's
// template parameters matches the corresponding template parameter in
// the template-parameter-list of P.
if (TemplateParameterListsAreEqual(
Template->getTemplateParameters(), Params, false,
TPL_TemplateTemplateArgumentMatch, Arg.getLocation()) &&
// If the argument has no associated constraints, then the parameter is
// definitely at least as specialized as the argument.
// Otherwise - we need a more thorough check.
!Template->hasAssociatedConstraints())
return false;

if (isTemplateTemplateParameterAtLeastAsSpecializedAs(
Params, Template, DefaultArgs, Arg.getLocation(), IsDeduced)) {
// P2113
// C++20[temp.func.order]p2
// [...] If both deductions succeed, the partial ordering selects the
// more constrained template (if one exists) as determined below.
SmallVector<const Expr *, 3> ParamsAC, TemplateAC;
Params->getAssociatedConstraints(ParamsAC);
// C++2a[temp.arg.template]p3
// [...] In this comparison, if P is unconstrained, the constraints on A
// are not considered.
if (ParamsAC.empty())
return false;
if (!isTemplateTemplateParameterAtLeastAsSpecializedAs(
Params, Param, Template, DefaultArgs, Arg.getLocation(), IsDeduced))
return true;
// P2113
// C++20[temp.func.order]p2
// [...] If both deductions succeed, the partial ordering selects the
// more constrained template (if one exists) as determined below.
SmallVector<const Expr *, 3> ParamsAC, TemplateAC;
Params->getAssociatedConstraints(ParamsAC);
// C++20[temp.arg.template]p3
// [...] In this comparison, if P is unconstrained, the constraints on A
// are not considered.
if (ParamsAC.empty())
return false;

Template->getAssociatedConstraints(TemplateAC);
Template->getAssociatedConstraints(TemplateAC);

bool IsParamAtLeastAsConstrained;
if (IsAtLeastAsConstrained(Param, ParamsAC, Template, TemplateAC,
IsParamAtLeastAsConstrained))
return true;
if (!IsParamAtLeastAsConstrained) {
Diag(Arg.getLocation(),
diag::err_template_template_parameter_not_at_least_as_constrained)
<< Template << Param << Arg.getSourceRange();
Diag(Param->getLocation(), diag::note_entity_declared_at) << Param;
Diag(Template->getLocation(), diag::note_entity_declared_at)
<< Template;
MaybeEmitAmbiguousAtomicConstraintsDiagnostic(Param, ParamsAC, Template,
TemplateAC);
return true;
}
return false;
}
// FIXME: Produce better diagnostics for deduction failures.
bool IsParamAtLeastAsConstrained;
if (IsAtLeastAsConstrained(Param, ParamsAC, Template, TemplateAC,
IsParamAtLeastAsConstrained))
return true;
if (!IsParamAtLeastAsConstrained) {
Diag(Arg.getLocation(),
diag::err_template_template_parameter_not_at_least_as_constrained)
<< Template << Param << Arg.getSourceRange();
Diag(Param->getLocation(), diag::note_entity_declared_at) << Param;
Diag(Template->getLocation(), diag::note_entity_declared_at) << Template;
MaybeEmitAmbiguousAtomicConstraintsDiagnostic(Param, ParamsAC, Template,
TemplateAC);
return true;
}

return !TemplateParameterListsAreEqual(Template->getTemplateParameters(),
Params,
true,
TPL_TemplateTemplateArgumentMatch,
Arg.getLocation());
return false;
}

static Sema::SemaDiagnosticBuilder noteLocation(Sema &S, const NamedDecl &Decl,
Expand Down
Loading

0 comments on commit 958703d

Please sign in to comment.