Skip to content

Commit

Permalink
Revert "[clang] Finish implementation of P0522 (#96023)"
Browse files Browse the repository at this point in the history
This caused Clang to reject valid code, see discussion on the PR
#96023 (comment)
and #111363

This reverts commit 6afe567 and
follow-up commit 9abb97f.
  • Loading branch information
zmodem committed Oct 9, 2024
1 parent 3be6916 commit ada6372
Show file tree
Hide file tree
Showing 19 changed files with 277 additions and 681 deletions.
10 changes: 0 additions & 10 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,6 @@ 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 @@ -335,10 +331,6 @@ 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 @@ -444,8 +436,6 @@ 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: 0 additions & 7 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -5262,13 +5262,6 @@ 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: 2 additions & 12 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -12417,9 +12417,8 @@ class Sema final : public SemaBase {
sema::TemplateDeductionInfo &Info);

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

/// Mark which template parameters are used in a given expression.
///
Expand Down Expand Up @@ -12728,9 +12727,6 @@ 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 @@ -12952,12 +12948,6 @@ 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: 0 additions & 2 deletions clang/lib/Frontend/FrontendActions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -457,8 +457,6 @@ class DefaultTemplateInstCallback : public TemplateInstantiationCallback {
return "BuildingDeductionGuides";
case CodeSynthesisContext::TypeAliasTemplateInstantiation:
return "TypeAliasTemplateInstantiation";
case CodeSynthesisContext::PartialOrderingTTP:
return "PartialOrderingTTP";
}
return "";
}
Expand Down
115 changes: 65 additions & 50 deletions clang/lib/Sema/SemaTemplate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5498,7 +5498,8 @@ bool Sema::CheckTemplateArgumentList(
DefaultArgs && ParamIdx >= DefaultArgs.StartPos) {
// All written arguments should have been consumed by this point.
assert(ArgIdx == NumArgs && "bad default argument deduction");
if (ParamIdx == DefaultArgs.StartPos) {
// FIXME: Don't ignore parameter packs.
if (ParamIdx == DefaultArgs.StartPos && !(*Param)->isParameterPack()) {
assert(Param + DefaultArgs.Args.size() <= ParamEnd);
// Default arguments from a DeducedTemplateName are already converted.
for (const TemplateArgument &DefArg : DefaultArgs.Args) {
Expand Down Expand Up @@ -5575,6 +5576,9 @@ bool Sema::CheckTemplateArgumentList(
return true;
}

// We're now done with this argument.
++ArgIdx;

if ((*Param)->isTemplateParameterPack()) {
// The template parameter was a template parameter pack, so take the
// deduced argument and place it on the argument pack. Note that we
Expand All @@ -5585,19 +5589,8 @@ bool Sema::CheckTemplateArgumentList(
} else {
// Move to the next template parameter.
++Param;
if (PartialOrderingTTP && PackExpansionIntoNonPack) {
// Keep converting the pattern in the argument against
// subsequent parameters. The argument is converted
// in place and will be added back when we are done.
SugaredConverted.pop_back();
CanonicalConverted.pop_back();
continue;
}
}

// We're now done with this argument.
++ArgIdx;

// If we just saw a pack expansion into a non-pack, then directly convert
// the remaining arguments, because we don't know what parameters they'll
// match up with.
Expand Down Expand Up @@ -5731,10 +5724,14 @@ 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).
while (ArgIdx < NumArgs && NewArgs[ArgIdx].getArgument().isPackExpansion()) {
const TemplateArgument &Arg = NewArgs[ArgIdx++].getArgument();
SugaredConverted.push_back(Arg);
CanonicalConverted.push_back(Context.getCanonicalTemplateArgument(Arg));
if (ArgIdx < NumArgs && CurrentInstantiationScope &&
CurrentInstantiationScope->getPartiallySubstitutedPack()) {
while (ArgIdx < NumArgs &&
NewArgs[ArgIdx].getArgument().isPackExpansion()) {
const TemplateArgument &Arg = NewArgs[ArgIdx++].getArgument();
SugaredConverted.push_back(Arg);
CanonicalConverted.push_back(Context.getCanonicalTemplateArgument(Arg));
}
}

// If we have any leftover arguments, then there were too many arguments.
Expand Down Expand Up @@ -7324,46 +7321,64 @@ 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 (!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;
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;

Template->getAssociatedConstraints(TemplateAC);
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;

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;
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.
}
return false;

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

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

0 comments on commit ada6372

Please sign in to comment.