[clang] Enable C++17 relaxed template template argument matching by default #89807
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: Matheus Izvekov (mizvekov) ChangesThis patch will finally allow us to mark C++17 support in clang as complete. This is a continuation of the review process from an old PR in phab. Recap: The original patch from phab initially contained no workarounds / provisional resolutions for core defects, In order to implement this as a DR and avoid breaking reasonable code that worked before P0522, this patch implements a provisional resolution for CWG2398: When deducing template template parameters against each other, and the argument side names a template specialization, instead of just deducing A, we instead deduce a synthesized template template parameter based on A, but with it's parameters using the template specialization's arguments as defaults. This does not fix all possible regressions introduced by P0522, but it does seem to match in effect an undocumented workaround implemented by GCC. Though it's unconfirmed how closely we match, as I have not received official word regarding this yet. The driver flag is deprecated with a warning, and it will not have any effect. @yxsamliu Can you confirm this version avoids any breakages in your setup? CC: @yuanfang-chen @MaskRay @chandlerc @jicama @lichray @AaronBallman Patch is 30.33 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/89807.diff 18 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index ed3fd9b1c4a55b..9781fcaa4ff5e9 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -435,7 +435,7 @@ def warn_drv_diagnostics_misexpect_requires_pgo : Warning<
def warn_drv_clang_unsupported : Warning<
"the clang compiler does not support '%0'">;
def warn_drv_deprecated_arg : Warning<
- "argument '%0' is deprecated, use '%1' instead">, InGroup<Deprecated>;
+ "argument '%0' is deprecated%select{|, use '%2' instead}1">, InGroup<Deprecated>;
def warn_drv_deprecated_custom : Warning<
"argument '%0' is deprecated, %1">, InGroup<Deprecated>;
def warn_drv_assuming_mfloat_abi_is : Warning<
diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def
index 8ef6700ecdc78e..2dfe70f29d7c85 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -158,7 +158,6 @@ LANGOPT(GNUAsm , 1, 1, "GNU-style inline assembly")
LANGOPT(Coroutines , 1, 0, "C++20 coroutines")
LANGOPT(CoroAlignedAllocation, 1, 0, "prefer Aligned Allocation according to P2014 Option 2")
LANGOPT(DllExportInlines , 1, 1, "dllexported classes dllexport inline methods")
-LANGOPT(RelaxedTemplateTemplateArgs, 1, 0, "C++17 relaxed matching of template template arguments")
LANGOPT(ExperimentalLibrary, 1, 0, "enable unstable and experimental library features")
LANGOPT(PointerAuthIntrinsics, 1, 0, "pointer authentication intrinsics")
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 922bda721dc780..f907102f7f813b 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3359,11 +3359,8 @@ defm application_extension : BoolFOption<"application-extension",
PosFlag<SetTrue, [], [ClangOption, CC1Option],
"Restrict code to those available for App Extensions">,
NegFlag<SetFalse>>;
-defm relaxed_template_template_args : BoolFOption<"relaxed-template-template-args",
- LangOpts<"RelaxedTemplateTemplateArgs">, DefaultFalse,
- PosFlag<SetTrue, [], [ClangOption, CC1Option],
- "Enable C++17 relaxed template template argument matching">,
- NegFlag<SetFalse>>;
+def frelaxed_template_template_args : Flag<["-"], "frelaxed-template-template-args">, Flags<[]>;
+def fno_relaxed_template_template_args : Flag<["-"], "fno-relaxed-template-template-args">, Flags<[]>;
defm sized_deallocation : BoolFOption<"sized-deallocation",
LangOpts<"SizedDeallocation">, DefaultFalse,
PosFlag<SetTrue, [], [ClangOption, CC1Option],
diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index 6a4f2548c0bffa..aa416b9844ee1d 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -797,7 +797,8 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
Arg->claim();
if (LegacySanitizeCoverage != 0 && DiagnoseErrors) {
D.Diag(diag::warn_drv_deprecated_arg)
- << Arg->getAsString(Args) << "-fsanitize-coverage=trace-pc-guard";
+ << Arg->getAsString(Args) << true
+ << "-fsanitize-coverage=trace-pc-guard";
}
continue;
}
@@ -833,11 +834,11 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
// enabled.
if (CoverageFeatures & CoverageTraceBB)
D.Diag(clang::diag::warn_drv_deprecated_arg)
- << "-fsanitize-coverage=trace-bb"
+ << "-fsanitize-coverage=trace-bb" << true
<< "-fsanitize-coverage=trace-pc-guard";
if (CoverageFeatures & Coverage8bitCounters)
D.Diag(clang::diag::warn_drv_deprecated_arg)
- << "-fsanitize-coverage=8bit-counters"
+ << "-fsanitize-coverage=8bit-counters" << true
<< "-fsanitize-coverage=trace-pc-guard";
}
@@ -849,7 +850,7 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
if ((CoverageFeatures & InsertionPointTypes) &&
!(CoverageFeatures & InstrumentationTypes) && DiagnoseErrors) {
D.Diag(clang::diag::warn_drv_deprecated_arg)
- << "-fsanitize-coverage=[func|bb|edge]"
+ << "-fsanitize-coverage=[func|bb|edge]" << true
<< "-fsanitize-coverage=[func|bb|edge],[trace-pc-guard|trace-pc],["
"control-flow]";
}
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index e7ccf9a23e7eda..bd6e8292b16830 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6561,7 +6561,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
if (const Arg *A =
Args.getLastArg(options::OPT_fvisibility_global_new_delete_hidden)) {
D.Diag(diag::warn_drv_deprecated_arg)
- << A->getAsString(Args)
+ << A->getAsString(Args) << true
<< "-fvisibility-global-new-delete=force-hidden";
}
@@ -7288,11 +7288,10 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
Args.addOptOutFlag(CmdArgs, options::OPT_fassume_unique_vtables,
options::OPT_fno_assume_unique_vtables);
- // -frelaxed-template-template-args is off by default, as it is a severe
- // breaking change until a corresponding change to template partial ordering
- // is provided.
- Args.addOptInFlag(CmdArgs, options::OPT_frelaxed_template_template_args,
- options::OPT_fno_relaxed_template_template_args);
+ // -frelaxed-template-template-args is deprecated, with no effect.
+ if (Arg *A = Args.getLastArg(options::OPT_frelaxed_template_template_args,
+ options::OPT_fno_relaxed_template_template_args))
+ D.Diag(diag::warn_drv_deprecated_arg) << A->getAsString(Args) << false;
// -fsized-deallocation is off by default, as it is an ABI-breaking change for
// most platforms.
diff --git a/clang/lib/Frontend/InitPreprocessor.cpp b/clang/lib/Frontend/InitPreprocessor.cpp
index 4f44c3b7b89d4d..1cffd59b91ab80 100644
--- a/clang/lib/Frontend/InitPreprocessor.cpp
+++ b/clang/lib/Frontend/InitPreprocessor.cpp
@@ -713,8 +713,8 @@ static void InitializeCPlusPlusFeatureTestMacros(const LangOptions &LangOpts,
}
if (LangOpts.AlignedAllocation && !LangOpts.AlignedAllocationUnavailable)
Builder.defineMacro("__cpp_aligned_new", "201606L");
- if (LangOpts.RelaxedTemplateTemplateArgs)
- Builder.defineMacro("__cpp_template_template_args", "201611L");
+
+ Builder.defineMacro("__cpp_template_template_args", "201611L");
// C++20 features.
if (LangOpts.CPlusPlus20) {
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 4bda31ba67c02d..3a8d58a9352b94 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -8343,58 +8343,52 @@ bool Sema::CheckTemplateTemplateArgument(TemplateTemplateParmDecl *Param,
// 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.
- // FIXME: We should enable RelaxedTemplateTemplateArgs by default as it is a
- // defect report resolution from C++17 and shouldn't be introduced by
- // concepts.
- 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;
+ // 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,
- Arg.getLocation())) {
- // 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, Template,
+ Arg.getLocation())) {
+ // 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;
- 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;
+ 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;
}
- // FIXME: Produce better diagnostics for deduction failures.
+ return false;
}
+ // FIXME: Produce better diagnostics for deduction failures.
return !TemplateParameterListsAreEqual(Template->getTemplateParameters(),
Params,
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index 0b6375001f5326..890d6bff9f4073 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -507,10 +507,62 @@ static TemplateDeductionResult DeduceNonTypeTemplateArgument(
S, TemplateParams, NTTP, DeducedTemplateArgument(New), T, Info, Deduced);
}
+static NamedDecl *DeduceTemplateArguments(Sema &S, NamedDecl *A,
+ TemplateArgument Default) {
+ switch (A->getKind()) {
+ case Decl::TemplateTypeParm: {
+ auto *T = cast<TemplateTypeParmDecl>(A);
+ // FIXME: DefaultArgument can't represent a pack.
+ if (T->isParameterPack())
+ return A;
+ auto *R = TemplateTypeParmDecl::Create(
+ S.Context, A->getDeclContext(), SourceLocation(), SourceLocation(),
+ T->getDepth(), T->getIndex(), T->getIdentifier(),
+ T->wasDeclaredWithTypename(), /*ParameterPack=*/false,
+ T->hasTypeConstraint());
+ R->setDefaultArgument(
+ S.Context.getTrivialTypeSourceInfo(Default.getAsType()));
+ if (R->hasTypeConstraint()) {
+ auto *C = R->getTypeConstraint();
+ R->setTypeConstraint(C->getConceptReference(),
+ C->getImmediatelyDeclaredConstraint());
+ }
+ return R;
+ }
+ case Decl::NonTypeTemplateParm: {
+ auto *T = cast<NonTypeTemplateParmDecl>(A);
+ // FIXME: DefaultArgument can't represent a pack.
+ if (T->isParameterPack())
+ return A;
+ auto *R = NonTypeTemplateParmDecl::Create(
+ S.Context, A->getDeclContext(), SourceLocation(), SourceLocation(),
+ T->getDepth(), T->getIndex(), T->getIdentifier(), T->getType(),
+ /*ParameterPack=*/false, T->getTypeSourceInfo());
+ R->setDefaultArgument(Default.getAsExpr());
+ if (auto *PTC = T->getPlaceholderTypeConstraint())
+ R->setPlaceholderTypeConstraint(PTC);
+ return R;
+ }
+ case Decl::TemplateTemplateParm: {
+ auto *T = cast<TemplateTemplateParmDecl>(A);
+ auto *R = TemplateTemplateParmDecl::Create(
+ S.Context, A->getDeclContext(), SourceLocation(), T->getDepth(),
+ T->getIndex(), T->isParameterPack(), T->getIdentifier(),
+ T->wasDeclaredWithTypename(), T->getTemplateParameters());
+ R->setDefaultArgument(
+ S.Context, TemplateArgumentLoc(Default, TemplateArgumentLocInfo()));
+ return R;
+ }
+ default:
+ llvm_unreachable("Unexpected Decl Kind");
+ }
+}
+
static TemplateDeductionResult
DeduceTemplateArguments(Sema &S, TemplateParameterList *TemplateParams,
TemplateName Param, TemplateName Arg,
TemplateDeductionInfo &Info,
+ ArrayRef<TemplateArgument> DefaultArguments,
SmallVectorImpl<DeducedTemplateArgument> &Deduced) {
TemplateDecl *ParamDecl = Param.getAsTemplateDecl();
if (!ParamDecl) {
@@ -519,13 +571,45 @@ DeduceTemplateArguments(Sema &S, TemplateParameterList *TemplateParams,
return TemplateDeductionResult::Success;
}
- if (TemplateTemplateParmDecl *TempParam
- = dyn_cast<TemplateTemplateParmDecl>(ParamDecl)) {
+ if (auto *TempParam = dyn_cast<TemplateTemplateParmDecl>(ParamDecl)) {
// If we're not deducing at this depth, there's nothing to deduce.
if (TempParam->getDepth() != Info.getDeducedDepth())
return TemplateDeductionResult::Success;
- DeducedTemplateArgument NewDeduced(S.Context.getCanonicalTemplateName(Arg));
+ auto NewDeduced = DeducedTemplateArgument(Arg);
+ // Provisional resolution for CWG2398: If Arg is also a template template
+ // param, and it names a template specialization, then we deduce a
+ // synthesized template template parameter based on A, but using the TS's
+ // arguments as defaults.
+ if (auto *TempArg = dyn_cast_or_null<TemplateTemplateParmDecl>(
+ Arg.getAsTemplateDecl())) {
+ assert(Arg.getKind() == TemplateName::Template);
+ assert(!TempArg->isExpandedParameterPack());
+
+ TemplateParameterList *As = TempArg->getTemplateParameters();
+ if (DefaultArguments.size() != 0) {
+ assert(DefaultArguments.size() <= As->size());
+ SmallVector<NamedDecl *, 4> Params(As->size());
+ for (unsigned I = 0; I < DefaultArguments.size(); ++I)
+ Params[I] =
+ DeduceTemplateArguments(S, As->getParam(I), DefaultArguments[I]);
+ for (unsigned I = DefaultArguments.size(); I < As->size(); ++I)
+ Params[I] = As->getParam(I);
+ // FIXME: We could unique these, and also the parameters, but we don't
+ // expect programs to contain a large enough amount of these deductions
+ // for that to be worthwhile.
+ auto *TPL = TemplateParameterList::Create(
+ S.Context, SourceLocation(), SourceLocation(), Params,
+ SourceLocation(), As->getRequiresClause());
+ NewDeduced = DeducedTemplateArgument(
+ TemplateName(TemplateTemplateParmDecl::Create(
+ S.Context, TempArg->getDeclContext(), SourceLocation(),
+ TempArg->getDepth(), TempArg->getPosition(),
+ TempArg->isParameterPack(), TempArg->getIdentifier(),
+ TempArg->wasDeclaredWithTypename(), TPL)));
+ }
+ }
+
DeducedTemplateArgument Result = checkDeducedTemplateArguments(S.Context,
Deduced[TempParam->getIndex()],
NewDeduced);
@@ -604,7 +688,8 @@ DeduceTemplateSpecArguments(Sema &S, TemplateParameterList *TemplateParams,
// Perform template argument deduction for the template name.
if (auto Result =
- DeduceTemplateArguments(S, TemplateParams, TNP, TNA, Info, Deduced);
+ DeduceTemplateArguments(S, TemplateParams, TNP, TNA, Info,
+ SA->template_arguments(), Deduced);
Result != TemplateDeductionResult::Success)
return Result;
// Perform template argument deduction on each template
@@ -630,7 +715,8 @@ DeduceTemplateSpecArguments(Sema &S, TemplateParameterList *TemplateParams,
// Perform template argument deduction for the template name.
if (auto Result = DeduceTemplateArguments(
S, TemplateParams, TP->getTemplateName(),
- TemplateName(SA->getSpecializedTemplate()), Info, Deduced);
+ TemplateName(SA->getSpecializedTemplate()), Info,
+ SA->getTemplateArgs().asArray(), Deduced);
Result != TemplateDeductionResult::Success)
return Result;
@@ -2320,7 +2406,8 @@ DeduceTemplateArguments(Sema &S, TemplateParameterList *TemplateParams,
case TemplateArgument::Template:
if (A.getKind() == TemplateArgument::Template)
return DeduceTemplateArguments(S, TemplateParams, P.getAsTemplate(),
- A.getAsTemplate(), Info, Deduced);
+ A.getAsTemplate(), Info,
+ /*DefaultArguments=*/{}, Deduced);
Info.FirstArg = P;
Info.SecondArg = A;
return TemplateDeductionResult::NonDeducedMismatch;
diff --git a/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-2a.cpp b/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-2a.cpp
index f586069638614b..342ffba53dbfaf 100644
--- a/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-2a.cpp
+++ b/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-2a.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++2a -frelaxed-template-template-args -verify %s
+// RUN: %clang_cc1 -std=c++2a -verify %s
template<typename T> concept C = T::f(); // #C
template<typename T> concept D = C<T> && T::g();
diff --git a/clang/test/CodeGenCXX/mangle-concept.cpp b/clang/test/CodeGenCXX/mangle-concept.cpp
index bbd2cf6555e3ec..e9c46d87635abb 100644
--- a/clang/test/CodeGenCXX/mangle-concept.cpp
+++ b/clang/test/CodeGenCXX/mangle-concept.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -verify -frelaxed-template-template-args -std=c++20 -emit-llvm -triple %itanium_abi_triple -o - %s -fclang-abi-compat=latest | FileCheck %s
-// RUN: %clang_cc1 -verify -frelaxed-template-template-args -std=c++20 -emit-llvm -triple %itanium_abi_triple -o - %s -fclang-abi-compat=16 | FileCheck %s --check-prefix=CLANG16
+// RUN: %clang_cc1 -verify -std=c++20 -emit-llvm -triple %itanium_abi_triple -o - %s -fclang-abi-compat=...
[truncated]
|
erichkeane
reviewed
Apr 23, 2024
erichkeane
reviewed
Apr 23, 2024
cor3ntin
reviewed
Apr 24, 2024
There was a problem hiding this comment.
I'm pretty happy with the direction this is going in.
I'd like to see the specific test in https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#2398, the ones we exchanged by mails and some tests with constraints
|
Reference to #55894 . |
|
So Jason pointed out that GCC's provisional wording for CWG2398 picks a dubious candidate for this example: template<typename T, typename U> struct match2;
template<template<typename A> class t1,typename T>
struct match2<t1<T>, typename t1<T>::type > { typedef int type; }; // #5
template<template<typename B, typename C> class t2,typename T0,typename T1>
struct match2<t2<T0,T1>, typename t2<T0,T0>::type > { typedef int type; }; // #6
template <class T, class U = T> struct Q { typedef int type; };
match2<Q<int>, int> m;They pick |
I confirm this version avoids breakages in rocThrust which were reported before. Thanks. |
mizvekov
force-pushed
the
users/mizvekov/clang-deprecate-frelaxed-template-template-args
branch
from
873043f
to
3f6e50e
Compare
mizvekov
changed the title
Endilll
previously requested changes
Apr 26, 2024
mizvekov
force-pushed
the
users/mizvekov/clang-deprecate-frelaxed-template-template-args
branch
from
3f6e50e
to
43f813d
Compare
mizvekov
requested review from
Endilll,
cor3ntin and
erichkeane
and removed request for
Endilll
|
@mizvekov We have a bunch of related issues, could you look at them (and add tests + "Fixes #XXXX` to the commit message, as well as mentioning all the fixed issues in the release notes) Thanks! |
mizvekov
force-pushed
the
users/mizvekov/clang-deprecate-frelaxed-template-template-args
branch
from
43f813d
to
4ee58ef
Compare
Removed a bunch of duplicates. 4 issues remaining:
|
Can you provide more information? How widespread are we talking about? You have example of common breakages? |
|
Thank you for the quick response to this! |
|
No problem! It looks like this example is salvageable, nothing is stopping us from just applying the same rules when deducing a template template parameter against other kinds of templates. This shouldn't stop you from cleaning up the code, whatever rules we come up here are for salvaging old code, not necessarily to entomb these as examples of good practice. |
mizvekov
added a commit
that referenced
this pull request
May 21, 2024
This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. When performing template argument deduction, we extend the provisional wording introduced in #89807 so it also covers deduction of class templates. Given the following example: ```C++ template <class T1, class T2 = float> struct A; template <class T3> struct B; template <template <class T4> class TT1, class T5> struct B<TT1<T5>>; // #1 template <class T6, class T7> struct B<A<T6, T7>>; // #2 template struct B<A<int>>; ``` Prior to P0522, `#2` was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, `#2` is picked again. This has the beneficial side effect of making the following code valid: ```C++ template<class T, class U> struct A {}; A<int, float> v; template<template<class> class TT> void f(TT<int>); // OK: TT picks 'float' as the default argument for the second parameter. void g() { f(v); } ``` --- Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here.
This was referenced May 21, 2024
mizvekov
added a commit
that referenced
this pull request
May 22, 2024
This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. When performing template argument deduction, we extend the provisional wording introduced in #89807 so it also covers deduction of class templates. Given the following example: ```C++ template <class T1, class T2 = float> struct A; template <class T3> struct B; template <template <class T4> class TT1, class T5> struct B<TT1<T5>>; // #1 template <class T6, class T7> struct B<A<T6, T7>>; // #2 template struct B<A<int>>; ``` Prior to P0522, `#2` was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, `#2` is picked again. This has the beneficial side effect of making the following code valid: ```C++ template<class T, class U> struct A {}; A<int, float> v; template<template<class> class TT> void f(TT<int>); // OK: TT picks 'float' as the default argument for the second parameter. void g() { f(v); } ``` --- Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here.
mizvekov
added a commit
that referenced
this pull request
May 22, 2024
…#92855) This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. When performing template argument deduction, we extend the provisional wording introduced in #89807 so it also covers deduction of class templates. Given the following example: ```C++ template <class T1, class T2 = float> struct A; template <class T3> struct B; template <template <class T4> class TT1, class T5> struct B<TT1<T5>>; // #1 template <class T6, class T7> struct B<A<T6, T7>>; // #2 template struct B<A<int>>; ``` Prior to P0522, `#2` was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, `#2` is picked again. This has the beneficial side effect of making the following code valid: ```C++ template<class T, class U> struct A {}; A<int, float> v; template<template<class> class TT> void f(TT<int>); // OK: TT picks 'float' as the default argument for the second parameter. void g() { f(v); } ``` --- Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here.
|
@mizvekov - can you please take a look at https://godbolt.org/z/ahro3vnPd ? This is what I ended up reducing yet another The reduced example fails somewhat differently, but it complains on |
Thank you, this example does look interesting. I will try to reduce further. |
|
@eaeltsin You example boils down to https://godbolt.org/z/axnanxP1z: template <class> struct xtype_for_shape;
template <template <class, long> class S, class X, long N>
struct xtype_for_shape<S<X, N>> {};
template <class, bool> struct svector;
template struct xtype_for_shape<svector<int, false>>;There is wide divergence between implementations on what happens during deduction, at least when NTTPs are involved. clang picks the partial specialization, all other implementations pick the primary template. The other implementations reject the matching in deduction, but all implementations agree that it would work if directly specified: https://godbolt.org/z/Gfs3dbYEe (discounting EDG which doesn't seem to implement P0522R0 at all). I think what clang does makes the most sense; I see no point in having deduction be more strict than the matching itself. Also, you can create a similar situation with NTTPs where this adds new ambiguity: https://godbolt.org/z/cWPM7jWvd |
|
Thanks @mizvekov ! This explanation sheds some light on why the original tests diverge in behavior so much, will look further. Is there a way to dump which template was picked for which instantiation? (I tried templight but it doesn't seem to handle this case yet, for me it went in one step through the recursive instantiation that hits the depth limit). |
|
No, I never used templight for this. I don't know of any easy to use tools that would work for a complex sample without modification. Otherwise, the reduction wasn't difficult, it was mostly that creduce / cvise are lacking steps to reduce template type aliases, and the typedef within a class template equivalent. There is also a seemingly complex pack expansion of a boolean expression, that you can just replace with false; cvise missed this as well. |
|
:( My goal now is to fix xtensor implementation/original tests, so this is not a question of reduction. I need to understand where the compiler picked a different specialization with relaxed argument matching. |
So from the reduction you can see you have a problem where I see that you have a very similar problem to your first one, which is not fixed in main yet, where one of the partial specializations is ifdef'd out in a similar manner, that also very suspiciously looks like it should have been using the feature test macro instead: Does changing that to the feature test macro help? |
|
Huge thanks, @mizvekov! That was a precise guess! |
mizvekov
added a commit
to mizvekov/llvm-project
that referenced
this pull request
Jun 10, 2024
This extends default argument deduction to cover class templates as well, and also applies outside of partial ordering, adding to the provisional wording introduced in llvm#89807. This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. Given the following example: ```C++ template <class T1, class T2 = float> struct A; template <class T3> struct B; template <template <class T4> class TT1, class T5> struct B<TT1<T5>>; // llvm#1 template <class T6, class T7> struct B<A<T6, T7>>; // llvm#2 template struct B<A<int>>; ``` Prior to P0522, `llvm#2` was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, `llvm#2` is picked again. As the consequences are not restricted to partial ordering, the following code becomes valid: ```C++ template<class T, class U> struct A {}; A<int, float> v; template<template<class> class TT> void f(TT<int>); // OK: TT picks 'float' as the default argument for the second parameter. void g() { f(v); } ``` Also, since 'f' deduced from `A<int, float>` is different from 'f' deduced from `A<int, double>`, this implements an additional mangling rule. --- Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here.
mizvekov
added a commit
to mizvekov/llvm-project
that referenced
this pull request
Jun 10, 2024
This extends default argument deduction to cover class templates as well, and also applies outside of partial ordering, adding to the provisional wording introduced in llvm#89807. This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. Given the following example: ```C++ template <class T1, class T2 = float> struct A; template <class T3> struct B; template <template <class T4> class TT1, class T5> struct B<TT1<T5>>; // llvm#1 template <class T6, class T7> struct B<A<T6, T7>>; // llvm#2 template struct B<A<int>>; ``` Prior to P0522, `llvm#2` was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, `llvm#2` is picked again. As the consequences are not restricted to partial ordering, the following code becomes valid: ```C++ template<class T, class U> struct A {}; A<int, float> v; template<template<class> class TT> void f(TT<int>); // OK: TT picks 'float' as the default argument for the second parameter. void g() { f(v); } ``` Also, since 'f' deduced from `A<int, float>` is different from 'f' deduced from `A<int, double>`, this implements an additional mangling rule. --- Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here.
mizvekov
added a commit
that referenced
this pull request
Jun 10, 2024
This extends default argument deduction to cover class templates as well, and also applies outside of partial ordering, adding to the provisional wording introduced in #89807. This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. Given the following example: ```C++ template <class T1, class T2 = float> struct A; template <class T3> struct B; template <template <class T4> class TT1, class T5> struct B<TT1<T5>>; // #1 template <class T6, class T7> struct B<A<T6, T7>>; // #2 template struct B<A<int>>; ``` Prior to P0522, `#2` was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, `#2` is picked again. As the consequences are not restricted to partial ordering, the following code becomes valid: ```C++ template<class T, class U> struct A {}; A<int, float> v; template<template<class> class TT> void f(TT<int>); // OK: TT picks 'float' as the default argument for the second parameter. void g() { f(v); } ``` Also, since 'f' deduced from `A<int, float>` is different from 'f' deduced from `A<int, double>`, this implements an additional mangling rule. --- Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here.
mizvekov
added a commit
that referenced
this pull request
Jun 11, 2024
This extends default argument deduction to cover class templates as well, and also applies outside of partial ordering, adding to the provisional wording introduced in #89807. This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. Given the following example: ```C++ template <class T1, class T2 = float> struct A; template <class T3> struct B; template <template <class T4> class TT1, class T5> struct B<TT1<T5>>; // #1 template <class T6, class T7> struct B<A<T6, T7>>; // #2 template struct B<A<int>>; ``` Prior to P0522, `#2` was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, `#2` is picked again. As the consequences are not restricted to partial ordering, the following code becomes valid: ```C++ template<class T, class U> struct A {}; A<int, float> v; template<template<class> class TT> void f(TT<int>); // OK: TT picks 'float' as the default argument for the second parameter. void g() { f(v); } ``` Also, since 'f' deduced from `A<int, float>` is different from 'f' deduced from `A<int, double>`, this implements an additional mangling rule. --- Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here.
mizvekov
added a commit
that referenced
this pull request
Jun 11, 2024
This extends default argument deduction to cover class templates as well, and also applies outside of partial ordering, adding to the provisional wording introduced in #89807. This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. Given the following example: ```C++ template <class T1, class T2 = float> struct A; template <class T3> struct B; template <template <class T4> class TT1, class T5> struct B<TT1<T5>>; // #1 template <class T6, class T7> struct B<A<T6, T7>>; // #2 template struct B<A<int>>; ``` Prior to P0522, `#2` was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, `#2` is picked again. As the consequences are not restricted to partial ordering, the following code becomes valid: ```C++ template<class T, class U> struct A {}; A<int, float> v; template<template<class> class TT> void f(TT<int>); // OK: TT picks 'float' as the default argument for the second parameter. void g() { f(v); } ``` Also, since 'f' deduced from `A<int, float>` is different from 'f' deduced from `A<int, double>`, this implements an additional mangling rule. --- Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here.
mizvekov
added a commit
that referenced
this pull request
Jun 12, 2024
This extends default argument deduction to cover class templates as well, and also applies outside of partial ordering, adding to the provisional wording introduced in #89807. This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. Given the following example: ```C++ template <class T1, class T2 = float> struct A; template <class T3> struct B; template <template <class T4> class TT1, class T5> struct B<TT1<T5>>; // #1 template <class T6, class T7> struct B<A<T6, T7>>; // #2 template struct B<A<int>>; ``` Prior to P0522, `#2` was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, `#2` is picked again. As the consequences are not restricted to partial ordering, the following code becomes valid: ```C++ template<class T, class U> struct A {}; A<int, float> v; template<template<class> class TT> void f(TT<int>); // OK: TT picks 'float' as the default argument for the second parameter. void g() { f(v); } ``` Also, since 'f' deduced from `A<int, float>` is different from 'f' deduced from `A<int, double>`, this implements an additional mangling rule. --- Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here.
mizvekov
added a commit
that referenced
this pull request
Jun 13, 2024
This extends default argument deduction to cover class templates as well, and also applies outside of partial ordering, adding to the provisional wording introduced in #89807. This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. Given the following example: ```C++ template <class T1, class T2 = float> struct A; template <class T3> struct B; template <template <class T4> class TT1, class T5> struct B<TT1<T5>>; // #1 template <class T6, class T7> struct B<A<T6, T7>>; // #2 template struct B<A<int>>; ``` Prior to P0522, `#2` was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, `#2` is picked again. As the consequences are not restricted to partial ordering, the following code becomes valid: ```C++ template<class T, class U> struct A {}; A<int, float> v; template<template<class> class TT> void f(TT<int>); // OK: TT picks 'float' as the default argument for the second parameter. void g() { f(v); } ``` Also, since 'f' deduced from `A<int, float>` is different from 'f' deduced from `A<int, double>`, this implements an additional mangling rule. --- Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here.
mizvekov
added a commit
that referenced
this pull request
Jun 19, 2024
This extends default argument deduction to cover class templates as well, and also applies outside of partial ordering, adding to the provisional wording introduced in #89807. This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. Given the following example: ```C++ template <class T1, class T2 = float> struct A; template <class T3> struct B; template <template <class T4> class TT1, class T5> struct B<TT1<T5>>; // #1 template <class T6, class T7> struct B<A<T6, T7>>; // #2 template struct B<A<int>>; ``` Prior to P0522, `#2` was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, `#2` is picked again. As the consequences are not restricted to partial ordering, the following code becomes valid: ```C++ template<class T, class U> struct A {}; A<int, float> v; template<template<class> class TT> void f(TT<int>); // OK: TT picks 'float' as the default argument for the second parameter. void g() { f(v); } ``` Also, since 'f' deduced from `A<int, float>` is different from 'f' deduced from `A<int, double>`, this implements an additional mangling rule. --- Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here.
mizvekov
added a commit
that referenced
this pull request
Sep 7, 2024
…#94981) This extends default argument deduction to cover class templates as well, applying only to partial ordering, adding to the provisional wording introduced in #89807. This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. Given the following example: ```C++ template <class T1, class T2 = float> struct A; template <class T3> struct B; template <template <class T4> class TT1, class T5> struct B<TT1<T5>>; // #1 template <class T6, class T7> struct B<A<T6, T7>>; // #2 template struct B<A<int>>; ``` Prior to P0522, `#2` was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, `#2` is picked again.
VitaNuo
pushed a commit
to VitaNuo/llvm-project
that referenced
this pull request
Sep 12, 2024
…llvm#94981) This extends default argument deduction to cover class templates as well, applying only to partial ordering, adding to the provisional wording introduced in llvm#89807. This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. Given the following example: ```C++ template <class T1, class T2 = float> struct A; template <class T3> struct B; template <template <class T4> class TT1, class T5> struct B<TT1<T5>>; // llvm#1 template <class T6, class T7> struct B<A<T6, T7>>; // llvm#2 template struct B<A<int>>; ``` Prior to P0522, `llvm#2` was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, `llvm#2` is picked again.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch will finally allow us to mark C++17 support in clang as complete.
This is a continuation of the review process from an old PR in phab.
Recap: The original patch from phab initially contained no workarounds / provisional resolutions for core defects,
Though testing by @yxsamliu here showed problems with some important libraries used in GPU applications, and we ended up reverting it.
In order to implement this as a DR and avoid breaking reasonable code that worked before P0522, this patch implements a provisional resolution for CWG2398: When deducing template template parameters against each other, and the argument side names a template specialization, instead of just deducing A, we deduce a synthesized template template parameter based on A, but with it's parameters using the template specialization's arguments as defaults.
This does not fix all possible regressions introduced by P0522, but it does seem to match in effect an undocumented workaround implemented by GCC. Though it's unconfirmed how closely we match, as I have not received official word regarding this yet.
Fixing other regressions will require more extensive changes, some of them would require possibly controversial wording changes, and so is left for future work.
The driver flag is deprecated with a warning, and it will not have any effect.
Fixes #36505
@yxsamliu Can you confirm this version avoids any breakages in your setup?
CC: @yuanfang-chen @MaskRay @chandlerc @jicama @lichray @AaronBallman