Skip to content

Commit a24523a

Browse files
mizvekovmizvekov
authored
[clang] Implement instantiation context note for checking template parameters (#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 #125453, which is needed to avoid losing the context note for errors occuring during template argument deduction.
1 parent 37aad2c commit a24523a

File tree

97 files changed

+648
-575
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

97 files changed

+648
-575
lines changed

‎clang/docs/ReleaseNotes.rst

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,16 @@ Bug Fixes to C++ Support
282282
- Clang no longer crashes when a coroutine is declared ``[[noreturn]]``. (#GH127327)
283283
- Clang now uses the parameter location for abbreviated function templates in ``extern "C"``. (#GH46386)
284284

285+
Improvements to C++ diagnostics
286+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
287+
288+
- Clang now more consistently adds a note pointing to the relevant template
289+
parameter. Some diagnostics are reworded to better take advantage of this.
290+
- Template Template Parameter diagnostics now stop referring to template
291+
parameters as template arguments, in some circumstances, better hiding
292+
from the users template template parameter partial ordering arcana.
293+
294+
285295
Bug Fixes to AST Handling
286296
^^^^^^^^^^^^^^^^^^^^^^^^^
287297
- Fixed type checking when a statement expression ends in an l-value of atomic type. (#GH106576)

‎clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5210,16 +5210,11 @@ def err_template_unnamed_class : Error<
52105210
def err_template_param_list_different_arity : Error<
52115211
"%select{too few|too many}0 template parameters in template "
52125212
"%select{|template parameter }1redeclaration">;
5213-
def note_template_param_list_different_arity : Note<
5214-
"%select{too few|too many}0 template parameters in template template "
5215-
"argument">;
52165213
def note_template_prev_declaration : Note<
52175214
"previous template %select{declaration|template parameter}0 is here">;
52185215
def err_template_param_different_kind : Error<
52195216
"template parameter has a different kind in template "
52205217
"%select{|template parameter }0redeclaration">;
5221-
def note_template_param_different_kind : Note<
5222-
"template parameter has a different kind in template argument">;
52235218

52245219
def err_invalid_decl_specifier_in_nontype_parm : Error<
52255220
"invalid declaration specifier in template non-type parameter">;
@@ -5228,8 +5223,6 @@ def err_template_nontype_parm_different_type : Error<
52285223
"template non-type parameter has a different type %0 in template "
52295224
"%select{|template parameter }1redeclaration">;
52305225

5231-
def note_template_nontype_parm_different_type : Note<
5232-
"template non-type parameter has a different type %0 in template argument">;
52335226
def note_template_nontype_parm_prev_declaration : Note<
52345227
"previous non-type template parameter with type %0 is here">;
52355228
def err_template_nontype_parm_bad_type : Error<
@@ -5320,10 +5313,15 @@ def err_template_missing_args : Error<
53205313
"%select{class template|function template|variable template|alias template|"
53215314
"template template parameter|concept|template}0 %1 requires template "
53225315
"arguments">;
5323-
def err_template_arg_list_different_arity : Error<
5324-
"%select{too few|too many}0 template arguments for "
5316+
def err_template_param_missing_arg : Error<
5317+
"missing template argument for template parameter">;
5318+
def err_template_template_param_missing_param : Error<
5319+
"no template parameter in this template template parameter "
5320+
"corresponds to non-defaulted template parameter of argument template">;
5321+
def err_template_too_many_args : Error<
5322+
"too many template arguments for "
53255323
"%select{class template|function template|variable template|alias template|"
5326-
"template template parameter|concept|template}1 %2">;
5324+
"template template parameter|concept|template}0 %1">;
53275325
def note_template_decl_here : Note<"template is declared here">;
53285326
def note_template_decl_external : Note<
53295327
"template declaration from hidden source: %0">;
@@ -5361,11 +5359,8 @@ def err_template_arg_not_valid_template : Error<
53615359
"template parameter">;
53625360
def note_template_arg_refers_here_func : Note<
53635361
"template argument refers to function template %0, here">;
5364-
def err_template_arg_template_params_mismatch : Error<
5365-
"template template argument has different template parameters than its "
5366-
"corresponding template template parameter">;
53675362
def note_template_arg_template_params_mismatch : Note<
5368-
"template template argument has different template parameters than its "
5363+
"template template argument is incompatible with its "
53695364
"corresponding template template parameter">;
53705365
def err_non_deduced_mismatch : Error<
53715366
"could not match %diff{$ against $|types}0,1">;
@@ -5927,10 +5922,6 @@ def err_template_parameter_pack_non_pack : Error<
59275922
"%select{template type|non-type template|template template}0 parameter"
59285923
"%select{| pack}1 conflicts with previous %select{template type|"
59295924
"non-type template|template template}0 parameter%select{ pack|}1">;
5930-
def note_template_parameter_pack_non_pack : Note<
5931-
"%select{template type|non-type template|template template}0 parameter"
5932-
"%select{| pack}1 does not match %select{template type|non-type template"
5933-
"|template template}0 parameter%select{ pack|}1 in template argument">;
59345925
def note_template_parameter_pack_here : Note<
59355926
"previous %select{template type|non-type template|template template}0 "
59365927
"parameter%select{| pack}1 declared here">;

‎clang/include/clang/Sema/Sema.h

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11827,7 +11827,7 @@ class Sema final : public SemaBase {
1182711827
bool *ConstraintsNotSatisfied = nullptr);
1182811828

1182911829
bool CheckTemplateTypeArgument(
11830-
TemplateTypeParmDecl *Param, TemplateArgumentLoc &Arg,
11830+
TemplateArgumentLoc &Arg,
1183111831
SmallVectorImpl<TemplateArgument> &SugaredConverted,
1183211832
SmallVectorImpl<TemplateArgument> &CanonicalConverted);
1183311833

@@ -11863,9 +11863,13 @@ class Sema final : public SemaBase {
1186311863
bool PartialOrdering,
1186411864
bool *StrictPackMatch);
1186511865

11866+
/// Print the given named declaration to a string,
11867+
/// using the current PrintingPolicy, except that
11868+
/// TerseOutput will always be set.
11869+
SmallString<128> toTerseString(const NamedDecl &D) const;
11870+
1186611871
void NoteTemplateLocation(const NamedDecl &Decl,
1186711872
std::optional<SourceRange> ParamRange = {});
11868-
void NoteTemplateParameterLocation(const NamedDecl &Decl);
1186911873

1187011874
/// Given a non-type template argument that refers to a
1187111875
/// declaration and the type of its corresponding non-type template
@@ -11980,15 +11984,13 @@ class Sema final : public SemaBase {
1198011984
bool TemplateParameterListsAreEqual(
1198111985
const TemplateCompareNewDeclInfo &NewInstFrom, TemplateParameterList *New,
1198211986
const NamedDecl *OldInstFrom, TemplateParameterList *Old, bool Complain,
11983-
TemplateParameterListEqualKind Kind,
11984-
SourceLocation TemplateArgLoc = SourceLocation());
11987+
TemplateParameterListEqualKind Kind);
1198511988

11986-
bool TemplateParameterListsAreEqual(
11987-
TemplateParameterList *New, TemplateParameterList *Old, bool Complain,
11988-
TemplateParameterListEqualKind Kind,
11989-
SourceLocation TemplateArgLoc = SourceLocation()) {
11989+
bool TemplateParameterListsAreEqual(TemplateParameterList *New,
11990+
TemplateParameterList *Old, bool Complain,
11991+
TemplateParameterListEqualKind Kind) {
1199011992
return TemplateParameterListsAreEqual(nullptr, New, nullptr, Old, Complain,
11991-
Kind, TemplateArgLoc);
11993+
Kind);
1199211994
}
1199311995

1199411996
/// Check whether a template can be declared within this scope.
@@ -12868,6 +12870,11 @@ class Sema final : public SemaBase {
1286812870

1286912871
/// We are performing partial ordering for template template parameters.
1287012872
PartialOrderingTTP,
12873+
12874+
/// We are Checking a Template Parameter, so for any diagnostics which
12875+
/// occur in this scope, we will add a context note which points to this
12876+
/// template parameter.
12877+
CheckTemplateParameter,
1287112878
} Kind;
1287212879

1287312880
/// Was the enclosing context a non-instantiation SFINAE context?
@@ -13095,6 +13102,11 @@ class Sema final : public SemaBase {
1309513102
PartialOrderingTTP, TemplateDecl *PArg,
1309613103
SourceRange InstantiationRange = SourceRange());
1309713104

13105+
struct CheckTemplateParameter {};
13106+
/// \brief Note that we are checking a template parameter.
13107+
InstantiatingTemplate(Sema &SemaRef, CheckTemplateParameter,
13108+
NamedDecl *Param);
13109+
1309813110
/// Note that we have finished instantiating this template.
1309913111
void Clear();
1310013112

@@ -13128,6 +13140,13 @@ class Sema final : public SemaBase {
1312813140
InstantiatingTemplate &operator=(const InstantiatingTemplate &) = delete;
1312913141
};
1313013142

13143+
/// For any diagnostics which occur within its scope, adds a context note
13144+
/// pointing to the declaration of the template parameter.
13145+
struct CheckTemplateParameterRAII : InstantiatingTemplate {
13146+
CheckTemplateParameterRAII(Sema &S, NamedDecl *Param)
13147+
: InstantiatingTemplate(S, CheckTemplateParameter(), Param) {}
13148+
};
13149+
1313113150
bool SubstTemplateArgument(const TemplateArgumentLoc &Input,
1313213151
const MultiLevelTemplateArgumentList &TemplateArgs,
1313313152
TemplateArgumentLoc &Output,

‎clang/lib/Frontend/FrontendActions.cpp

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,8 @@ class DefaultTemplateInstCallback : public TemplateInstantiationCallback {
403403
}
404404

405405
private:
406-
static std::string toString(CodeSynthesisContext::SynthesisKind Kind) {
406+
static std::optional<std::string>
407+
toString(CodeSynthesisContext::SynthesisKind Kind) {
407408
switch (Kind) {
408409
case CodeSynthesisContext::TemplateInstantiation:
409410
return "TemplateInstantiation";
@@ -461,21 +462,25 @@ class DefaultTemplateInstCallback : public TemplateInstantiationCallback {
461462
return "TypeAliasTemplateInstantiation";
462463
case CodeSynthesisContext::PartialOrderingTTP:
463464
return "PartialOrderingTTP";
465+
case CodeSynthesisContext::CheckTemplateParameter:
466+
return std::nullopt;
464467
}
465-
return "";
468+
return std::nullopt;
466469
}
467470

468471
template <bool BeginInstantiation>
469472
static void displayTemplightEntry(llvm::raw_ostream &Out, const Sema &TheSema,
470473
const CodeSynthesisContext &Inst) {
471474
std::string YAML;
472475
{
476+
std::optional<TemplightEntry> Entry =
477+
getTemplightEntry<BeginInstantiation>(TheSema, Inst);
478+
if (!Entry)
479+
return;
473480
llvm::raw_string_ostream OS(YAML);
474481
llvm::yaml::Output YO(OS);
475-
TemplightEntry Entry =
476-
getTemplightEntry<BeginInstantiation>(TheSema, Inst);
477482
llvm::yaml::EmptyContext Context;
478-
llvm::yaml::yamlize(YO, Entry, true, Context);
483+
llvm::yaml::yamlize(YO, *Entry, true, Context);
479484
}
480485
Out << "---" << YAML << "\n";
481486
}
@@ -555,10 +560,13 @@ class DefaultTemplateInstCallback : public TemplateInstantiationCallback {
555560
}
556561

557562
template <bool BeginInstantiation>
558-
static TemplightEntry getTemplightEntry(const Sema &TheSema,
559-
const CodeSynthesisContext &Inst) {
563+
static std::optional<TemplightEntry>
564+
getTemplightEntry(const Sema &TheSema, const CodeSynthesisContext &Inst) {
560565
TemplightEntry Entry;
561-
Entry.Kind = toString(Inst.Kind);
566+
std::optional<std::string> Kind = toString(Inst.Kind);
567+
if (!Kind)
568+
return std::nullopt;
569+
Entry.Kind = *Kind;
562570
Entry.Event = BeginInstantiation ? "Begin" : "End";
563571
llvm::raw_string_ostream OS(Entry.Name);
564572
printEntryName(TheSema, Inst.Entity, OS);

‎clang/lib/Sema/SemaInit.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7271,7 +7271,7 @@ static void CheckCXX98CompatAccessibleCopy(Sema &S,
72717271

72727272
void InitializationSequence::PrintInitLocationNote(Sema &S,
72737273
const InitializedEntity &Entity) {
7274-
if (Entity.isParamOrTemplateParamKind() && Entity.getDecl()) {
7274+
if (Entity.isParameterKind() && Entity.getDecl()) {
72757275
if (Entity.getDecl()->getLocation().isInvalid())
72767276
return;
72777277

@@ -7280,9 +7280,8 @@ void InitializationSequence::PrintInitLocationNote(Sema &S,
72807280
<< Entity.getDecl()->getDeclName();
72817281
else
72827282
S.Diag(Entity.getDecl()->getLocation(), diag::note_parameter_here);
7283-
}
7284-
else if (Entity.getKind() == InitializedEntity::EK_RelatedResult &&
7285-
Entity.getMethodDecl())
7283+
} else if (Entity.getKind() == InitializedEntity::EK_RelatedResult &&
7284+
Entity.getMethodDecl())
72867285
S.Diag(Entity.getMethodDecl()->getLocation(),
72877286
diag::note_method_return_type_change)
72887287
<< Entity.getMethodDecl()->getDeclName();

‎clang/lib/Sema/SemaLambda.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1506,14 +1506,13 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro,
15061506
TemplateParameterList *TemplateParams =
15071507
getGenericLambdaTemplateParameterList(LSI, *this);
15081508
if (TemplateParams) {
1509-
for (const auto *TP : TemplateParams->asArray()) {
1509+
for (auto *TP : TemplateParams->asArray()) {
15101510
if (!TP->getIdentifier())
15111511
continue;
1512+
CheckTemplateParameterRAII CTP(*this, TP);
15121513
for (const auto &Capture : Intro.Captures) {
1513-
if (Capture.Id == TP->getIdentifier()) {
1514+
if (Capture.Id == TP->getIdentifier())
15141515
Diag(Capture.Loc, diag::err_template_param_shadow) << Capture.Id;
1515-
NoteTemplateParameterLocation(*TP);
1516-
}
15171516
}
15181517
}
15191518
}

‎clang/lib/Sema/SemaLookup.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1580,9 +1580,13 @@ llvm::DenseSet<Module*> &Sema::getLookupModules() {
15801580
unsigned N = CodeSynthesisContexts.size();
15811581
for (unsigned I = CodeSynthesisContextLookupModules.size();
15821582
I != N; ++I) {
1583-
Module *M = CodeSynthesisContexts[I].Entity ?
1584-
getDefiningModule(*this, CodeSynthesisContexts[I].Entity) :
1585-
nullptr;
1583+
auto &Ctx = CodeSynthesisContexts[I];
1584+
// FIXME: Are there any other context kinds that shouldn't be looked at
1585+
// here?
1586+
if (Ctx.Kind == CodeSynthesisContext::PartialOrderingTTP ||
1587+
Ctx.Kind == CodeSynthesisContext::CheckTemplateParameter)
1588+
continue;
1589+
Module *M = Ctx.Entity ? getDefiningModule(*this, Ctx.Entity) : nullptr;
15861590
if (M && !LookupModulesCache.insert(M).second)
15871591
M = nullptr;
15881592
CodeSynthesisContextLookupModules.push_back(M);
@@ -3703,7 +3707,8 @@ Sema::LookupLiteralOperator(Scope *S, LookupResult &R,
37033707
TemplateParameterList *Params = FD->getTemplateParameters();
37043708
if (Params->size() == 1) {
37053709
IsTemplate = true;
3706-
if (!Params->getParam(0)->isTemplateParameterPack() && !StringLit) {
3710+
NamedDecl *Param = Params->getParam(0);
3711+
if (!Param->isTemplateParameterPack() && !StringLit) {
37073712
// Implied but not stated: user-defined integer and floating literals
37083713
// only ever use numeric literal operator templates, not templates
37093714
// taking a parameter of class type.
@@ -3716,6 +3721,7 @@ Sema::LookupLiteralOperator(Scope *S, LookupResult &R,
37163721
if (StringLit) {
37173722
SFINAETrap Trap(*this);
37183723
CheckTemplateArgumentInfo CTAI;
3724+
CheckTemplateParameterRAII CTP(*this, Param);
37193725
TemplateArgumentLoc Arg(TemplateArgument(StringLit), StringLit);
37203726
if (CheckTemplateArgument(
37213727
Params->getParam(0), Arg, FD, R.getNameLoc(), R.getNameLoc(),

0 commit comments

Comments
 (0)