[PATCH] D120258: [clangd] Add inlay hints for auto-typed parameters with one instantiation.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 4 inline comments as done.
sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/AST.cpp:572
+#define TEMPLATE_TYPE(SomeTemplateDecl)                                        \
+  if (auto *STD = llvm::dyn_cast<SomeTemplateDecl>(TD)) {                      \
+    for (auto *Spec : STD->specializations()) {                                \
----------------
nridge wrote:
> Is it possible to extract this into a helper function template?
> 
> I'm not a huge fan of using macros like this, as the macro definition is harder to read (e.g. its tokens don't get semantic highlighting) and edit (e.g. completion).
Oops, of course.


================
Comment at: clang-tools-extra/clangd/AST.h:136
+// FIXME: handle more type patterns.
+llvm::Optional<TemplateTypeParmTypeLoc> getContainedAutoParamType(TypeLoc TL);
+
----------------
nridge wrote:
> Why `Optional` if `TypeLoc` can represent a null type loc?
It was to match getContainedAutoParamType, which I guess wanted to be explicit about the fact that we may not find one. But this isn't common, so I've dropped it.


================
Comment at: clang-tools-extra/clangd/InlayHints.cpp:323
+           "Instantiated function has fewer (non-pack) parameters?");
+    return InstantiatedFunction->getParamDecl(ParamIdx);
+  }
----------------
nridge wrote:
> Here's a test case which slips past these checks:
> 
> ```
> int waldo(auto... args, auto last);
> int x = waldo<int, float>(1, 2.0, 'x');
> ```
> 
> `last` incorrectly gets `float` (rather than `char`) as a hint
Ah, good catch! For some reason I had it in my head that a pack had to be at the end. (It can't be deduced though? Weird...)

I made this give up hinting once we see a pack.

Added that test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120258