From 8ac9d2ae5839172013ac6e9108398902da8a8969 Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Fri, 12 Nov 2021 15:03:23 +0100 Subject: [PATCH] [clangd] Fix function-arg-placeholder suppression with macros. While here, unhide function-arg-placeholders flag. It's reasonable to want and maybe we should consider making it default. Fixes https://github.com/clangd/clangd/issues/922 Differential Revision: https://reviews.llvm.org/D113765 --- clang-tools-extra/clangd/CodeComplete.cpp | 42 ++++++++++------------ clang-tools-extra/clangd/tool/ClangdMain.cpp | 1 - .../clangd/unittests/CodeCompleteTests.cpp | 20 +++++++++++ 3 files changed, 38 insertions(+), 25 deletions(-) diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index f1f1cae..f033b5e 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -466,32 +466,27 @@ private: // FIXME(ibiryukov): sometimes add template arguments to a snippet, e.g. // we need to complete 'forward<$1>($0)'. return "($0)"; - // Suppress function argument snippets cursor is followed by left - // parenthesis (and potentially arguments) or if there are potentially - // template arguments. There are cases where it would be wrong (e.g. next - // '<' token is a comparison rather than template argument list start) but - // it is less common and suppressing snippet provides better UX. - if (Completion.Kind == CompletionItemKind::Function || - Completion.Kind == CompletionItemKind::Method || - Completion.Kind == CompletionItemKind::Constructor) { - // If there is a potential template argument list, drop snippet and just - // complete symbol name. Ideally, this could generate an edit that would - // paste function arguments after template argument list but it would be - // complicated. Example: - // - // fu^ -> function + + bool MayHaveArgList = Completion.Kind == CompletionItemKind::Function || + Completion.Kind == CompletionItemKind::Method || + Completion.Kind == CompletionItemKind::Constructor || + Completion.Kind == CompletionItemKind::Text /*Macro*/; + // If likely arg list already exists, don't add new parens & placeholders. + // Snippet: function(int x, int y) + // func^(1,2) -> function(1, 2) + // NOT function(int x, int y)(1, 2) + if (MayHaveArgList) { + // Check for a template argument list in the code. + // Snippet: function(int x) + // fu^(1) -> function(1) if (NextTokenKind == tok::less && Snippet->front() == '<') return ""; - // Potentially followed by argument list. + // Potentially followed by regular argument list. if (NextTokenKind == tok::l_paren) { - // If snippet contains template arguments we will emit them and drop - // function arguments. Example: - // - // fu^(42) -> function(42); + // Snippet: function(int x) + // fu^(1,2) -> function(1, 2) if (Snippet->front() == '<') { - // Find matching '>'. Snippet->find('>') will not work in cases like - // template >. Hence, iterate through - // the snippet until the angle bracket balance reaches zero. + // Find matching '>', handling nested brackets. int Balance = 0; size_t I = 0; do { @@ -512,8 +507,7 @@ private: // Replace argument snippets with a simplified pattern. if (Snippet->empty()) return ""; - if (Completion.Kind == CompletionItemKind::Function || - Completion.Kind == CompletionItemKind::Method) { + if (MayHaveArgList) { // Functions snippets can be of 2 types: // - containing only function arguments, e.g. // foo(${1:int p1}, ${2:int p2}); diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp index c6cf5c3..08631a3 100644 --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -233,7 +233,6 @@ opt EnableFunctionArgSnippets{ "function calls. When enabled, completions also contain " "placeholders for method parameters"), init(CodeCompleteOptions().EnableFunctionArgSnippets), - Hidden, }; opt HeaderInsertion{ diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index aff6f6c..81dfdcb 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -1662,6 +1662,9 @@ TEST(CompletionTest, OverloadBundling) { // Overload with bool int a(bool); int b(float); + + X(int); + X(float); }; int GFuncC(int); int GFuncD(int); @@ -1671,6 +1674,10 @@ TEST(CompletionTest, OverloadBundling) { EXPECT_THAT(completions(Context + "int y = X().^", {}, Opts).Completions, UnorderedElementsAre(Labeled("a(…)"), Labeled("b(float)"))); + // Constructor completions are bundled. + EXPECT_THAT(completions(Context + "X z = X^", {}, Opts).Completions, + UnorderedElementsAre(Labeled("X"), Labeled("X(…)"))); + // Non-member completions are bundled, including index+sema. Symbol NoArgsGFunc = func("GFuncC"); EXPECT_THAT( @@ -2323,6 +2330,15 @@ TEST(CompletionTest, CompletionFunctionArgsDisabled) { UnorderedElementsAre(AllOf(Named("foo_class"), SnippetSuffix("<$0>")), AllOf(Named("foo_alias"), SnippetSuffix("<$0>")))); } + { + auto Results = completions( + R"cpp( + #define FOO(x, y) x##f + FO^ )cpp", + {}, Opts); + EXPECT_THAT(Results.Completions, UnorderedElementsAre(AllOf( + Named("FOO"), SnippetSuffix("($0)")))); + } } TEST(CompletionTest, SuggestOverrides) { @@ -3170,6 +3186,7 @@ TEST(CompletionTest, FunctionArgsExist) { clangd::CodeCompleteOptions Opts; Opts.EnableSnippets = true; std::string Context = R"cpp( + #define MACRO(x) int foo(int A); int bar(); struct Object { @@ -3217,6 +3234,9 @@ TEST(CompletionTest, FunctionArgsExist) { Contains(AllOf(Labeled("Container(int Size)"), SnippetSuffix(""), Kind(CompletionItemKind::Constructor)))); + EXPECT_THAT(completions(Context + "MAC^(2)", {}, Opts).Completions, + Contains(AllOf(Labeled("MACRO(x)"), SnippetSuffix(""), + Kind(CompletionItemKind::Text)))); } TEST(CompletionTest, NoCrashDueToMacroOrdering) { -- 2.7.4