Reland: [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.

Original PR: #126088

Author: mizvekov

clang/docs/ReleaseNotes.rst

@@ -433,6 +433,16 @@ Bug Fixes to C++ Support
   object type. (#GH151531)
 - Suppress ``-Wdouble-promotion`` when explicitly asked for with C++ list initialization (#GH33409).
 
+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
 ^^^^^^^^^^^^^^^^^^^^^^^^^
 - Fix incorrect name qualifiers applied to alias CTAD. (#GH136624)

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -5358,19 +5358,14 @@ def err_template_linkage : Error<"templates must have C++ linkage">;
 def err_template_typedef : Error<"a typedef cannot be a template">;
 def err_template_unnamed_class : Error<
   "cannot declare a class template with no name">;
-def err_template_param_list_different_arity : Error<
-  "%select{too few|too many}0 template parameters in template "
-  "%select{|template parameter }1redeclaration">;
-def note_template_param_list_different_arity : Note<
-  "%select{too few|too many}0 template parameters in template template "
-  "argument">;
+def err_template_param_list_different_arity
+    : Error<"%select{too few|too many}0 template parameters in template "
+            "%select{|template parameter }1redeclaration">;
 def note_template_prev_declaration : Note<
   "previous template %select{declaration|template parameter}0 is here">;
-def err_template_param_different_kind : Error<
-  "template parameter has a different kind in template "
-  "%select{|template parameter }0redeclaration">;
-def note_template_param_different_kind : Note<
-  "template parameter has a different kind in template argument">;
+def err_template_param_different_kind
+    : Error<"template parameter has a different kind in template "
+            "%select{|template parameter }0redeclaration">;
 
 def err_invalid_decl_specifier_in_nontype_parm : Error<
   "invalid declaration specifier in template non-type parameter">;
@@ -5379,8 +5374,6 @@ def err_template_nontype_parm_different_type : Error<
   "template non-type parameter has a different type %0 in template "
   "%select{|template parameter }1redeclaration">;
 
-def note_template_nontype_parm_different_type : Note<
-  "template non-type parameter has a different type %0 in template argument">;
 def note_template_nontype_parm_prev_declaration : Note<
   "previous non-type template parameter with type %0 is here">;
 def err_template_nontype_parm_bad_type : Error<
@@ -5454,10 +5447,17 @@ 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 "
-  "%select{class template|function template|variable template|alias template|"
-  "template template parameter|concept|template}1 %2">;
+def err_template_param_missing_arg
+    : Error<"missing template argument for template parameter">;
+def err_template_template_param_missing_param
+    : Error<"no template parameter in this template template parameter "
+            "corresponds to non-defaulted template parameter of argument "
+            "template">;
+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}0 %1">;
 def note_template_decl_here : Note<"template is declared here">;
 def note_template_decl_external : Note<
   "template declaration from hidden source: %0">;
@@ -5500,12 +5500,9 @@ def err_template_arg_not_valid_template
 def note_template_arg_refers_to_template_here
     : Note<"template argument refers to a %select{function template|class "
            "template|variable template|concept}0 %1, here">;
-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 note_template_arg_template_params_mismatch
+    : Note<"template template argument is incompatible with its "
+           "corresponding template template parameter">;
 def err_non_deduced_mismatch : Error<
   "could not match %diff{$ against $|types}0,1">;
 def err_inconsistent_deduction : Error<
@@ -6050,14 +6047,11 @@ def err_template_param_pack_default_arg : Error<
 def err_template_param_pack_must_be_last_template_parameter : Error<
   "template parameter pack must be the last template parameter">;
 
-def err_template_parameter_pack_non_pack : Error<
-  "%select{template type|non-type template|template template}0 parameter"
-  "%select{| pack}1 conflicts with previous %select{template type|"
-  "non-type template|template template}0 parameter%select{ pack|}1">;
-def note_template_parameter_pack_non_pack : Note<
-  "%select{template type|non-type template|template template}0 parameter"
-  "%select{| pack}1 does not match %select{template type|non-type template"
-  "|template template}0 parameter%select{ pack|}1 in template argument">;
+def err_template_parameter_pack_non_pack
+    : Error<"%select{template type|non-type template|template template}0 "
+            "parameter"
+            "%select{| pack}1 conflicts with previous %select{template type|"
+            "non-type template|template template}0 parameter%select{ pack|}1">;
 def note_template_parameter_pack_here : Note<
   "previous %select{template type|non-type template|template template}0 "
   "parameter%select{| pack}1 declared here">;

clang/include/clang/Sema/Sema.h

@@ -12026,7 +12026,7 @@ class Sema final : public SemaBase {
                                  bool *ConstraintsNotSatisfied = nullptr);
 
   bool CheckTemplateTypeArgument(
-      TemplateTypeParmDecl *Param, TemplateArgumentLoc &Arg,
+      TemplateArgumentLoc &Arg,
       SmallVectorImpl<TemplateArgument> &SugaredConverted,
       SmallVectorImpl<TemplateArgument> &CanonicalConverted);
 
@@ -12066,9 +12066,13 @@ class Sema final : public SemaBase {
                                                TemplateTemplateParmDecl *Param,
                                                const TemplateArgumentLoc &Arg);
 
+  /// Print the given named declaration to a string,
+  /// using the current PrintingPolicy, except that
+  /// TerseOutput will always be set.
+  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
@@ -12183,15 +12187,13 @@ class Sema final : public SemaBase {
   bool TemplateParameterListsAreEqual(
       const TemplateCompareNewDeclInfo &NewInstFrom, TemplateParameterList *New,
       const NamedDecl *OldInstFrom, TemplateParameterList *Old, bool Complain,
-      TemplateParameterListEqualKind Kind,
-      SourceLocation TemplateArgLoc = SourceLocation());
+      TemplateParameterListEqualKind Kind);
 
-  bool TemplateParameterListsAreEqual(
-      TemplateParameterList *New, TemplateParameterList *Old, bool Complain,
-      TemplateParameterListEqualKind Kind,
-      SourceLocation TemplateArgLoc = SourceLocation()) {
+  bool TemplateParameterListsAreEqual(TemplateParameterList *New,
+                                      TemplateParameterList *Old, bool Complain,
+                                      TemplateParameterListEqualKind Kind) {
     return TemplateParameterListsAreEqual(nullptr, New, nullptr, Old, Complain,
-                                          Kind, TemplateArgLoc);
+                                          Kind);
   }
 
   /// Check whether a template can be declared within this scope.
@@ -13088,6 +13090,11 @@ class Sema final : public SemaBase {
 
       /// We are performing partial ordering for template template parameters.
       PartialOrderingTTP,
+
+      /// We are Checking a Template Parameter, so for any diagnostics which
+      /// occur in this scope, we will add a context note which points to this
+      /// template parameter.
+      CheckTemplateParameter,
     } Kind;
 
     /// Was the enclosing context a non-instantiation SFINAE context?
@@ -13318,6 +13325,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();
 
@@ -13351,6 +13363,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,

clang/lib/Frontend/FrontendActions.cpp

@@ -418,7 +418,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";
@@ -476,21 +477,25 @@ class DefaultTemplateInstCallback : public TemplateInstantiationCallback {
       return "TypeAliasTemplateInstantiation";
     case CodeSynthesisContext::PartialOrderingTTP:
       return "PartialOrderingTTP";
+    case CodeSynthesisContext::CheckTemplateParameter:
+      return std::nullopt;
     }
-    return "";
+    return std::nullopt;
   }
 
   template <bool BeginInstantiation>
   static void displayTemplightEntry(llvm::raw_ostream &Out, const Sema &TheSema,
                                     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";
   }
@@ -570,10 +575,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);

clang/lib/Sema/SemaInit.cpp

@@ -7342,7 +7342,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;
 
@@ -7351,9 +7351,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();

clang/lib/Sema/SemaLambda.cpp

@@ -1525,14 +1525,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);
-        }
       }
     }
   }

clang/lib/Sema/SemaLookup.cpp

@@ -1613,9 +1613,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);
@@ -3738,7 +3742,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.
@@ -3751,6 +3756,7 @@ Sema::LookupLiteralOperator(Scope *S, LookupResult &R,
         if (StringLit) {
           SFINAETrap Trap(*this);
           CheckTemplateArgumentInfo CTAI;
+          CheckTemplateParameterRAII CTP(*this, Param);
           TemplateArgumentLoc Arg(
               TemplateArgument(StringLit, /*IsCanonical=*/false), StringLit);
           if (CheckTemplateArgument(