dblaikie added a subscriber: aaron.ballman.
dblaikie added inline comments.


================
Comment at: clang/include/clang/AST/PrettyPrinter.h:307
+  /// decltype(s) will be printed as "S<Point{1,2}>" if enabled and as 
"S<{1,2}>" if disabled,
+  /// regardless if PrintCanonicalTypes is enabled.
+  unsigned AlwaysIncludeTypeForNonTypeTemplateArgument : 1;
----------------
DoDoENT wrote:
> dblaikie wrote:
> > What does `PrintCanonicalTypes` have to do with this? Does it overlap with 
> > this functionality in some way, but doesn't provide the functionality you 
> > want in particular?
> Thank you for the question. If you set the `PrintCanonicalTypes` to `false`, 
> the `S<Point{1, 2}>` would be printed as `S<Point{1, 2}>` even without this 
> patch. However, if you set it to `true`, it will be printed as `S<{1, 2}>`.
> 
> I don't fully understand why it does that, but it's quite annoying.
> 
> For a better example, please take a look at the 
> `TemplateIdWithComplexFullTypeNTTP` unit tests that I've added: if 
> `PrintCanonicalTypes` is set to `true`, the original print output of type is 
> `NDArray<float, {{{0}}}, {{{0}}}, {{{0}}}>`, and if set to `false` (which is 
> default), the output is `NDArray<float, Height{{{0}}}, Width{{{0}}}, 
> Channels{{{0}}}>` - so the NTTP type is neither fully written nor fully 
> omitted, which is weird.
> 
> As I said, I don't really understand the idea behind `PrintCanonicalTypes`, 
> but when my new `AlwaysIncludeTypeForNonTypeTemplateArgument` is enabled, you 
> will get the full type printed, regardless of value of `PrintCanonicalTypes` 
> setting.
> 
Perhaps this might be more of a bug in PrintCanonicalTypes than something to 
add a separate flag for.

@aaron.ballman D55552 for context here... 

Hmm, actually, just adding the top level `Height{{0}}, Width{{0}}, 
Channels{{0}}` is sufficient to make this code compile (whereas with the 
`{{{0}}}` it doesn't form a valid identifier.

So what's your use case for needing more explicitness than that top level? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134453

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to