[PATCH] D156605: [clangd][CodeComplete] Improve FunctionCanBeCall

[Nathan Ridge via Phabricator via cfe-commits]

nridge accepted this revision.
nridge added a comment.
This revision is now accepted and ready to land.

Thank you, the updates look good! Please go ahead and merge after addressing the last minor nits.



================
Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:594
+        Contains(AllOf(named("generic"),
+                       signature("<typename T, typename U, typename V>(U, V)"),
+                       snippetSuffix("<${1:typename T}, ${2:typename U}>"))));
----------------
It seems a bit inconsistent that the signature includes parameters with default arguments in the non-call case, and not in the call case. I guess the reason for this is that in the non-call case, the parameters with defaults become a `CK_Optional` chunk which clangd adds to the signature [here](https://searchfox.org/llvm/rev/4c241a9335c3046e418e1b4afc162bc576c072fd/clang-tools-extra/clangd/CodeCompletionStrings.cpp#213-214), while in the call case the deduced parameters (which include the ones with defaults) are omitted from the completion string altogether.

It may be more consistent to put the deduced parameters into an optional chunk as well, and let the consumer of the CodeCompletionString decide whether to use them?

Anyways, that's an idea for the future, no change needed now.


================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:1301
+
+  // When completing a non-static member function (and not via
+  // dot/arrow member access) and we're not inside that class' scope,
----------------
nit: let's shorten this comment to just "If we're not inside the scope of the method's class, it can't be a call"

(The parts about non-static and dot/arrow member access are checked and explained in `canFunctionBeCalled()`)


================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3589
     llvm::SmallBitVector Deduced;
-    Sema::MarkDeducedTemplateParameters(Ctx, FunTmpl, Deduced);
+    // Avoid running it if this is not a call: We'd emit *all* template
+    // parameters.
----------------
nit: "We'd" --> "We should"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156605