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

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.


================
Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:470
+                  ExpectedHint{": S1", "x"},
+                  ExpectedHint{": S2::Inner<int>", "y"});
 }
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110216