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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
