[PATCH] D155370: [CodeComplete] Don't emit parameters when not FunctionCanBeCall

Sun Jul 16 00:29:07 PDT 2023

sammccall added a comment.

I'm not sure switching to RK_Pattern is ideal, but I think there are a couple of (easy?) alternatives that achieve the same.

> Regard to the heuristic itself, it's currently a bit inconvenient that I have to switch headers and sources back and forth and do copy-pastes when I'm writing member function definitions outside of a class.

Yeah, it would be nice to have completions that provide the declaration syntax.
There's currently support for adding declarations of method overrides to derived classes: see `AddOverrideResults` in SemaCodeComplete, this should be kind-of similar.
These *do* use RK_Pattern, and I think it's appropriate there. (Though it does cause some ranking problems...)

================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:441
                    C.SemaResult->CursorKind, &Completion.RequiredQualifier);
-      if (!C.SemaResult->FunctionCanBeCall)
-        S.SnippetSuffix.clear();
----------------
I guess an alternative to this patch would be to clear S.Signature here too?

================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:1425
+            Method, Builder);
+        Result R(Builder.TakeString(), /*Priority=*/CCP_Declaration,
+                 /*CursorKind=*/CXCursor_CXXMethod,
----------------
the problem (from clangd's perspective) with emitting this as a pattern is we don't know which decl this is, and so we lose:

 - ranking information (score popular decls higher)
 - semantic information (this is a function and should be marked in LSP as such)

Is it possible to change the behavior of Result::CreateCodeCompletionString() instead, based on Result::FunctionCanBeCall? Then we get to keep RK_Declaration and get the right string

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155370