[PATCH] D137040: [clangd] Add heuristic for dropping snippet when completing member function pointer

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

In D137040#3909906 <https://reviews.llvm.org/D137040#3909906>, @tom-anders wrote:

> We now probably also need a unit test for Sema that tests the new flag, right?

Good point. My understanding is that most test coverage of libSema's code completion takes the form of lit tests <https://searchfox.org/llvm/source/clang/test/CodeCompletion/using-enum.cpp> which are suitable for checking the completion proposals themselves but not flags like this. However, we do have one unittest file <https://searchfox.org/llvm/source/clang/unittests/Sema/CodeCompleteTest.cpp> which could be adapted for testing the flag. It may require some light refactoring to make this work (e.g. I don't think we can just stuff the `CodeCompletionResult`s themselves into the returned `CompletionContext` because they point to `Decl`s and such whose lifetime is tied to the AST which is destroyed by the time `runCompletion()` returns).



================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:414
                    &Completion.RequiredQualifier, IsPattern);
+      if (!C.SemaResult->FunctionCanBeCall)
+        S.SnippetSuffix.clear();
----------------
tom-anders wrote:
> Here, the SnippetSuffix is still created and then immediately cleared again. But the logic inside getSignature is quite complex and it's used in multiple places, so I didn't really want to touch that function for now.
Fair enough, we can optimize this later if it turns out to be necessary


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137040