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

Matheus Izvekov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 24 13:25:48 PDT 2022


mizvekov added a comment.

In D134453#3880403 <https://reviews.llvm.org/D134453#3880403>, @dblaikie wrote:

> Could you describe this in more detail? I find that to be more verbose than is necessary/helpful and probably the variable name based option to be more informative than the type name based option (if I have `Shape{3, 5}` (because I have two int members) that's less helpful than `Shape{.length = 3, .width = 5}` and all the type information seems pretty large/verbose.

I agree printing all of the types of all of the subobjects is too much.

I wouldn't object to what you are suggesting, it seems that if all we had was the canonical type of the NTTP, and no idea of what would be helpful for this diagnostic, then printing it in this designated-initializer style seems more readable, for reasonably small objects.

Though if we had the as-written type, I think the best option here is just printing the expression that initializes it, it's type, and perhaps if it was simple declref, pointing to the declaration, but that is probably situational.

> The former seems to not add a lot of value for the user - in this case due to the CRTP and other shenanigans. Admittedly other types might have nested aggregates that are more interesting/less obvious - but variable names still seem to me like they'd be more informative than types. (what I'm suggesting is the outermost has a type (since there's nothing else to use there, and C++ requires a type there) and nested things could use variable names (because they're more contextually relevant - differentiating a `width` and `height`, for instance, even though they might be the same type))

Yeah, that definitely looks better than all the other options, if all we had was the NTTP value.

> That feels liable to be /really/ verbose & make it harder to find the signal in all that noise, but maybe there's ways to make that work. Certainly having the type as written gives us more choices about how to do it/see what can be made to work, for sure.

It seems to me that, at least for this diagnostic, and probably most others, just printing the expression that initializes the NTTP and it's type is enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134453



More information about the cfe-commits mailing list