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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D77598#2064193 <https://reviews.llvm.org/D77598#2064193>, @rsmith wrote:

> In D77598#1990197 <https://reviews.llvm.org/D77598#1990197>, @rsmith wrote:
>
> > Is is feasible to check whether the corresponding template parameter either has a deduced type or is a parameter of a function template? If not, we don't need to clarify the type of the template argument and could leave off the suffix to shorten the printed argument.
>
>
> Ping on this comment. If possible, I'd prefer for us to not produce suffixes and casts in the case where there's no need to clarify the type of the template argument. I would imagine we can pass in a flag into the printing routine to indicate whether it needs to produce an expression with the right type, or merely one convertible to the right type.


This comment has not been addressed.



================
Comment at: clang/lib/AST/TemplateBase.cpp:71-72
 
   if (T->isBooleanType() && !Policy.MSVCFormatting) {
     Out << (Val.getBoolValue() ? "true" : "false");
   } else if (T->isCharType()) {
----------------
rsmith wrote:
> It looks like `MSVCFormatting` wants `bool` values to be printed as `0` and `1`, and this patch presumably changes that (along with the printing of other builtin types). I wonder if this is a problem in practice (eg, if such things are used as keys for debug info or similar)...
Do we need to suppress printing the suffixes below in `MSVCFormatting` mode too?


================
Comment at: clang/lib/AST/TemplateBase.cpp:80
     Out << "'";
+  } else if (auto *DT = T->getContainedDeducedType()) {
+    if (DT->isDeduced()) {
----------------
This looks suspicious: we should make sure we always include an indicator of the type if it was deduced, and this suppresses that. Also you don't print anything at all if the type is not deduced.


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

https://reviews.llvm.org/D77598