[PATCH] D138425: [clangd] Parameter hints for template specialization

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D138425#3941183 <https://reviews.llvm.org/D138425#3941183>, @v1nh1shungry wrote:

> @sammccall Thank you for reviewing and giving suggestions!
>
> I must admit I didn't use it for very long. But I do think this is helpful, at least for templates I'm unfamiliar with.

That sounds good, maybe you can give some examples?
I'm thinking about a heuristic like "show hints where the param name is >2 chars long" or something, at least as a starting point - would be good to see whether this would still provide the value you're seeing.

> Yes, there is a common situation where people use a meaningless template parameter name, but I think the same for functions.

I don't think it's the same: my wild guess would be 30% of function params have useless names, and 90% of template params do.
If this were accurate, it seems function param hints are usually (potentially) useful, while template params are almost always useless.

> I have seen many meaningless parameter names like `D`, `E` even in the LLVM codebase.

Agree. LLVM does this (much) more than the other codebases I'm familiar with.
I'd personally be very happy if we could somehow detect and suppress hints for such params.
But it's not clear-cut: `template<class K, class V> class map` can be meaningful, and with push-to-show-hints interaction the lack of a hint can be confusing.

> Since we can tolerate these, why can't we bear the template parameter?

If template params are useless a larger fraction of the time, then enabling them without doing some filtering might be a net negative in practice.

> And yes, it is a serious problem this is unconfigurable.

We can add a new config category for these (use Config::InlayHints::TemplateParameters), it's just a bit ugly to do so.



================
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,
----------------
v1nh1shungry wrote:
> sammccall wrote:
> > we're missing some mangling of the name to remove the leading `_`
> > (stripLeadingUnderscore?)
> I don't think we should, since we don't do this for function parameters, are there any special reasons for us to do this for template parameters?
We do this for function params, see line 574 of the LHS


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138425