[PATCH] D93553: [clangd] Make our printing policies for Hover more consistent, especially tags

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 18 15:51:29 PST 2020


sammccall added a comment.

In D93553#2463577 <https://reviews.llvm.org/D93553#2463577>, @qchateau wrote:

> Looks good to me, it removes a lot of duplication and corner cases.
>
> I'd have printed the tag for reference and pointers as well (`class Foo*` instead of `Foo*`). I don't think `Foo*` is much more understandable than `Foo`, so we decide that printing `class Foo` is easier to understand for the user, we should print `class Foo*` as well. 
> But I agree it's even less idiomatic, so I won't argue with your choice.

Yeah, `class Foo *` doesn't seem bad, though for some reason my brain struggles with `class Foo &&` :-)
I really think I like drawing the line *somewhere* in terms of complexity, but maybe it's not in the ideal place.

There's a pragmatic reason to draw it here: simply prepending "class " doesn't work in the presence of qualifiers. So the current patch will print `class Foo` but `const f(oo`, but at least the latter is rare. The inconsistency between `class Foo*` and `const Foo*` would be a bigger deal, I think. And getting `const class Foo*` without also producing `class Foo<class Bar>` would need changes to TypePrinter, which is a bit of a yak-shave.

So I think I'll land this as an improvement over the status quo, at least.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93553



More information about the cfe-commits mailing list