[clang] Implement instantiation context note for checking template parameters

clang/docs/ReleaseNotes.rst

@@ -236,6 +236,16 @@ Bug Fixes to C++ Support
   direct-list-initialized from an array is corrected to direct-initialization.
 - Clang no longer crashes when a coroutine is declared ``[[noreturn]]``. (#GH127327)
 
+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
 ^^^^^^^^^^^^^^^^^^^^^^^^^
 - Fixed type checking when a statement expression ends in an l-value of atomic type. (#GH106576)

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -5193,16 +5193,11 @@ def err_template_unnamed_class : Error<
 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 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_invalid_decl_specifier_in_nontype_parm : Error<
   "invalid declaration specifier in template non-type parameter">;
@@ -5211,8 +5206,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<
@@ -5303,10 +5296,15 @@ 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<
+  "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}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">;
@@ -5344,11 +5342,8 @@ def err_template_arg_not_valid_template : Error<
   "template parameter">;
 def note_template_arg_refers_here_func : Note<
   "template argument refers to function template %0, 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 "
+  "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">;
@@ -5910,10 +5905,6 @@ 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 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

@@ -11820,7 +11820,7 @@ class Sema final : public SemaBase {
                                  bool *ConstraintsNotSatisfied = nullptr);
 
   bool CheckTemplateTypeArgument(
-      TemplateTypeParmDecl *Param, TemplateArgumentLoc &Arg,
+      TemplateArgumentLoc &Arg,
       SmallVectorImpl<TemplateArgument> &SugaredConverted,
       SmallVectorImpl<TemplateArgument> &CanonicalConverted);
 
@@ -11856,9 +11856,13 @@ class Sema final : public SemaBase {
                                      bool PartialOrdering,
                                      bool *StrictPackMatch);
 
+  /// 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
@@ -11973,15 +11977,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.
@@ -12860,6 +12862,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?
@@ -13087,6 +13094,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();
 
@@ -13120,6 +13132,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

@@ -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,21 +462,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";
   }
@@ -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);

clang/lib/Sema/SemaInit.cpp

@@ -7269,7 +7269,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;
 
@@ -7278,9 +7278,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

@@ -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);
-        }
       }
     }
   }

clang/lib/Sema/SemaLookup.cpp

@@ -1580,9 +1580,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);
@@ -3703,7 +3707,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.
@@ -3716,6 +3721,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(),