Reland: [clang] unified CWG2398 and P0522 changes; finishes implementation of P3310 #124137
Conversation
This file contains hidden or 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
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.
With this change, we discriminate if the primary template and which partial specializations would have participated in overload resolution prior to P0522 changes. We collect those in an initial set. If this set is not empty, or the primary template would have matched, we proceed with this set as the candidates for overload resolution. Otherwise, we build a new overload set with everything else, and proceed as usual.
…emplate calls. Clang previously missed implementing P0522 pack matching for deduced function template calls.
mizvekov
requested review from
zygoloid,
cor3ntin,
erichkeane,
sdkrystian and
zyn0217
llvmbot
added
clang
|
@llvm/pr-subscribers-libcxx @llvm/pr-subscribers-clang Author: Matheus Izvekov (mizvekov) ChangesThis patch relands the following PRs:
All of these patches were reverted due to issue reported in #111711 (comment), due to interdependencies. In order to avoid churn, this relands all of them in one go. All of the patches are identical to the originally reverted ones, except "[clang] Changes to template argument list checking", which contains the changes which fix the revert-causing issue. Patch is 121.28 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/124137.diff 21 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5d4b182f29afa0..7801bad3439142 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -325,6 +325,10 @@ C++20 Feature Support
- Implemented module level lookup for C++20 modules. (#GH90154)
+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
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -351,7 +355,8 @@ Resolutions to C++ Defect Reports
(`CWG2351: void{} <https://cplusplus.github.io/CWG/issues/2351.html>`_).
- Clang now has improved resolution to CWG2398, allowing class templates to have
- default arguments deduced when partial ordering.
+ default arguments deduced when partial ordering, and better backwards compatibility
+ in overload resolution.
- Clang now allows comparing unequal object pointers that have been cast to ``void *``
in constant expressions. These comparisons always worked in non-constant expressions.
@@ -636,6 +641,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).
@@ -885,6 +894,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
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 36b693c6a304e7..774e5484cfa0e7 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5323,6 +5323,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">;
diff --git a/clang/include/clang/Sema/Overload.h b/clang/include/clang/Sema/Overload.h
index 176a2a8d2a35e5..c7f2422b542dd1 100644
--- a/clang/include/clang/Sema/Overload.h
+++ b/clang/include/clang/Sema/Overload.h
@@ -930,6 +930,11 @@ class Sema;
LLVM_PREFERRED_TYPE(bool)
unsigned TookAddressOfOverload : 1;
+ /// Have we matched any packs on the parameter side, versus any non-packs on
+ /// the argument side, in a context where the opposite matching is also
+ /// allowed?
+ bool HasMatchedPackOnParmToNonPackOnArg : 1;
+
/// True if the candidate was found using ADL.
LLVM_PREFERRED_TYPE(CallExpr::ADLCallKind)
unsigned IsADLCandidate : 1;
@@ -1006,6 +1011,7 @@ class Sema;
OverloadCandidate()
: IsSurrogate(false), IgnoreObjectArgument(false),
TookAddressOfOverload(false),
+ HasMatchedPackOnParmToNonPackOnArg(false),
IsADLCandidate(llvm::to_underlying(CallExpr::NotADL)),
RewriteKind(CRK_None) {}
};
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 9a9998b114e0f7..4d6e02fe2956e0 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -10169,7 +10169,8 @@ class Sema final : public SemaBase {
ADLCallKind IsADLCandidate = ADLCallKind::NotADL,
ConversionSequenceList EarlyConversions = {},
OverloadCandidateParamOrder PO = {},
- bool AggregateCandidateDeduction = false);
+ bool AggregateCandidateDeduction = false,
+ bool HasMatchedPackOnParmToNonPackOnArg = false);
/// Add all of the function declarations in the given function set to
/// the overload candidate set.
@@ -10204,7 +10205,8 @@ class Sema final : public SemaBase {
bool SuppressUserConversions = false,
bool PartialOverloading = false,
ConversionSequenceList EarlyConversions = {},
- OverloadCandidateParamOrder PO = {});
+ OverloadCandidateParamOrder PO = {},
+ bool HasMatchedPackOnParmToNonPackOnArg = false);
/// Add a C++ member function template as a candidate to the candidate
/// set, using template argument deduction to produce an appropriate member
@@ -10250,7 +10252,8 @@ class Sema final : public SemaBase {
CXXConversionDecl *Conversion, DeclAccessPair FoundDecl,
CXXRecordDecl *ActingContext, Expr *From, QualType ToType,
OverloadCandidateSet &CandidateSet, bool AllowObjCConversionOnExplicit,
- bool AllowExplicit, bool AllowResultConversion = true);
+ bool AllowExplicit, bool AllowResultConversion = true,
+ bool HasMatchedPackOnParmToNonPackOnArg = false);
/// Adds a conversion function template specialization
/// candidate to the overload set, using template argument deduction
@@ -11678,7 +11681,8 @@ class Sema final : public SemaBase {
SourceLocation RAngleLoc, unsigned ArgumentPackIndex,
SmallVectorImpl<TemplateArgument> &SugaredConverted,
SmallVectorImpl<TemplateArgument> &CanonicalConverted,
- CheckTemplateArgumentKind CTAK);
+ CheckTemplateArgumentKind CTAK, bool PartialOrdering,
+ bool *MatchedPackOnParmToNonPackOnArg);
/// Check that the given template arguments can be provided to
/// the given template, converting the arguments along the way.
@@ -11725,7 +11729,8 @@ class Sema final : public SemaBase {
SmallVectorImpl<TemplateArgument> &SugaredConverted,
SmallVectorImpl<TemplateArgument> &CanonicalConverted,
bool UpdateArgsWithConversions = true,
- bool *ConstraintsNotSatisfied = nullptr, bool PartialOrderingTTP = false);
+ bool *ConstraintsNotSatisfied = nullptr, bool PartialOrderingTTP = false,
+ bool *MatchedPackOnParmToNonPackOnArg = nullptr);
bool CheckTemplateTypeArgument(
TemplateTypeParmDecl *Param, TemplateArgumentLoc &Arg,
@@ -11759,7 +11764,9 @@ class Sema final : public SemaBase {
/// It returns true if an error occurred, and false otherwise.
bool CheckTemplateTemplateArgument(TemplateTemplateParmDecl *Param,
TemplateParameterList *Params,
- TemplateArgumentLoc &Arg, bool IsDeduced);
+ TemplateArgumentLoc &Arg,
+ bool PartialOrdering,
+ bool *MatchedPackOnParmToNonPackOnArg);
void NoteTemplateLocation(const NamedDecl &Decl,
std::optional<SourceRange> ParamRange = {});
@@ -12270,8 +12277,8 @@ class Sema final : public SemaBase {
SmallVectorImpl<DeducedTemplateArgument> &Deduced,
unsigned NumExplicitlySpecified, FunctionDecl *&Specialization,
sema::TemplateDeductionInfo &Info,
- SmallVectorImpl<OriginalCallArg> const *OriginalCallArgs = nullptr,
- bool PartialOverloading = false,
+ SmallVectorImpl<OriginalCallArg> const *OriginalCallArgs,
+ bool PartialOverloading, bool PartialOrdering,
llvm::function_ref<bool()> CheckNonDependent = [] { return false; });
/// Perform template argument deduction from a function call
@@ -12305,7 +12312,8 @@ class Sema final : public SemaBase {
TemplateArgumentListInfo *ExplicitTemplateArgs, ArrayRef<Expr *> Args,
FunctionDecl *&Specialization, sema::TemplateDeductionInfo &Info,
bool PartialOverloading, bool AggregateDeductionCandidate,
- QualType ObjectType, Expr::Classification ObjectClassification,
+ bool PartialOrdering, QualType ObjectType,
+ Expr::Classification ObjectClassification,
llvm::function_ref<bool(ArrayRef<QualType>)> CheckNonDependent);
/// Deduce template arguments when taking the address of a function
@@ -12458,8 +12466,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 PartialOrdering, bool *MatchedPackOnParmToNonPackOnArg);
/// Mark which template parameters are used in a given expression.
///
@@ -12768,6 +12777,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?
@@ -12989,6 +13001,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();
@@ -13450,7 +13468,8 @@ class Sema final : public SemaBase {
bool InstantiateClassTemplateSpecialization(
SourceLocation PointOfInstantiation,
ClassTemplateSpecializationDecl *ClassTemplateSpec,
- TemplateSpecializationKind TSK, bool Complain = true);
+ TemplateSpecializationKind TSK, bool Complain = true,
+ bool PrimaryHasMatchedPackOnParmToNonPackOnArg = false);
/// Instantiates the definitions of all of the member
/// of the given class, which is an instantiation of a class template
diff --git a/clang/include/clang/Sema/TemplateDeduction.h b/clang/include/clang/Sema/TemplateDeduction.h
index 28b014fd84e4b3..9c12eef5c42a06 100644
--- a/clang/include/clang/Sema/TemplateDeduction.h
+++ b/clang/include/clang/Sema/TemplateDeduction.h
@@ -51,6 +51,11 @@ class TemplateDeductionInfo {
/// Have we suppressed an error during deduction?
bool HasSFINAEDiagnostic = false;
+ /// Have we matched any packs on the parameter side, versus any non-packs on
+ /// the argument side, in a context where the opposite matching is also
+ /// allowed?
+ bool MatchedPackOnParmToNonPackOnArg = false;
+
/// The template parameter depth for which we're performing deduction.
unsigned DeducedDepth;
@@ -87,6 +92,14 @@ class TemplateDeductionInfo {
return DeducedDepth;
}
+ bool hasMatchedPackOnParmToNonPackOnArg() const {
+ return MatchedPackOnParmToNonPackOnArg;
+ }
+
+ void setMatchedPackOnParmToNonPackOnArg() {
+ MatchedPackOnParmToNonPackOnArg = true;
+ }
+
/// Get the number of explicitly-specified arguments.
unsigned getNumExplicitArgs() const {
return ExplicitArgs;
diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp
index 30dfa5481d070a..1ea4a2e9e88cf5 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -459,6 +459,8 @@ class DefaultTemplateInstCallback : public TemplateInstantiationCallback {
return "BuildingDeductionGuides";
case CodeSynthesisContext::TypeAliasTemplateInstantiation:
return "TypeAliasTemplateInstantiation";
+ case CodeSynthesisContext::PartialOrderingTTP:
+ return "PartialOrderingTTP";
}
return "";
}
diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index e18e3c197383e2..2ed8d3608d49ec 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -3675,7 +3675,9 @@ Sema::LookupLiteralOperator(Scope *S, LookupResult &R,
TemplateArgumentLoc Arg(TemplateArgument(StringLit), StringLit);
if (CheckTemplateArgument(
Params->getParam(0), Arg, FD, R.getNameLoc(), R.getNameLoc(),
- 0, SugaredChecked, CanonicalChecked, CTAK_Specified) ||
+ 0, SugaredChecked, CanonicalChecked, CTAK_Specified,
+ /*PartialOrdering=*/false,
+ /*MatchedPackOnParmToNonPackOnArg=*/nullptr) ||
Trap.hasErrorOccurred())
IsTemplate = false;
}
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 23056ca5deba3c..6ae9c51c06b315 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -6917,7 +6917,8 @@ void Sema::AddOverloadCandidate(
OverloadCandidateSet &CandidateSet, bool SuppressUserConversions,
bool PartialOverloading, bool AllowExplicit, bool AllowExplicitConversions,
ADLCallKind IsADLCandidate, ConversionSequenceList EarlyConversions,
- OverloadCandidateParamOrder PO, bool AggregateCandidateDeduction) {
+ OverloadCandidateParamOrder PO, bool AggregateCandidateDeduction,
+ bool HasMatchedPackOnParmToNonPackOnArg) {
const FunctionProtoType *Proto
= dyn_cast<FunctionProtoType>(Function->getType()->getAs<FunctionType>());
assert(Proto && "Functions without a prototype cannot be overloaded");
@@ -6936,7 +6937,8 @@ void Sema::AddOverloadCandidate(
AddMethodCandidate(Method, FoundDecl, Method->getParent(), QualType(),
Expr::Classification::makeSimpleLValue(), Args,
CandidateSet, SuppressUserConversions,
- PartialOverloading, EarlyConversions, PO);
+ PartialOverloading, EarlyConversions, PO,
+ HasMatchedPackOnParmToNonPackOnArg);
return;
}
// We treat a constructor like a non-member function, since its object
@@ -6979,6 +6981,8 @@ void Sema::AddOverloadCandidate(
CandidateSet.getRewriteInfo().getRewriteKind(Function, PO);
Candidate.IsADLCandidate = llvm::to_underlying(IsADLCandidate);
Candidate.ExplicitCallArguments = Args.size();
+ Candidate.HasMatchedPackOnParmToNonPackOnArg =
+ HasMatchedPackOnParmToNonPackOnArg;
// Explicit functions are not actually candidates at all if we're not
// allowing them in this context, but keep them around so we can point
@@ -7521,16 +7525,13 @@ void Sema::AddMethodCandidate(DeclAccessPair FoundDecl, QualType ObjectType,
}
}
-void
-Sema::AddMethodCandidate(CXXMethodDecl *Method, DeclAccessPair FoundDecl,
- CXXRecordDecl *ActingContext, QualType ObjectType,
- Expr::Classification ObjectClassification,
- ArrayRef<Expr *> Args,
- OverloadCandidateSet &CandidateSet,
- bool SuppressUserConversions,
- bool PartialOverloading,
- ConversionSequenceList EarlyConversions,
- OverloadCandidateParamOrder PO) {
+void Sema::AddMethodCandidate(
+ CXXMethodDecl *Method, DeclAccessPair FoundDecl,
+ CXXRecordDecl *ActingContext, QualType ObjectType,
+ Expr::Classification ObjectClassification, ArrayRef<Expr *> Args,
+ OverloadCandidateSet &CandidateSet, bool SuppressUserConversions,
+ bool PartialOverloading, ConversionSequenceList EarlyConversions,
+ OverloadCandidateParamOrder PO, bool HasMatchedPackOnParmToNonPackOnArg) {
const FunctionProtoType *Proto
= dyn_cast<FunctionProtoType>(Method->getType()->getAs<FunctionType>());
assert(Proto && "Methods without a prototype cannot be overloaded");
@@ -7561,6 +7562,8 @@ Sema::AddMethodCandidate(CXXMethodDecl *Method, DeclAccessPair FoundDecl,
Candidate.TookAddressOfOverload =
CandidateSet.getKind() == OverloadCandidateSet::CSK_AddressOfOverloadSet;
Candidate.ExplicitCallArguments = Args.size();
+ Candidate.HasMatchedPackOnParmToNonPackOnArg =
+ HasMatchedPackOnParmToNonPackOnArg;
bool IgnoreExplicitObject =
(Method->isExplicitObjectMemberFunction() &&
@@ -7731,8 +7734,8 @@ void Sema::AddMethodTemplateCandidate(
ConversionSequenceList Conversions;
if (TemplateDeductionResult Result = DeduceTemplateArguments(
MethodTmpl, ExplicitTemplateArgs, Args, Specialization, Info,
- PartialOverloading, /*AggregateDeductionCandidate=*/false, ObjectType,
- ObjectClassification,
+ PartialOverloading, /*AggregateDeductionCandidate=*/false,
+ /*PartialOrdering=*/false, ObjectType, ObjectClassification,
[&](ArrayRef<QualType> ParamTypes) {
return CheckNonDependentConversions(
MethodTmpl, ParamTypes, Args, CandidateSet, Conversions,
@@ -7770,7 +7773,8 @@ void Sema::AddMethodTemplateCandidate(
AddMethodCandidate(cast<CXXMethodDecl>(Specialization), FoundDecl,
ActingContext, ObjectType, ObjectClassification, Args,
CandidateSet, SuppressUserConversions, PartialOverloading,
- Conversions, PO);
+ Conversions, PO,
+ Info.hasMatchedPackOnParmToNonPackOnArg());
}
/// Determine whether a given function template has a simple explicit specifier
@@ -7816,6 +7820,7 @@ void Sema::AddTemplateOverloadCandidate(
if (TemplateDeductionResult Result = DeduceTemplateArguments(
FunctionTemplate, ExplicitTemplateArgs, Args, Specialization, Info,
PartialOverloading, AggregateCandidateDeduction,
+ /*PartialOrdering=*/false,
/*ObjectType=*/QualType(),
/*ObjectClassification=*/Expr::Classification(),
[&](ArrayRef<QualType> ParamTypes) {
@@ -7856,7 +7861,8 @@ void Sema::AddTemplateOverloadCandidate(
Specialization, FoundDecl, Args, CandidateSet, SuppressUserConversions,
PartialOverloading, AllowExplicit,
/*AllowExplicitConversions=*/false, IsADLCandidate, Conversions, PO,
- Info.AggregateDeductionCandidateHasMismatchedArity);
+ Info.AggregateDeductionCandidateHasMismatchedArity,
+ Info.hasMatchedPackOnParmToNonPackOnArg());
}
bool Sema::CheckNonDependentConversions(
@@ -7978,7 +7984,8 @@ void Sema::AddConversionCandidate(
CXXConversionDecl *Conversion, DeclAccessPair FoundDecl,
CXXRecordDecl *ActingContext, Expr *From, QualType ToType,
OverloadCandidateSet &CandidateSet, bool AllowObjCConversionOnExplicit,
- bool AllowExplicit, bool AllowResultConversion) {
+ bool AllowExplicit, bool AllowResultConversion,
+ bool HasMatchedPackOnParmToNonPackOnArg) {
assert(!Conversion->getDescribedFunctionTemplate() &&
"Conversion function templates use AddTemplateConversionCandidate");
QualType ConvType = Convers...
[truncated]
|
erichkeane
reviewed
Jan 23, 2025
If you go on the commit list, you can click on the commit which I pointed out in the OP ([clang] Changes to template argument list checking", which contains the changes which fix the revert-causing issue) The commit message there has explanation of the fix. |
Ah, neat. Github helpfully hid the commit message there for me. |
erichkeane
approved these changes
Jan 23, 2025
cor3ntin
reviewed
Jan 23, 2025
mizvekov
force-pushed
the
users/mizvekov/clang-unified-p0522-cwg2398-changes
branch
from
3731183 to
eedbdbd
Compare
cor3ntin
approved these changes
Jan 23, 2025
Implement some missing changes to support checking of argument-side template parameter packs matching parameter-side non-pack parameters, which are required to support non-exact matches in these cases, and applies appropriate conversions which help deducing NTTPs of placeholder types.
mizvekov
force-pushed
the
users/mizvekov/clang-unified-p0522-cwg2398-changes
branch
from
eedbdbd to
833ea13
Compare
llvmbot
added
the
libc++
mizvekov
deleted the
users/mizvekov/clang-unified-p0522-cwg2398-changes
branch
|
Hi, I think there might be a bug introduced here - would you mind taking a look at #125290? |
|
/cherry-pick 28ad897 |
|
Failed to cherry-pick: 28ad897 https://github.com/llvm/llvm-project/actions/runs/13209349652 Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request |
|
Hi @mizvekov , It seems this change may have caused clang to reject the code that has been accepted before. My concern is that GCC and MSVC continue to accept. The sample is the error is https://godbolt.org/z/GrE733f89 I wonder if that is expected? |
|
Hi @Fznamznon First of all, thank you for the example! Yes, clang is absolutely correct to reject this. Note that the design principle of P0522 was to only allow matching of templates to template template parameters where all possible uses of the template template parameter are also valid uses of the template. Slightly simplified reproducer, which hopefully shows the issue better: https://godbolt.org/z/rKq8WeW7e By otherwise allowing this, clang prior to this patch and the other compilers are effectively laundering a template with parameters "class T, char" into a template with parameters "class T, T". This means invalid uses of TSequence thru TT can only be diagnosed when instantiated. Longer example showing the invalid match: https://godbolt.org/z/3vvvET1eh Another example showing broken behavior by other implementations when type checking this: https://godbolt.org/z/bd6sEz6r9 CC: @Bekenn |
|
Hi @mizvekov I believe this change is impacting our downstream compiler. Here is a minimal example: It was accepted by clang 19.1.0 and gcc but not the clang trunk: I'm not an expert, could you explain to me why |
@mizvekov, can you take a look at this? |
|
@ziqingluo-90 I had already responded here: #130778 (comment) I think you moved your post between issues after I had responded. |
@mizvekov Oh, I see! Thank you! |
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 relands the following PRs:
All of these patches were reverted due to issue reported in #111711 (comment), due to interdependencies.
In order to avoid churn, this relands all of them in one go.
All of the patches are identical to the originally reverted ones, except "[clang] Changes to template argument list checking", which contains the changes which fix the revert-causing issue.