[PATCH] D134453: Disambiguate type names when printing NTTP types

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

> you would get something like `S<Shape{.length = Length{StrongTypedef<unsigned int, Length>{.value = 50}}, .width = Width{StrongTypedef<unsigned int, Width>{.value = 70}}}>` (inspired by this GCC output), which is truly verbose. However, the current way of printing (assuming member names are printed) it would print something like `S<{.length = {{.value = 50}}, .width = {{.value = 70}}}>`. Ideally, in this scenario, probably the best possible output would be `S<Shape{.length = Length{{.value = 50}}, .width = Width{{.value = 70}}}>`. however, I'm not exactly sure how to achieve this (my patch would print Shape, but not Length and Width with my policy disabled.

Yeah, considering this dimension I'd have thought /just/ printing the member names would be more informative than printing the type (but no doubt folks might have different opinions) - and I guess keeping the top level type so the identifier is valid code (not a strict necessity, but seems nice to have). That way a type /without/ strong typedefs is more readable `S<Shape{.length = 50, .width = 70}>` and the strong typedefs would be `S<Shape{.length = {{.value = 50}}, .width = {{.value = 70}}}>`. Rather than `S<Shape{50, 70}>` V `S<Shape{Width{StrongTypedef<unsigned int, Width>{50}, Length{StrongTypedef<unsigned int, Length>{70}}>`.

But yeah, not sure/open to perspectives.

@aaron.ballman - member names V type names V both?



================
Comment at: clang/test/CodeGenCXX/debug-info-template.cpp:224
 template void f1<t1 () volatile, t1 () const volatile, t1 () &, t1 () &&>();
-// CHECK: !DISubprogram(name: "f1<RawFuncQual::t1 () volatile, RawFuncQual::t1 () const volatile, RawFuncQual::t1 () &, RawFuncQual::t1 () &&>", 
+// CHECK: !DISubprogram(name: "f1<RawFuncQual::t1 () volatile, RawFuncQual::t1 () const volatile, RawFuncQual::t1 () &, RawFuncQual::t1 () &&>",
 // CHECK-SAME: templateParams: ![[RAW_FUNC_QUAL_ARGS:[0-9]*]],
----------------
DoDoENT wrote:
> dblaikie wrote:
> > Looks like some unintended whitespace changes? (removing trailing whitespace from these lines) 
> Sorry about that. I've configured my editor to always remove trailing whitespace (we have this policy in the company) on save action. I can return the trailing whitespace if you want, although I would like to understand the reasoning for keeping the trailing whitespace...
Oh, no problem with fixing them when you're touching that line of code (& I guess that's what your editor does - and then when these lines were undone, the whitespace removal was leftover) - but we tend not to do trailing whitespace cleanup by itself because it isn't worth the cost/disadvantage when it comes to doing source archaeology/blame/etc - and even if we are doing the cleanup it's usually important to separate it into independent commits so it's easier to focus on the important parts of a change under review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134453