[clang] Implement instantiation context note for checking template parameters #126088
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
mizvekov
requested review from
zygoloid,
cor3ntin,
AaronBallman,
erichkeane and
zyn0217
llvmbot
added
clang
|
@llvm/pr-subscribers-hlsl @llvm/pr-subscribers-clang-modules Author: Matheus Izvekov (mizvekov) ChangesInstead of manually adding a note pointing to the relevant template parameter to every relevant error, which is very easy to miss, this patch adds a new instantiation context note, so that this can work using RAII magic. This fixes a bunch of places where these notes were missing, and is more future-proof. Some diagnostics are reworked to make better use of this note:
It's likely this will add the note to some diagnostics where the parameter is not super relevant, but this can be reworked with time and the decrease in maintenance burden makes up for it. This bypasses the templight dumper for the new context entry, as the tests are very hard to update. This depends on #125453, which is needed to avoid losing the context note for errors occuring during template argument deduction. Patch is 166.58 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/126088.diff 95 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index dfce173d414ebb2..7e20954c65e67b4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -149,6 +149,16 @@ Bug Fixes to C++ Support
- Clang now prints the correct instantiation context for diagnostics suppressed
by template argument deduction.
+Improvements to C++ diagnostics
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+- Clang now more consistently adds a note pointing to the relevant template
+ parameter. Some diagnostics are reworded to better take advantage of this.
+- Template Template Parameter diagnostics now stop referring to template
+ parameters as template arguments, in some circumstances, better hiding
+ from the users template template parameter partial ordering arcana.
+
+
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 7b3b932c482baa2..6ab26fb25d8238f 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5290,10 +5290,14 @@ def err_template_missing_args : Error<
"%select{class template|function template|variable template|alias template|"
"template template parameter|concept|template}0 %1 requires template "
"arguments">;
-def err_template_arg_list_different_arity : Error<
- "%select{too few|too many}0 template arguments for "
+def err_template_param_missing_arg : Error<
+ "missing template argument for template parameter">;
+def err_template_template_param_missing_param : Error<
+ "missing template parameter to bind to template template parameter">;
+def err_template_too_many_args : Error<
+ "too many template arguments for "
"%select{class template|function template|variable template|alias template|"
- "template template parameter|concept|template}1 %2">;
+ "template template parameter|concept|template}0 %1">;
def note_template_decl_here : Note<"template is declared here">;
def note_template_decl_external : Note<
"template declaration from hidden source: %0">;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index e39930c1a450343..93b1094616d6a1a 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -11781,7 +11781,7 @@ class Sema final : public SemaBase {
bool *ConstraintsNotSatisfied = nullptr);
bool CheckTemplateTypeArgument(
- TemplateTypeParmDecl *Param, TemplateArgumentLoc &Arg,
+ TemplateArgumentLoc &Arg,
SmallVectorImpl<TemplateArgument> &SugaredConverted,
SmallVectorImpl<TemplateArgument> &CanonicalConverted);
@@ -11817,9 +11817,10 @@ class Sema final : public SemaBase {
bool PartialOrdering,
bool *StrictPackMatch);
+ SmallString<128> toTerseString(const NamedDecl &D) const;
+
void NoteTemplateLocation(const NamedDecl &Decl,
std::optional<SourceRange> ParamRange = {});
- void NoteTemplateParameterLocation(const NamedDecl &Decl);
/// Given a non-type template argument that refers to a
/// declaration and the type of its corresponding non-type template
@@ -12821,6 +12822,9 @@ class Sema final : public SemaBase {
/// We are performing partial ordering for template template parameters.
PartialOrderingTTP,
+
+ /// Checking a Template Parameter
+ CheckTemplateParameter,
} Kind;
/// Was the enclosing context a non-instantiation SFINAE context?
@@ -13048,6 +13052,11 @@ class Sema final : public SemaBase {
PartialOrderingTTP, TemplateDecl *PArg,
SourceRange InstantiationRange = SourceRange());
+ struct CheckTemplateParameter {};
+ /// \brief Note that we are checking a template parameter.
+ InstantiatingTemplate(Sema &SemaRef, CheckTemplateParameter,
+ NamedDecl *Param);
+
/// Note that we have finished instantiating this template.
void Clear();
@@ -13081,6 +13090,13 @@ class Sema final : public SemaBase {
InstantiatingTemplate &operator=(const InstantiatingTemplate &) = delete;
};
+ /// For any diagnostics which occur within its scope, adds a context note
+ /// pointing to the declaration of the template parameter.
+ struct CheckTemplateParameterRAII : InstantiatingTemplate {
+ CheckTemplateParameterRAII(Sema &S, NamedDecl *Param)
+ : InstantiatingTemplate(S, CheckTemplateParameter(), Param) {}
+ };
+
bool SubstTemplateArgument(const TemplateArgumentLoc &Input,
const MultiLevelTemplateArgumentList &TemplateArgs,
TemplateArgumentLoc &Output,
diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp
index 1ea4a2e9e88cf59..60e103e643e273b 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -403,7 +403,8 @@ class DefaultTemplateInstCallback : public TemplateInstantiationCallback {
}
private:
- static std::string toString(CodeSynthesisContext::SynthesisKind Kind) {
+ static std::optional<std::string>
+ toString(CodeSynthesisContext::SynthesisKind Kind) {
switch (Kind) {
case CodeSynthesisContext::TemplateInstantiation:
return "TemplateInstantiation";
@@ -461,8 +462,10 @@ class DefaultTemplateInstCallback : public TemplateInstantiationCallback {
return "TypeAliasTemplateInstantiation";
case CodeSynthesisContext::PartialOrderingTTP:
return "PartialOrderingTTP";
+ case CodeSynthesisContext::CheckTemplateParameter:
+ return std::nullopt;
}
- return "";
+ return std::nullopt;
}
template <bool BeginInstantiation>
@@ -470,12 +473,14 @@ class DefaultTemplateInstCallback : public TemplateInstantiationCallback {
const CodeSynthesisContext &Inst) {
std::string YAML;
{
+ std::optional<TemplightEntry> Entry =
+ getTemplightEntry<BeginInstantiation>(TheSema, Inst);
+ if (!Entry)
+ return;
llvm::raw_string_ostream OS(YAML);
llvm::yaml::Output YO(OS);
- TemplightEntry Entry =
- getTemplightEntry<BeginInstantiation>(TheSema, Inst);
llvm::yaml::EmptyContext Context;
- llvm::yaml::yamlize(YO, Entry, true, Context);
+ llvm::yaml::yamlize(YO, *Entry, true, Context);
}
Out << "---" << YAML << "\n";
}
@@ -555,10 +560,13 @@ class DefaultTemplateInstCallback : public TemplateInstantiationCallback {
}
template <bool BeginInstantiation>
- static TemplightEntry getTemplightEntry(const Sema &TheSema,
- const CodeSynthesisContext &Inst) {
+ static std::optional<TemplightEntry>
+ getTemplightEntry(const Sema &TheSema, const CodeSynthesisContext &Inst) {
TemplightEntry Entry;
- Entry.Kind = toString(Inst.Kind);
+ std::optional<std::string> Kind = toString(Inst.Kind);
+ if (!Kind)
+ return std::nullopt;
+ Entry.Kind = *Kind;
Entry.Event = BeginInstantiation ? "Begin" : "End";
llvm::raw_string_ostream OS(Entry.Name);
printEntryName(TheSema, Inst.Entity, OS);
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index f206cd57eca898b..9281cfc950232f1 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -7246,7 +7246,7 @@ static void CheckCXX98CompatAccessibleCopy(Sema &S,
void InitializationSequence::PrintInitLocationNote(Sema &S,
const InitializedEntity &Entity) {
- if (Entity.isParamOrTemplateParamKind() && Entity.getDecl()) {
+ if (Entity.isParameterKind() && Entity.getDecl()) {
if (Entity.getDecl()->getLocation().isInvalid())
return;
@@ -7255,9 +7255,8 @@ void InitializationSequence::PrintInitLocationNote(Sema &S,
<< Entity.getDecl()->getDeclName();
else
S.Diag(Entity.getDecl()->getLocation(), diag::note_parameter_here);
- }
- else if (Entity.getKind() == InitializedEntity::EK_RelatedResult &&
- Entity.getMethodDecl())
+ } else if (Entity.getKind() == InitializedEntity::EK_RelatedResult &&
+ Entity.getMethodDecl())
S.Diag(Entity.getMethodDecl()->getLocation(),
diag::note_method_return_type_change)
<< Entity.getMethodDecl()->getDeclName();
diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp
index ceb32ee15dfa39c..4d278bbc67d281a 100644
--- a/clang/lib/Sema/SemaLambda.cpp
+++ b/clang/lib/Sema/SemaLambda.cpp
@@ -1506,14 +1506,13 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro,
TemplateParameterList *TemplateParams =
getGenericLambdaTemplateParameterList(LSI, *this);
if (TemplateParams) {
- for (const auto *TP : TemplateParams->asArray()) {
+ for (auto *TP : TemplateParams->asArray()) {
if (!TP->getIdentifier())
continue;
+ CheckTemplateParameterRAII CTP(*this, TP);
for (const auto &Capture : Intro.Captures) {
- if (Capture.Id == TP->getIdentifier()) {
+ if (Capture.Id == TP->getIdentifier())
Diag(Capture.Loc, diag::err_template_param_shadow) << Capture.Id;
- NoteTemplateParameterLocation(*TP);
- }
}
}
}
diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index 0f5b7426e743e05..71d9add9d8c098f 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -1586,9 +1586,13 @@ llvm::DenseSet<Module*> &Sema::getLookupModules() {
unsigned N = CodeSynthesisContexts.size();
for (unsigned I = CodeSynthesisContextLookupModules.size();
I != N; ++I) {
- Module *M = CodeSynthesisContexts[I].Entity ?
- getDefiningModule(*this, CodeSynthesisContexts[I].Entity) :
- nullptr;
+ auto &Ctx = CodeSynthesisContexts[I];
+ // FIXME: Are there any other context kinds that shouldn't be looked at
+ // here?
+ if (Ctx.Kind == CodeSynthesisContext::PartialOrderingTTP ||
+ Ctx.Kind == CodeSynthesisContext::CheckTemplateParameter)
+ continue;
+ Module *M = Ctx.Entity ? getDefiningModule(*this, Ctx.Entity) : nullptr;
if (M && !LookupModulesCache.insert(M).second)
M = nullptr;
CodeSynthesisContextLookupModules.push_back(M);
@@ -3709,7 +3713,8 @@ Sema::LookupLiteralOperator(Scope *S, LookupResult &R,
TemplateParameterList *Params = FD->getTemplateParameters();
if (Params->size() == 1) {
IsTemplate = true;
- if (!Params->getParam(0)->isTemplateParameterPack() && !StringLit) {
+ NamedDecl *Param = Params->getParam(0);
+ if (!Param->isTemplateParameterPack() && !StringLit) {
// Implied but not stated: user-defined integer and floating literals
// only ever use numeric literal operator templates, not templates
// taking a parameter of class type.
@@ -3722,6 +3727,7 @@ Sema::LookupLiteralOperator(Scope *S, LookupResult &R,
if (StringLit) {
SFINAETrap Trap(*this);
CheckTemplateArgumentInfo CTAI;
+ CheckTemplateParameterRAII CTP(*this, Param);
TemplateArgumentLoc Arg(TemplateArgument(StringLit), StringLit);
if (CheckTemplateArgument(
Params->getParam(0), Arg, FD, R.getNameLoc(), R.getNameLoc(),
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 9e68972b33f0a03..78093c66c1a53aa 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -869,9 +869,11 @@ void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl,
? diag::ext_template_param_shadow
: (SupportedForCompatibility ? diag::ext_compat_template_param_shadow
: diag::err_template_param_shadow);
- const auto *ND = cast<NamedDecl>(PrevDecl);
+ auto *ND = cast<NamedDecl>(PrevDecl);
+ CheckTemplateParameterRAII CTP(*this, ND);
+ // FIXME: Don't put the name in the diagnostic, unless there is no source
+ // location.
Diag(Loc, DiagId) << ND->getDeclName();
- NoteTemplateParameterLocation(*ND);
}
TemplateDecl *Sema::AdjustDeclIfTemplate(Decl *&D) {
@@ -4824,7 +4826,7 @@ TemplateNameKind Sema::ActOnTemplateName(Scope *S,
}
bool Sema::CheckTemplateTypeArgument(
- TemplateTypeParmDecl *Param, TemplateArgumentLoc &AL,
+ TemplateArgumentLoc &AL,
SmallVectorImpl<TemplateArgument> &SugaredConverted,
SmallVectorImpl<TemplateArgument> &CanonicalConverted) {
const TemplateArgument &Arg = AL.getArgument();
@@ -4880,7 +4882,6 @@ bool Sema::CheckTemplateTypeArgument(
? diag::ext_ms_template_type_arg_missing_typename
: diag::err_template_arg_must_be_type_suggest)
<< FixItHint::CreateInsertion(Loc, "typename ");
- NoteTemplateParameterLocation(*Param);
// Recover by synthesizing a type using the location information that we
// already have.
@@ -4918,7 +4919,6 @@ bool Sema::CheckTemplateTypeArgument(
// is not a type.
SourceRange SR = AL.getSourceRange();
Diag(SR.getBegin(), diag::err_template_arg_must_be_type) << SR;
- NoteTemplateParameterLocation(*Param);
return true;
}
@@ -5208,8 +5208,8 @@ bool Sema::CheckTemplateArgument(NamedDecl *Param, TemplateArgumentLoc &ArgLoc,
CheckTemplateArgumentInfo &CTAI,
CheckTemplateArgumentKind CTAK) {
// Check template type parameters.
- if (TemplateTypeParmDecl *TTP = dyn_cast<TemplateTypeParmDecl>(Param))
- return CheckTemplateTypeArgument(TTP, ArgLoc, CTAI.SugaredConverted,
+ if (isa<TemplateTypeParmDecl>(Param))
+ return CheckTemplateTypeArgument(ArgLoc, CTAI.SugaredConverted,
CTAI.CanonicalConverted);
const TemplateArgument &Arg = ArgLoc.getArgument();
@@ -5354,8 +5354,6 @@ bool Sema::CheckTemplateArgument(NamedDecl *Param, TemplateArgumentLoc &ArgLoc,
// therefore cannot be a non-type template argument.
Diag(ArgLoc.getLocation(), diag::err_template_arg_must_be_expr)
<< ArgLoc.getSourceRange();
- NoteTemplateParameterLocation(*Param);
-
return true;
case TemplateArgument::Type: {
@@ -5375,7 +5373,6 @@ bool Sema::CheckTemplateArgument(NamedDecl *Param, TemplateArgumentLoc &ArgLoc,
Diag(SR.getBegin(), diag::err_template_arg_nontype_ambig) << SR << T;
else
Diag(SR.getBegin(), diag::err_template_arg_must_be_expr) << SR;
- NoteTemplateParameterLocation(*Param);
return true;
}
@@ -5466,11 +5463,11 @@ bool Sema::CheckTemplateArgument(NamedDecl *Param, TemplateArgumentLoc &ArgLoc,
}
/// Diagnose a missing template argument.
-template<typename TemplateParmDecl>
+template <typename TemplateParmDecl>
static bool diagnoseMissingArgument(Sema &S, SourceLocation Loc,
- TemplateDecl *TD,
- const TemplateParmDecl *D,
- TemplateArgumentListInfo &Args) {
+ TemplateDecl *TD, const TemplateParmDecl *D,
+ TemplateArgumentListInfo &Args,
+ bool MatchingTTP) {
// Dig out the most recent declaration of the template parameter; there may be
// declarations of the template that are more recent than TD.
D = cast<TemplateParmDecl>(cast<TemplateDecl>(TD->getMostRecentDecl())
@@ -5488,16 +5485,12 @@ static bool diagnoseMissingArgument(Sema &S, SourceLocation Loc,
return true;
}
+ SourceLocation DiagLoc = Args.getRAngleLoc();
// FIXME: If there's a more recent default argument that *is* visible,
// diagnose that it was declared too late.
-
- TemplateParameterList *Params = TD->getTemplateParameters();
-
- S.Diag(Loc, diag::err_template_arg_list_different_arity)
- << /*not enough args*/0
- << (int)S.getTemplateNameKindForDiagnostics(TemplateName(TD))
- << TD;
- S.NoteTemplateLocation(*TD, Params->getSourceRange());
+ S.Diag(DiagLoc.isValid() ? DiagLoc : Loc,
+ MatchingTTP ? diag::err_template_template_param_missing_param
+ : diag::err_template_param_missing_arg);
return true;
}
@@ -5536,6 +5529,8 @@ bool Sema::CheckTemplateArgumentList(
Param = ParamBegin;
Param != ParamEnd;
/* increment in loop */) {
+ CheckTemplateParameterRAII CTP1(*this, *Param);
+
if (size_t ParamIdx = Param - ParamBegin;
DefaultArgs && ParamIdx >= DefaultArgs.StartPos) {
// All written arguments should have been consumed by this point.
@@ -5572,11 +5567,9 @@ bool Sema::CheckTemplateArgumentList(
continue;
} else if (ArgIdx == NumArgs && !PartialTemplateArgs) {
// Not enough arguments for this parameter pack.
- Diag(TemplateLoc, diag::err_template_arg_list_different_arity)
- << /*not enough args*/0
- << (int)getTemplateNameKindForDiagnostics(TemplateName(Template))
- << Template;
- NoteTemplateLocation(*Template, Params->getSourceRange());
+ Diag(RAngleLoc, CTAI.MatchingTTP
+ ? diag::err_template_template_param_missing_param
+ : diag::err_template_param_missing_arg);
return true;
}
}
@@ -5589,8 +5582,10 @@ bool Sema::CheckTemplateArgumentList(
if (ArgIsExpansion && CTAI.MatchingTTP) {
SmallVector<TemplateArgument, 4> Args(ParamEnd - Param);
+ CTP1.Clear(); // Will continue processing parameters below.
for (TemplateParameterList::iterator First = Param; Param != ParamEnd;
++Param) {
+ CheckTemplateParameterRAII CTP2(*this, *Param);
TemplateArgument &Arg = Args[Param - First];
Arg = ArgLoc.getArgument();
if (!(*Param)->isTemplateParameterPack() ||
@@ -5631,7 +5626,6 @@ bool Sema::CheckTemplateArgumentList(
diag::err_template_expansion_into_fixed_list)
<< (isa<ConceptDecl>(Template) ? 1 : 0)
<< ArgLoc.getSourceRange();
- NoteTemplateParameterLocation(**Param);
return true;
}
}
@@ -5738,14 +5732,14 @@ bool Sema::CheckTemplateArgumentList(
if (!HasDefaultArg) {
if (TemplateTypeParmDecl *TTP = dyn_cast<TemplateTypeParmDecl>(*Param))
return diagnoseMissingArgument(*this, TemplateLoc, Template, TTP,
- NewArgs);
+ NewArgs, CTAI.MatchingTTP);
if (NonTypeTemplateParmDecl *NTTP =
dyn_cast<NonTypeTemplateParmDecl>(*Param))
return diagnoseMissingArgument(*this, TemplateLoc, Template, NTTP,
- NewArgs);
+ NewArgs, CTAI.MatchingTTP);
return diagnoseMissingArgument(*this, TemplateLoc, Template,
cast<TemplateTemplateParmDecl>(*Param),
- NewArgs);
+ NewArgs, CTAI.MatchingTTP);
}
return true;
}
@@ -5801,8 +5795,7 @@ bool Sema::CheckTemplateArgumentList(
// If we have any leftover arguments, then there were too many arguments.
// Complain and fail.
if (ArgIdx < NumArgs) {
- Diag(TemplateLoc, diag::err_template_arg_list_different_arity)
- << /*too many args*/1
+ Diag(TemplateLoc, diag::err_template_too_many_args)
<< (int)getTemplateNameKindForDiagnostics(TemplateName(Template))
<< Template
<< SourceRange(NewArgs[ArgIdx].getLocation(), NewArgs.getRAngleLoc());
@@ -6227,8 +6220,6 @@ isNullPointerValueTemplateArgument(Sema &S, NonTypeTemplateParmDecl *Param,
<< Arg->getType() << Arg->getSourceRange();
for (unsigned I = 0, N = Notes.size(); I != N; ++I)
S.Diag(Notes[I].first, Notes[I].second);
-
- S.NoteTemplateParameterLocation(*Param);
return NPV_Error;
}
@@ -6253,8 +6244,7 @@ isNullPointerValueTemplateArgument(Sema &S, NonTypeTemplateParmDecl *Param,
// The types didn't match, but we ...
[truncated]
|
|
@llvm/pr-subscribers-clang Author: Matheus Izvekov (mizvekov) ChangesInstead of manually adding a note pointing to the relevant template parameter to every relevant error, which is very easy to miss, this patch adds a new instantiation context note, so that this can work using RAII magic. This fixes a bunch of places where these notes were missing, and is more future-proof. Some diagnostics are reworked to make better use of this note:
It's likely this will add the note to some diagnostics where the parameter is not super relevant, but this can be reworked with time and the decrease in maintenance burden makes up for it. This bypasses the templight dumper for the new context entry, as the tests are very hard to update. This depends on #125453, which is needed to avoid losing the context note for errors occuring during template argument deduction. Patch is 166.58 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/126088.diff 95 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index dfce173d414ebb2..7e20954c65e67b4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -149,6 +149,16 @@ Bug Fixes to C++ Support
- Clang now prints the correct instantiation context for diagnostics suppressed
by template argument deduction.
+Improvements to C++ diagnostics
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+- Clang now more consistently adds a note pointing to the relevant template
+ parameter. Some diagnostics are reworded to better take advantage of this.
+- Template Template Parameter diagnostics now stop referring to template
+ parameters as template arguments, in some circumstances, better hiding
+ from the users template template parameter partial ordering arcana.
+
+
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 7b3b932c482baa2..6ab26fb25d8238f 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5290,10 +5290,14 @@ def err_template_missing_args : Error<
"%select{class template|function template|variable template|alias template|"
"template template parameter|concept|template}0 %1 requires template "
"arguments">;
-def err_template_arg_list_different_arity : Error<
- "%select{too few|too many}0 template arguments for "
+def err_template_param_missing_arg : Error<
+ "missing template argument for template parameter">;
+def err_template_template_param_missing_param : Error<
+ "missing template parameter to bind to template template parameter">;
+def err_template_too_many_args : Error<
+ "too many template arguments for "
"%select{class template|function template|variable template|alias template|"
- "template template parameter|concept|template}1 %2">;
+ "template template parameter|concept|template}0 %1">;
def note_template_decl_here : Note<"template is declared here">;
def note_template_decl_external : Note<
"template declaration from hidden source: %0">;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index e39930c1a450343..93b1094616d6a1a 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -11781,7 +11781,7 @@ class Sema final : public SemaBase {
bool *ConstraintsNotSatisfied = nullptr);
bool CheckTemplateTypeArgument(
- TemplateTypeParmDecl *Param, TemplateArgumentLoc &Arg,
+ TemplateArgumentLoc &Arg,
SmallVectorImpl<TemplateArgument> &SugaredConverted,
SmallVectorImpl<TemplateArgument> &CanonicalConverted);
@@ -11817,9 +11817,10 @@ class Sema final : public SemaBase {
bool PartialOrdering,
bool *StrictPackMatch);
+ SmallString<128> toTerseString(const NamedDecl &D) const;
+
void NoteTemplateLocation(const NamedDecl &Decl,
std::optional<SourceRange> ParamRange = {});
- void NoteTemplateParameterLocation(const NamedDecl &Decl);
/// Given a non-type template argument that refers to a
/// declaration and the type of its corresponding non-type template
@@ -12821,6 +12822,9 @@ class Sema final : public SemaBase {
/// We are performing partial ordering for template template parameters.
PartialOrderingTTP,
+
+ /// Checking a Template Parameter
+ CheckTemplateParameter,
} Kind;
/// Was the enclosing context a non-instantiation SFINAE context?
@@ -13048,6 +13052,11 @@ class Sema final : public SemaBase {
PartialOrderingTTP, TemplateDecl *PArg,
SourceRange InstantiationRange = SourceRange());
+ struct CheckTemplateParameter {};
+ /// \brief Note that we are checking a template parameter.
+ InstantiatingTemplate(Sema &SemaRef, CheckTemplateParameter,
+ NamedDecl *Param);
+
/// Note that we have finished instantiating this template.
void Clear();
@@ -13081,6 +13090,13 @@ class Sema final : public SemaBase {
InstantiatingTemplate &operator=(const InstantiatingTemplate &) = delete;
};
+ /// For any diagnostics which occur within its scope, adds a context note
+ /// pointing to the declaration of the template parameter.
+ struct CheckTemplateParameterRAII : InstantiatingTemplate {
+ CheckTemplateParameterRAII(Sema &S, NamedDecl *Param)
+ : InstantiatingTemplate(S, CheckTemplateParameter(), Param) {}
+ };
+
bool SubstTemplateArgument(const TemplateArgumentLoc &Input,
const MultiLevelTemplateArgumentList &TemplateArgs,
TemplateArgumentLoc &Output,
diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp
index 1ea4a2e9e88cf59..60e103e643e273b 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -403,7 +403,8 @@ class DefaultTemplateInstCallback : public TemplateInstantiationCallback {
}
private:
- static std::string toString(CodeSynthesisContext::SynthesisKind Kind) {
+ static std::optional<std::string>
+ toString(CodeSynthesisContext::SynthesisKind Kind) {
switch (Kind) {
case CodeSynthesisContext::TemplateInstantiation:
return "TemplateInstantiation";
@@ -461,8 +462,10 @@ class DefaultTemplateInstCallback : public TemplateInstantiationCallback {
return "TypeAliasTemplateInstantiation";
case CodeSynthesisContext::PartialOrderingTTP:
return "PartialOrderingTTP";
+ case CodeSynthesisContext::CheckTemplateParameter:
+ return std::nullopt;
}
- return "";
+ return std::nullopt;
}
template <bool BeginInstantiation>
@@ -470,12 +473,14 @@ class DefaultTemplateInstCallback : public TemplateInstantiationCallback {
const CodeSynthesisContext &Inst) {
std::string YAML;
{
+ std::optional<TemplightEntry> Entry =
+ getTemplightEntry<BeginInstantiation>(TheSema, Inst);
+ if (!Entry)
+ return;
llvm::raw_string_ostream OS(YAML);
llvm::yaml::Output YO(OS);
- TemplightEntry Entry =
- getTemplightEntry<BeginInstantiation>(TheSema, Inst);
llvm::yaml::EmptyContext Context;
- llvm::yaml::yamlize(YO, Entry, true, Context);
+ llvm::yaml::yamlize(YO, *Entry, true, Context);
}
Out << "---" << YAML << "\n";
}
@@ -555,10 +560,13 @@ class DefaultTemplateInstCallback : public TemplateInstantiationCallback {
}
template <bool BeginInstantiation>
- static TemplightEntry getTemplightEntry(const Sema &TheSema,
- const CodeSynthesisContext &Inst) {
+ static std::optional<TemplightEntry>
+ getTemplightEntry(const Sema &TheSema, const CodeSynthesisContext &Inst) {
TemplightEntry Entry;
- Entry.Kind = toString(Inst.Kind);
+ std::optional<std::string> Kind = toString(Inst.Kind);
+ if (!Kind)
+ return std::nullopt;
+ Entry.Kind = *Kind;
Entry.Event = BeginInstantiation ? "Begin" : "End";
llvm::raw_string_ostream OS(Entry.Name);
printEntryName(TheSema, Inst.Entity, OS);
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index f206cd57eca898b..9281cfc950232f1 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -7246,7 +7246,7 @@ static void CheckCXX98CompatAccessibleCopy(Sema &S,
void InitializationSequence::PrintInitLocationNote(Sema &S,
const InitializedEntity &Entity) {
- if (Entity.isParamOrTemplateParamKind() && Entity.getDecl()) {
+ if (Entity.isParameterKind() && Entity.getDecl()) {
if (Entity.getDecl()->getLocation().isInvalid())
return;
@@ -7255,9 +7255,8 @@ void InitializationSequence::PrintInitLocationNote(Sema &S,
<< Entity.getDecl()->getDeclName();
else
S.Diag(Entity.getDecl()->getLocation(), diag::note_parameter_here);
- }
- else if (Entity.getKind() == InitializedEntity::EK_RelatedResult &&
- Entity.getMethodDecl())
+ } else if (Entity.getKind() == InitializedEntity::EK_RelatedResult &&
+ Entity.getMethodDecl())
S.Diag(Entity.getMethodDecl()->getLocation(),
diag::note_method_return_type_change)
<< Entity.getMethodDecl()->getDeclName();
diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp
index ceb32ee15dfa39c..4d278bbc67d281a 100644
--- a/clang/lib/Sema/SemaLambda.cpp
+++ b/clang/lib/Sema/SemaLambda.cpp
@@ -1506,14 +1506,13 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro,
TemplateParameterList *TemplateParams =
getGenericLambdaTemplateParameterList(LSI, *this);
if (TemplateParams) {
- for (const auto *TP : TemplateParams->asArray()) {
+ for (auto *TP : TemplateParams->asArray()) {
if (!TP->getIdentifier())
continue;
+ CheckTemplateParameterRAII CTP(*this, TP);
for (const auto &Capture : Intro.Captures) {
- if (Capture.Id == TP->getIdentifier()) {
+ if (Capture.Id == TP->getIdentifier())
Diag(Capture.Loc, diag::err_template_param_shadow) << Capture.Id;
- NoteTemplateParameterLocation(*TP);
- }
}
}
}
diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index 0f5b7426e743e05..71d9add9d8c098f 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -1586,9 +1586,13 @@ llvm::DenseSet<Module*> &Sema::getLookupModules() {
unsigned N = CodeSynthesisContexts.size();
for (unsigned I = CodeSynthesisContextLookupModules.size();
I != N; ++I) {
- Module *M = CodeSynthesisContexts[I].Entity ?
- getDefiningModule(*this, CodeSynthesisContexts[I].Entity) :
- nullptr;
+ auto &Ctx = CodeSynthesisContexts[I];
+ // FIXME: Are there any other context kinds that shouldn't be looked at
+ // here?
+ if (Ctx.Kind == CodeSynthesisContext::PartialOrderingTTP ||
+ Ctx.Kind == CodeSynthesisContext::CheckTemplateParameter)
+ continue;
+ Module *M = Ctx.Entity ? getDefiningModule(*this, Ctx.Entity) : nullptr;
if (M && !LookupModulesCache.insert(M).second)
M = nullptr;
CodeSynthesisContextLookupModules.push_back(M);
@@ -3709,7 +3713,8 @@ Sema::LookupLiteralOperator(Scope *S, LookupResult &R,
TemplateParameterList *Params = FD->getTemplateParameters();
if (Params->size() == 1) {
IsTemplate = true;
- if (!Params->getParam(0)->isTemplateParameterPack() && !StringLit) {
+ NamedDecl *Param = Params->getParam(0);
+ if (!Param->isTemplateParameterPack() && !StringLit) {
// Implied but not stated: user-defined integer and floating literals
// only ever use numeric literal operator templates, not templates
// taking a parameter of class type.
@@ -3722,6 +3727,7 @@ Sema::LookupLiteralOperator(Scope *S, LookupResult &R,
if (StringLit) {
SFINAETrap Trap(*this);
CheckTemplateArgumentInfo CTAI;
+ CheckTemplateParameterRAII CTP(*this, Param);
TemplateArgumentLoc Arg(TemplateArgument(StringLit), StringLit);
if (CheckTemplateArgument(
Params->getParam(0), Arg, FD, R.getNameLoc(), R.getNameLoc(),
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 9e68972b33f0a03..78093c66c1a53aa 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -869,9 +869,11 @@ void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl,
? diag::ext_template_param_shadow
: (SupportedForCompatibility ? diag::ext_compat_template_param_shadow
: diag::err_template_param_shadow);
- const auto *ND = cast<NamedDecl>(PrevDecl);
+ auto *ND = cast<NamedDecl>(PrevDecl);
+ CheckTemplateParameterRAII CTP(*this, ND);
+ // FIXME: Don't put the name in the diagnostic, unless there is no source
+ // location.
Diag(Loc, DiagId) << ND->getDeclName();
- NoteTemplateParameterLocation(*ND);
}
TemplateDecl *Sema::AdjustDeclIfTemplate(Decl *&D) {
@@ -4824,7 +4826,7 @@ TemplateNameKind Sema::ActOnTemplateName(Scope *S,
}
bool Sema::CheckTemplateTypeArgument(
- TemplateTypeParmDecl *Param, TemplateArgumentLoc &AL,
+ TemplateArgumentLoc &AL,
SmallVectorImpl<TemplateArgument> &SugaredConverted,
SmallVectorImpl<TemplateArgument> &CanonicalConverted) {
const TemplateArgument &Arg = AL.getArgument();
@@ -4880,7 +4882,6 @@ bool Sema::CheckTemplateTypeArgument(
? diag::ext_ms_template_type_arg_missing_typename
: diag::err_template_arg_must_be_type_suggest)
<< FixItHint::CreateInsertion(Loc, "typename ");
- NoteTemplateParameterLocation(*Param);
// Recover by synthesizing a type using the location information that we
// already have.
@@ -4918,7 +4919,6 @@ bool Sema::CheckTemplateTypeArgument(
// is not a type.
SourceRange SR = AL.getSourceRange();
Diag(SR.getBegin(), diag::err_template_arg_must_be_type) << SR;
- NoteTemplateParameterLocation(*Param);
return true;
}
@@ -5208,8 +5208,8 @@ bool Sema::CheckTemplateArgument(NamedDecl *Param, TemplateArgumentLoc &ArgLoc,
CheckTemplateArgumentInfo &CTAI,
CheckTemplateArgumentKind CTAK) {
// Check template type parameters.
- if (TemplateTypeParmDecl *TTP = dyn_cast<TemplateTypeParmDecl>(Param))
- return CheckTemplateTypeArgument(TTP, ArgLoc, CTAI.SugaredConverted,
+ if (isa<TemplateTypeParmDecl>(Param))
+ return CheckTemplateTypeArgument(ArgLoc, CTAI.SugaredConverted,
CTAI.CanonicalConverted);
const TemplateArgument &Arg = ArgLoc.getArgument();
@@ -5354,8 +5354,6 @@ bool Sema::CheckTemplateArgument(NamedDecl *Param, TemplateArgumentLoc &ArgLoc,
// therefore cannot be a non-type template argument.
Diag(ArgLoc.getLocation(), diag::err_template_arg_must_be_expr)
<< ArgLoc.getSourceRange();
- NoteTemplateParameterLocation(*Param);
-
return true;
case TemplateArgument::Type: {
@@ -5375,7 +5373,6 @@ bool Sema::CheckTemplateArgument(NamedDecl *Param, TemplateArgumentLoc &ArgLoc,
Diag(SR.getBegin(), diag::err_template_arg_nontype_ambig) << SR << T;
else
Diag(SR.getBegin(), diag::err_template_arg_must_be_expr) << SR;
- NoteTemplateParameterLocation(*Param);
return true;
}
@@ -5466,11 +5463,11 @@ bool Sema::CheckTemplateArgument(NamedDecl *Param, TemplateArgumentLoc &ArgLoc,
}
/// Diagnose a missing template argument.
-template<typename TemplateParmDecl>
+template <typename TemplateParmDecl>
static bool diagnoseMissingArgument(Sema &S, SourceLocation Loc,
- TemplateDecl *TD,
- const TemplateParmDecl *D,
- TemplateArgumentListInfo &Args) {
+ TemplateDecl *TD, const TemplateParmDecl *D,
+ TemplateArgumentListInfo &Args,
+ bool MatchingTTP) {
// Dig out the most recent declaration of the template parameter; there may be
// declarations of the template that are more recent than TD.
D = cast<TemplateParmDecl>(cast<TemplateDecl>(TD->getMostRecentDecl())
@@ -5488,16 +5485,12 @@ static bool diagnoseMissingArgument(Sema &S, SourceLocation Loc,
return true;
}
+ SourceLocation DiagLoc = Args.getRAngleLoc();
// FIXME: If there's a more recent default argument that *is* visible,
// diagnose that it was declared too late.
-
- TemplateParameterList *Params = TD->getTemplateParameters();
-
- S.Diag(Loc, diag::err_template_arg_list_different_arity)
- << /*not enough args*/0
- << (int)S.getTemplateNameKindForDiagnostics(TemplateName(TD))
- << TD;
- S.NoteTemplateLocation(*TD, Params->getSourceRange());
+ S.Diag(DiagLoc.isValid() ? DiagLoc : Loc,
+ MatchingTTP ? diag::err_template_template_param_missing_param
+ : diag::err_template_param_missing_arg);
return true;
}
@@ -5536,6 +5529,8 @@ bool Sema::CheckTemplateArgumentList(
Param = ParamBegin;
Param != ParamEnd;
/* increment in loop */) {
+ CheckTemplateParameterRAII CTP1(*this, *Param);
+
if (size_t ParamIdx = Param - ParamBegin;
DefaultArgs && ParamIdx >= DefaultArgs.StartPos) {
// All written arguments should have been consumed by this point.
@@ -5572,11 +5567,9 @@ bool Sema::CheckTemplateArgumentList(
continue;
} else if (ArgIdx == NumArgs && !PartialTemplateArgs) {
// Not enough arguments for this parameter pack.
- Diag(TemplateLoc, diag::err_template_arg_list_different_arity)
- << /*not enough args*/0
- << (int)getTemplateNameKindForDiagnostics(TemplateName(Template))
- << Template;
- NoteTemplateLocation(*Template, Params->getSourceRange());
+ Diag(RAngleLoc, CTAI.MatchingTTP
+ ? diag::err_template_template_param_missing_param
+ : diag::err_template_param_missing_arg);
return true;
}
}
@@ -5589,8 +5582,10 @@ bool Sema::CheckTemplateArgumentList(
if (ArgIsExpansion && CTAI.MatchingTTP) {
SmallVector<TemplateArgument, 4> Args(ParamEnd - Param);
+ CTP1.Clear(); // Will continue processing parameters below.
for (TemplateParameterList::iterator First = Param; Param != ParamEnd;
++Param) {
+ CheckTemplateParameterRAII CTP2(*this, *Param);
TemplateArgument &Arg = Args[Param - First];
Arg = ArgLoc.getArgument();
if (!(*Param)->isTemplateParameterPack() ||
@@ -5631,7 +5626,6 @@ bool Sema::CheckTemplateArgumentList(
diag::err_template_expansion_into_fixed_list)
<< (isa<ConceptDecl>(Template) ? 1 : 0)
<< ArgLoc.getSourceRange();
- NoteTemplateParameterLocation(**Param);
return true;
}
}
@@ -5738,14 +5732,14 @@ bool Sema::CheckTemplateArgumentList(
if (!HasDefaultArg) {
if (TemplateTypeParmDecl *TTP = dyn_cast<TemplateTypeParmDecl>(*Param))
return diagnoseMissingArgument(*this, TemplateLoc, Template, TTP,
- NewArgs);
+ NewArgs, CTAI.MatchingTTP);
if (NonTypeTemplateParmDecl *NTTP =
dyn_cast<NonTypeTemplateParmDecl>(*Param))
return diagnoseMissingArgument(*this, TemplateLoc, Template, NTTP,
- NewArgs);
+ NewArgs, CTAI.MatchingTTP);
return diagnoseMissingArgument(*this, TemplateLoc, Template,
cast<TemplateTemplateParmDecl>(*Param),
- NewArgs);
+ NewArgs, CTAI.MatchingTTP);
}
return true;
}
@@ -5801,8 +5795,7 @@ bool Sema::CheckTemplateArgumentList(
// If we have any leftover arguments, then there were too many arguments.
// Complain and fail.
if (ArgIdx < NumArgs) {
- Diag(TemplateLoc, diag::err_template_arg_list_different_arity)
- << /*too many args*/1
+ Diag(TemplateLoc, diag::err_template_too_many_args)
<< (int)getTemplateNameKindForDiagnostics(TemplateName(Template))
<< Template
<< SourceRange(NewArgs[ArgIdx].getLocation(), NewArgs.getRAngleLoc());
@@ -6227,8 +6220,6 @@ isNullPointerValueTemplateArgument(Sema &S, NonTypeTemplateParmDecl *Param,
<< Arg->getType() << Arg->getSourceRange();
for (unsigned I = 0, N = Notes.size(); I != N; ++I)
S.Diag(Notes[I].first, Notes[I].second);
-
- S.NoteTemplateParameterLocation(*Param);
return NPV_Error;
}
@@ -6253,8 +6244,7 @@ isNullPointerValueTemplateArgument(Sema &S, NonTypeTemplateParmDecl *Param,
// The types didn't match, but we ...
[truncated]
|
|
(Did you mean |
Oops, yes of course :) |
mizvekov
changed the title
mizvekov
changed the title
Endilll
reviewed
Feb 6, 2025
mizvekov
force-pushed
the
users/mizvekov/clang-print-context-stack-diags-supressed-by-deduction
branch
2 times, most recently
from
f7e2964 to
228d525
Compare
erichkeane
approved these changes
Feb 19, 2025
There was a problem hiding this comment.
A few quick comments, else the source changes LGTM. Note that @endill's suggestion to use 'bookmarks' for notes (or something like that) are good ones that I agree with.
mizvekov
force-pushed
the
users/mizvekov/clang-check-template-parameter-eval-context
branch
6 times, most recently
from
2af6244 to
ae8b23b
Compare
cor3ntin
reviewed
Feb 20, 2025
Comment on lines
5299
to
5291
| def err_template_template_param_missing_param : Error< | ||
| "missing template parameter to bind to template template parameter">; |
There was a problem hiding this comment.
The formulation of this is a bit weird.
Maybe: "template template argument 'foo' incompatible with 'bar': template parameter list arity mismatch"
There was a problem hiding this comment.
We can point to the template template parameter and argument source locations, so we shouldn't try to spell them in the diagnostic.
The 'parameter list arity mismatch' is an exact match oriented kind of diagnostic, which becomes confusing with P0522, where you can default the parameter to make this work.
Here we can be more helpful and say how the argument is incompatible with the template template parameter: we will point out in a later note which template parameter doesn't have a corresponding parameter in the template template parameter.
I think the ideal here is for the error to be formulated in such a way as to mention the template parameter which we will note later, so that it doesn't look like the note came out of the blue.
For a test case like:
template <class T1, class T2> struct A;
template <template <class T3> class TT> struct B;
template struct B<A>;I made another change to clarify this error. Here is how this prints now with this patch:
test.cc:2:29: error: no template parameter in this template template parameter corresponds to non-defaulted template parameter of argument template
2 | template <template <class T3> class TT> struct B;
| ^
test.cc:1:27: note: template parameter is declared here
1 | template <class T1, class T2> struct A;
| ~~~~~~^~
test.cc:3:19: note: template template argument is incompatible with its corresponding template template parameter
3 | template struct B<A>;
| ^
test.cc:2:37: note: template parameter is declared here
2 | template <template <class T3> class TT> struct B;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~^~
1 error generated.
There was a problem hiding this comment.
For comparison, before this patch, the diagnostic for this example is bad:
test.cc:3:19: error: too few template arguments for class template 'A'
3 | template struct B<A>;
| ^
test.cc:3:19: note: template template argument has different template parameters than its corresponding template template parameter
test.cc:2:37: note: previous template template parameter is here
2 | template <template <class T3> class TT> struct B;
| ~~~~~~~~~~~~~~~~~~~ ^
test.cc:1:38: note: template is declared here
1 | template <class T1, class T2> struct A;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
There was a problem hiding this comment.
From the peanut gallery: as someone who stares at error messages from building 100s of projects in a distribution a lot, I'd find it a lot more intuitive if the error message started with
test.cc:3:19: error: <something here>
3 | template struct B<A>;
| ^
test.cc:3:<line>: note: <elaboration>
where ideally the elaboration goes like "template A from <here> doesn't satisfy template template for B from <there>".
There was a problem hiding this comment.
Yeah, the diagnostic should have started with the second note as an error, but this is a different problem, and is out of scope for this patch.
There are too many possible things that can be diagnosed when matching template template parameters, and most of these diagnostics are implemented for problems which occur outside of that context.
My thinking is that we will eventually need to be able to turn errors and warnings into notes, and perform transformations on the sequence of diagnostics depending on the context where they occur.
Base automatically changed from
users/mizvekov/clang-print-context-stack-diags-supressed-by-deduction
to
main
February 20, 2025 11:50
mizvekov
force-pushed
the
users/mizvekov/clang-check-template-parameter-eval-context
branch
from
ae8b23b to
e094d95
Compare
mizvekov
force-pushed
the
users/mizvekov/clang-check-template-parameter-eval-context
branch
3 times, most recently
from
339f296 to
59048ff
Compare
Instead of manually adding a note pointing to the relevant template parameter to every relevant error, which is very easy to miss, this patch adds a new instantiation context note, so that this can work using RAII magic. This fixes a bunch of places where these notes were missing, and is more future-proof. Some diagnostics are reworked to make better use of this note: - Errors about missing template arguments now refer to the parameter which is missing an argument. - Template Template parameter mismatches now refer to template parameters as parameters instead of arguments. It's likely this will add the note to some diagnostics where the parameter is not super relevant, but this can be reworked with time and the decrease in maintenance burden makes up for it. This bypasses the templight dumper for the new context entry, as the tests are very hard to update. This depends on #125453, which is needed to avoid losing the context note for errors occuring during template argument deduction.
mizvekov
force-pushed
the
users/mizvekov/clang-check-template-parameter-eval-context
branch
from
59048ff to
e30bd8a
Compare
|
Ping. I'll merge tomorrow if there are no further objections. |
SGTM. I haven't been paying SUPER close attention, but have been keeping an eye on this at least a little, and have no objections. |
mizvekov
deleted the
users/mizvekov/clang-check-template-parameter-eval-context
branch
|
This causes a large compile-time regression for C++ code: https://llvm-compile-time-tracker.com/compare_clang.php?from=37aad2c1196ea3de242d855cfe38bc25a65d6f5e&to=a24523ac8dc07f3478311a5969184b922b520395&stat=instructions%3Au&sortBy=interestingness It adds 1-3% overhead per file during the stage2 build. Please revert this change until the regression can be resolved. |
nikic
added a commit
that referenced
this pull request
Mar 10, 2025
…plate parameters (#126088)" This reverts commit a24523a. This is causing significant compile-time regressions for C++ code, see: #126088 (comment)
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Mar 10, 2025
…hecking template parameters (#126088)" This reverts commit a24523a. This is causing significant compile-time regressions for C++ code, see: llvm/llvm-project#126088 (comment)
|
@nikic thanks for the report, I missed your message, only saw when Corentin pinged. Are you sure that's correct? I had run this patch through llvm-compile-time-tracker before merging, and it had shown practically no impact. |
|
Yes, reverting the change did recover the regression. Did you test exactly the change that was landed? It's also possible that it interacted with something that landed in the meantime... |
|
It's not exactly the same patch, had more changes on top, but the meat with the expected impact was the same. Anyway, thanks for the revert, will investigate. |
|
Correction, I was thinking of a different PR. I did not test this PR on compile-time-tracker, as we didn't anticipate an impact. |
mizvekov
added a commit
that referenced
this pull request
Mar 10, 2025
…rameters Instead of manually adding a note pointing to the relevant template parameter to every relevant error, which is very easy to miss, this patch adds a new instantiation context note, so that this can work using RAII magic. This fixes a bunch of places where these notes were missing, and is more future-proof. Some diagnostics are reworked to make better use of this note: - Errors about missing template arguments now refer to the parameter which is missing an argument. - Template Template parameter mismatches now refer to template parameters as parameters instead of arguments. It's likely this will add the note to some diagnostics where the parameter is not super relevant, but this can be reworked with time and the decrease in maintenance burden makes up for it. This bypasses the templight dumper for the new context entry, as the tests are very hard to update. This depends on #125453, which is needed to avoid losing the context note for errors occuring during template argument deduction. Original PR: #126088
jph-13
pushed a commit
to jph-13/llvm-project
that referenced
this pull request
Mar 21, 2025
…rameters (llvm#126088) Instead of manually adding a note pointing to the relevant template parameter to every relevant error, which is very easy to miss, this patch adds a new instantiation context note, so that this can work using RAII magic. This fixes a bunch of places where these notes were missing, and is more future-proof. Some diagnostics are reworked to make better use of this note: - Errors about missing template arguments now refer to the parameter which is missing an argument. - Template Template parameter mismatches now refer to template parameters as parameters instead of arguments. It's likely this will add the note to some diagnostics where the parameter is not super relevant, but this can be reworked with time and the decrease in maintenance burden makes up for it. This bypasses the templight dumper for the new context entry, as the tests are very hard to update. This depends on llvm#125453, which is needed to avoid losing the context note for errors occuring during template argument deduction.
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.
Instead of manually adding a note pointing to the relevant template parameter to every relevant error, which is very easy to miss, this patch adds a new instantiation context note, so that this can work using RAII magic.
This fixes a bunch of places where these notes were missing, and is more future-proof.
Some diagnostics are reworked to make better use of this note:
It's likely this will add the note to some diagnostics where the parameter is not super relevant, but this can be reworked with time and the decrease in maintenance burden makes up for it.
This bypasses the templight dumper for the new context entry, as the tests are very hard to update.
This depends on #125453, which is needed to avoid losing the context note for errors occuring during template argument deduction.