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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
