[PATCH] D140423: [WIP][clang] Add PrintingPolicy callback for identifying default template arguments

[Michael Buch via Phabricator via cfe-commits]

Michael137 created this revision.
Michael137 added reviewers: aprantl, dblaikie.
Herald added a project: All.
Michael137 added a comment.
Michael137 updated this revision to Diff 484341.
Michael137 added a reviewer: labath.
Michael137 edited the summary of this revision.
Michael137 added a reviewer: aaron.ballman.
Michael137 updated this revision to Diff 484434.
Michael137 updated this revision to Diff 484439.
Michael137 published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

An example of how this would be used from LLDB is: https://reviews.llvm.org/D140145


Michael137 added a comment.

- Remove debug print in test


aprantl added a comment.

Overall this seems to have a pretty small surface area, so I'd be fine with including this.


Michael137 added a comment.

@aaron.ballman we're trying to fix printing of C++ types with default template arguments in LLDB. Currently DWARF doesn't have the necessary info to construct a `ClassTemplateDecl` with generic parameters, which means the logic for suppressing them in the `TypePrinter` doesn't quite work.

This patch outlines of one approach we considered. We add a hook into the TypePrinter to allow external AST sources to answer the question "is template argument X defaulted?".

Another alternative would be to have a `TemplateArgument::IsDefaulted` which gets set in LLDB and read from the TypePrinter.

Do you think either of these approaches would be fine for inclusion?


Michael137 added a comment.

- Fix test. Move variable into loop


Michael137 added a comment.

- Rebase


Michael137 added a comment.

putting into review queue to hear people's opinion on something along these lines



================
Comment at: clang/include/clang/AST/PrettyPrinter.h:52
+
+  enum class TriState : int { kYes, kNo, kUnknown };
+
----------------
The TrisState is needed because LLDB has two AST sources:
1. DWARF
2. clang modules

When we import a type from a clang module we would never have consulted DWARF about whether a parameter was defaulted. So instead of more complex bookkeeping it seemed simpler to say "if we've never seen this decl when parsing DWARF, I can't say anything about the default-ness of the arguments"


================
Comment at: clang/lib/AST/TypePrinter.cpp:2095
+
+  while (!Args.empty()) {
+    if (Callbacks != nullptr)
----------------
maybe this body becomes slightly less oddly indented when converting it to a range-based for with a counter?


This patch adds a hook into the `TypePrinter` to allow clients
decide whether a template argument corresponds to the default
parameter.

**Motivation**

DWARF encodes information about template instantiations, but
not about the generic definition of a template. If an argument
in a template instantiation corresponds to the default parameter
of the generic template then DWARF will attach a `DW_AT_default_value`
to that argument. LLDB uses the Clang TypePrinter for
displaying/formatting C++ types but currently can't give Clang the `ClassTemplateDecl`
that it expects. Since LLDB does know whether a particular instantiation has
default arguments, this patch allows LLDB (and other clients with external AST sources)
to help Clang in identifying those default arguments.

**Alternatives**

1. Encode information about generic template definitions in DWARF. It's unclear how long it would take to get this proposed and implemented and whether something like this would work in the first place. If we could construct `ClassTemplateDecl`s with the correct generic parameters this patch wouldn't be required.

2. Add something like a `bool IsDefaulted` somewhere in Clang, e.g., in `TemplateArgument` and consult it from the `TypePrinter`. This would be much simpler but requires adding a field on one of the Clang types


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140423

Files:
  clang/include/clang/AST/PrettyPrinter.h
  clang/include/clang/AST/Type.h
  clang/lib/AST/TypePrinter.cpp
  clang/unittests/AST/TypePrinterTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D140423.484439.patch
Type: text/x-patch
Size: 8283 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221222/58d37a33/attachment.bin>