[PATCH] D70445: [clangd] Show lambda signature for lambda autocompletions

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.


================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3335
+    return nullptr;
+  const auto *LambdaType = VD->getType().getTypePtr();
+  if (const auto *SugaredType = dyn_cast<AutoType>(LambdaType))
----------------
sammccall wrote:
> VD->getType()->getAsCXXRecordDecl() should be all you need.
> getAs() and friends desugar as needed.
Could you please elaborate here? `VD->getType()` would be a `QualType`, so I first cast it to the pointer and then desugar it (otherwise I might not be able to call `->getAsCXXRecordDecl()`), then I do the `getAsCXXRecordDecl()` as suggested. Is there any way I could simplify this?


================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3342
+  const auto *RecordDecl = LambdaType->getAsCXXRecordDecl();
+  // FIXME: Don't ignore generic lambdas.
+  return (RecordDecl && RecordDecl->isLambda() &&
----------------
sammccall wrote:
> sammccall wrote:
> > can we fix that instead of leaving a FIXME? should only be a couple of lines I think
> also FIXME: support other functor types like std::function?
> 
> looking at the implementation of getLambdaCallOperatorHelper it looks pretty easy
I was thinking about it, but decided against it in the end because of possible incomplete support of the generic lambdas. I have added the basic handling which, given

```
auto foo(auto x, const auto &y) { return &y; }
```

would show

```
foo(auto x, const auto &y) -> auto
```

in code completion.

This should be about exposing some human-readable dummy template types - showing `foo<class A, class B>(A a, const B &b) -> B *` instead. This is a bit more work: not only should we come up with the classnames (which is somewhat OK), but also connect the dots, so that the same typename is "propagated" from the piece which is responsible for adding `<class A, class B>` to the following pieces - the arguments and return type. I don't see any simple heuristic here and probably the correct way of doing that would be assigning a dummy typename to every TemplateTypeParmDecl and then propagating this information to `AddTemplateParameterChunks`, `AddFunctionParameterChunks`, `AddFunctionTypeQualsToCompletionString`, but it's certainly not several lines of code and it might be better to leave this for the next patch. This probably requires tweaking `Sema::MarkDeducedTemplateParameters` or doing something similar.

This kind of increases the scope of the patch (which is probably not a great thing) and also makes it incomplete in some sense, but that might be OK.

What do you think? Should I do that in a separate patch?


================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3396
+
+  AddResultTypeChunk(Ctx, Policy, ND, CCContext.getBaseType(), Result);
+
----------------
sammccall wrote:
> above the return statement to below - why?
For lambdas, `ND` should be replaced with `CallOperator`, hence I think it's easier to repeat this line of code in the above function/lambda handler (inserting the correct delcaration) and then simply proceed with the general case (i.e. use `ND` for the result type).

Does this make sense? Can I simplify this somehow?


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

https://reviews.llvm.org/D70445