sammccall added a comment.
The code looks pretty good and this makes a lot of logical sense.
However, there's an ugly practical issue: it's overwhelmingly common for
template parameters to have cryptic and useless names.
It seems like turning `vector<int>` into `vector</*Tp: */int>` actually makes
things worse.
And this gets yet worse: because this is type `parameter` and controlled by
that config option, so people can't disable this without disabling the (very
practically useful) function param hints.
Have you been using this for a while? Do you find it to be a practical help?
Spammy?
================
Comment at: clang-tools-extra/clangd/InlayHints.cpp:378
+ bool VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc L) {
+ auto TP = L.getTypePtr()
----------------
we need a bailout `if (!Cfg.InlayHints.Parameters)`
================
Comment at: clang-tools-extra/clangd/InlayHints.cpp:434
+ break;
+ if (auto Name = TP[I]->getName(); shouldHintName(TA[I], Name))
+ addInlayHint(TA[I].getSourceRange(), HintSide::Left,
----------------
we're missing some mangling of the name to remove the leading `_`
(stripLeadingUnderscore?)
================
Comment at: clang-tools-extra/clangd/InlayHints.cpp:553
+ llvm::raw_string_ostream Out(ArgContent);
+ TA.getArgument().print(AST.getPrintingPolicy(), Out, false);
+ if (ArgContent == ParamName)
----------------
I think we should avoid running the printer on every arg. Instead probably
switch over the types of templatearguments, and delegate to
shouldHintName(Expr, handle TagType/Typedef/other common nodes specifically, as
done in shouldHintName).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138425/new/
https://reviews.llvm.org/D138425
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
