Skip to content

Commit e094d95

Browse files
mizvekovmizvekov
committed
[clang] Implement evaluation context for checking template parameters
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 0948fc8 commit e094d95

File tree

95 files changed

+574
-458
lines changed

Some content is hidden

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

95 files changed

+574
-458
lines changed

‎clang/docs/ReleaseNotes.rst‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,16 @@ Bug Fixes to C++ Support
174174
direct-list-initialized from an array is corrected to direct-initialization.
175175
- Clang no longer crashes when a coroutine is declared ``[[noreturn]]``. (#GH127327)
176176

177+
Improvements to C++ diagnostics
178+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
179+
180+
- Clang now more consistently adds a note pointing to the relevant template
181+
parameter. Some diagnostics are reworded to better take advantage of this.
182+
- Template Template Parameter diagnostics now stop referring to template
183+
parameters as template arguments, in some circumstances, better hiding
184+
from the users template template parameter partial ordering arcana.
185+
186+
177187
Bug Fixes to AST Handling
178188
^^^^^^^^^^^^^^^^^^^^^^^^^
179189
- Fixed type checking when a statement expression ends in an l-value of atomic type. (#GH106576)

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5285,10 +5285,14 @@ def err_template_missing_args : Error<
52855285
"%select{class template|function template|variable template|alias template|"
52865286
"template template parameter|concept|template}0 %1 requires template "
52875287
"arguments">;
5288-
def err_template_arg_list_different_arity : Error<
5289-
"%select{too few|too many}0 template arguments for "
5288+
def err_template_param_missing_arg : Error<
5289+
"missing template argument for template parameter">;
5290+
def err_template_template_param_missing_param : Error<
5291+
"missing template parameter to bind to template template parameter">;
5292+
def err_template_too_many_args : Error<
5293+
"too many template arguments for "
52905294
"%select{class template|function template|variable template|alias template|"
5291-
"template template parameter|concept|template}1 %2">;
5295+
"template template parameter|concept|template}0 %1">;
52925296
def note_template_decl_here : Note<"template is declared here">;
52935297
def note_template_decl_external : Note<
52945298
"template declaration from hidden source: %0">;

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

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11782,7 +11782,7 @@ class Sema final : public SemaBase {
1178211782
bool *ConstraintsNotSatisfied = nullptr);
1178311783

1178411784
bool CheckTemplateTypeArgument(
11785-
TemplateTypeParmDecl *Param, TemplateArgumentLoc &Arg,
11785+
TemplateArgumentLoc &Arg,
1178611786
SmallVectorImpl<TemplateArgument> &SugaredConverted,
1178711787
SmallVectorImpl<TemplateArgument> &CanonicalConverted);
1178811788

@@ -11818,9 +11818,13 @@ class Sema final : public SemaBase {
1181811818
bool PartialOrdering,
1181911819
bool *StrictPackMatch);
1182011820

11821+
/// Print the given named declaration to a string,
11822+
/// using the current PrintingPolicy, except that
11823+
/// TerseOutput will always be set.
11824+
SmallString<128> toTerseString(const NamedDecl &D) const;
11825+
1182111826
void NoteTemplateLocation(const NamedDecl &Decl,
1182211827
std::optional<SourceRange> ParamRange = {});
11823-
void NoteTemplateParameterLocation(const NamedDecl &Decl);
1182411828

1182511829
/// Given a non-type template argument that refers to a
1182611830
/// declaration and the type of its corresponding non-type template
@@ -12822,6 +12826,9 @@ class Sema final : public SemaBase {
1282212826

1282312827
/// We are performing partial ordering for template template parameters.
1282412828
PartialOrderingTTP,
12829+
12830+
/// Checking a Template Parameter
12831+
CheckTemplateParameter,
1282512832
} Kind;
1282612833

1282712834
/// Was the enclosing context a non-instantiation SFINAE context?
@@ -13049,6 +13056,11 @@ class Sema final : public SemaBase {
1304913056
PartialOrderingTTP, TemplateDecl *PArg,
1305013057
SourceRange InstantiationRange = SourceRange());
1305113058

13059+
struct CheckTemplateParameter {};
13060+
/// \brief Note that we are checking a template parameter.
13061+
InstantiatingTemplate(Sema &SemaRef, CheckTemplateParameter,
13062+
NamedDecl *Param);
13063+
1305213064
/// Note that we have finished instantiating this template.
1305313065
void Clear();
1305413066

@@ -13082,6 +13094,13 @@ class Sema final : public SemaBase {
1308213094
InstantiatingTemplate &operator=(const InstantiatingTemplate &) = delete;
1308313095
};
1308413096

13097+
/// For any diagnostics which occur within its scope, adds a context note
13098+
/// pointing to the declaration of the template parameter.
13099+
struct CheckTemplateParameterRAII : InstantiatingTemplate {
13100+
CheckTemplateParameterRAII(Sema &S, NamedDecl *Param)
13101+
: InstantiatingTemplate(S, CheckTemplateParameter(), Param) {}
13102+
};
13103+
1308513104
bool SubstTemplateArgument(const TemplateArgumentLoc &Input,
1308613105
const MultiLevelTemplateArgumentList &TemplateArgs,
1308713106
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
@@ -7268,7 +7268,7 @@ static void CheckCXX98CompatAccessibleCopy(Sema &S,
72687268

72697269
void InitializationSequence::PrintInitLocationNote(Sema &S,
72707270
const InitializedEntity &Entity) {
7271-
if (Entity.isParamOrTemplateParamKind() && Entity.getDecl()) {
7271+
if (Entity.isParameterKind() && Entity.getDecl()) {
72727272
if (Entity.getDecl()->getLocation().isInvalid())
72737273
return;
72747274

@@ -7277,9 +7277,8 @@ void InitializationSequence::PrintInitLocationNote(Sema &S,
72777277
<< Entity.getDecl()->getDeclName();
72787278
else
72797279
S.Diag(Entity.getDecl()->getLocation(), diag::note_parameter_here);
7280-
}
7281-
else if (Entity.getKind() == InitializedEntity::EK_RelatedResult &&
7282-
Entity.getMethodDecl())
7280+
} else if (Entity.getKind() == InitializedEntity::EK_RelatedResult &&
7281+
Entity.getMethodDecl())
72837282
S.Diag(Entity.getMethodDecl()->getLocation(),
72847283
diag::note_method_return_type_change)
72857284
<< 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
@@ -1586,9 +1586,13 @@ llvm::DenseSet<Module*> &Sema::getLookupModules() {
15861586
unsigned N = CodeSynthesisContexts.size();
15871587
for (unsigned I = CodeSynthesisContextLookupModules.size();
15881588
I != N; ++I) {
1589-
Module *M = CodeSynthesisContexts[I].Entity ?
1590-
getDefiningModule(*this, CodeSynthesisContexts[I].Entity) :
1591-
nullptr;
1589+
auto &Ctx = CodeSynthesisContexts[I];
1590+
// FIXME: Are there any other context kinds that shouldn't be looked at
1591+
// here?
1592+
if (Ctx.Kind == CodeSynthesisContext::PartialOrderingTTP ||
1593+
Ctx.Kind == CodeSynthesisContext::CheckTemplateParameter)
1594+
continue;
1595+
Module *M = Ctx.Entity ? getDefiningModule(*this, Ctx.Entity) : nullptr;
15921596
if (M && !LookupModulesCache.insert(M).second)
15931597
M = nullptr;
15941598
CodeSynthesisContextLookupModules.push_back(M);
@@ -3709,7 +3713,8 @@ Sema::LookupLiteralOperator(Scope *S, LookupResult &R,
37093713
TemplateParameterList *Params = FD->getTemplateParameters();
37103714
if (Params->size() == 1) {
37113715
IsTemplate = true;
3712-
if (!Params->getParam(0)->isTemplateParameterPack() && !StringLit) {
3716+
NamedDecl *Param = Params->getParam(0);
3717+
if (!Param->isTemplateParameterPack() && !StringLit) {
37133718
// Implied but not stated: user-defined integer and floating literals
37143719
// only ever use numeric literal operator templates, not templates
37153720
// taking a parameter of class type.
@@ -3722,6 +3727,7 @@ Sema::LookupLiteralOperator(Scope *S, LookupResult &R,
37223727
if (StringLit) {
37233728
SFINAETrap Trap(*this);
37243729
CheckTemplateArgumentInfo CTAI;
3730+
CheckTemplateParameterRAII CTP(*this, Param);
37253731
TemplateArgumentLoc Arg(TemplateArgument(StringLit), StringLit);
37263732
if (CheckTemplateArgument(
37273733
Params->getParam(0), Arg, FD, R.getNameLoc(), R.getNameLoc(),

0 commit comments

Comments
 (0)