[PATCH] D77598: Integral template argument suffix and cast printing

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Came across this while trying to do "simplified template names" - producing template names in DWARF without template parameter lists as part of the textual name, then rebuilding that name from the structural representation of template parameters in DWARF (DW_TAG_template_*_parameter, etc). The roundtripping is implemented to ensure that the simplified names are non-lossy - that all the data is still available through the structural representation. (some names are harder or not currently possible to rebuild)

The selective use of suffixes, especially contextually (overloading) seems like something I'll probably want to avoid entirely for DWARF to keep the names consistent across different contexts - but I am also just a bit confused about some things I'm seeing with this change.

For instance, it looks like https://github.com/llvm/llvm-project/blob/fcdefc8575866a36e80e91024b876937ae6a9965/clang/lib/AST/Decl.cpp#L2900 doesn't pass the list of template parameters, so function template names always get suffixes on their integer parameters.

Whereas the equivalent calls for printing class template specialization names here: https://github.com/llvm/llvm-project/blob/fcdefc8575866a36e80e91024b876937ae6a9965/clang/lib/AST/DeclTemplate.cpp#L949-L959 - is that just a minor bug/missing functionality?

I'm testing a fix for that:

  diff --git clang/lib/AST/Decl.cpp clang/lib/AST/Decl.cpp
  index 60ca8633224b..11cf068d2c4c 100644
  --- clang/lib/AST/Decl.cpp
  +++ clang/lib/AST/Decl.cpp
  @@ -2897,7 +2897,7 @@ void FunctionDecl::getNameForDiagnostic(
     NamedDecl::getNameForDiagnostic(OS, Policy, Qualified);
     const TemplateArgumentList *TemplateArgs = getTemplateSpecializationArgs();
     if (TemplateArgs)
  -    printTemplateArgumentList(OS, TemplateArgs->asArray(), Policy);
  +    printTemplateArgumentList(OS, TemplateArgs->asArray(), Policy, getPrimaryTemplate()->getTemplateParameters());
   }
   
   bool FunctionDecl::isVariadic() const {

I've sent out a patch with ^ applied/cleanups applied: D77598 <https://reviews.llvm.org/D77598>

But for the debug info, I'm considering changing debug info printing of names to use this:

  OS << ND->getDeclName();
  printTemplateArgumentList(OS, Args->Args, PP);

Without passing the template parameters/explicitly printing the name separately from the template arguments - but alternatively we could add a PrintingPolicy to address this issue instead. (though I have some reasons to prefer this separated approach - because I want to separate them explicitly for the simplified template name work, for some reasons).

Oh, and here's another case missing its template parameter list: https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/NestedNameSpecifier.cpp#L310-L328 - that one probably points to the DWARF usage needing a more complete way to opt out of this - since there might be nested name printing and such that would need to be canonicalized (honestly it points to flaws in my idea even without nested name specifiers -parameters of parameters would still not be handled correctly even if I split up the naming as suggested in the patch above).

@rsmith - any ideas/thoughts/perspectives here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77598/new/

https://reviews.llvm.org/D77598