[PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.


================
Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:470
+                  ExpectedHint{": S1", "x"},
+                  ExpectedHint{": S2::Inner<int>", "y"});
 }
----------------
nridge wrote:
> mizvekov wrote:
> > mizvekov wrote:
> > > nridge wrote:
> > > > This test expectation change is suspicious. The type is being printed with `PrintingPolicy::SuppressScope=true`, shouldn't that still be respected?
> > > Thanks for pointing that out, I will take a look, but I suspect this to be some TypePrinter issue.
> > Could you explain to me why the former behavior is more desirable here?
> > 
> > I initially understood that this hint should provide the type written in a form that would actually work if it replaced the 'auto'.
> > It is strange, but if it is just meant as a hint, it's still fine I guess.
> > 
> > Or maybe this was broken in the first place and just was just missing a FIXME?
> The type is being printed with a `PrintingPolicy` which has the [non-default SuppressScope flag set](https://searchfox.org/llvm/rev/e158b5634aa67ea3039a62c3d8bda79b77b3b21c/clang-tools-extra/clangd/InlayHints.cpp#34).
> 
> The [documentation](https://searchfox.org/llvm/rev/e158b5634aa67ea3039a62c3d8bda79b77b3b21c/clang/include/clang/AST/PrettyPrinter.h#129) of this flag says "Suppresses printing of scope specifiers", which I understand to mean both namespace and class scope specifiers.
> 
> If you're asking why we are using this flag for inlay hints, it's to keep the hints short so they don't take up too much horizontal space. Since the hints are displayed inline, hints that are too long can result in the line of code visually extending past the width of the editor window, and we try to minimize the impact of this.
I see.

The problem is that the ElaboratedType sugar does not respect this when printing the nested name specifier it contains.
The quick fix of trying to make it respect it shows that there are a bunch of tests that expect it not to.
It seems that specific policy is used in places that need it to recover the type as written.

So if we want this behavior, we probably need to extend the printing policy with a new flag here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110216