[clang-tools-extra] 8ac9d2a - [clangd] Fix function-arg-placeholder suppression with macros.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Sat Nov 13 11:51:13 PST 2021
Author: Sam McCall
Date: 2021-11-13T20:50:51+01:00
New Revision: 8ac9d2ae5839172013ac6e9108398902da8a8969
URL: https://github.com/llvm/llvm-project/commit/8ac9d2ae5839172013ac6e9108398902da8a8969
DIFF: https://github.com/llvm/llvm-project/commit/8ac9d2ae5839172013ac6e9108398902da8a8969.diff
LOG: [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
Added:
Modified:
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
index f1f1cae690bc8..f033b5ec001d8 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -466,32 +466,27 @@ struct CodeCompletionBuilder {
// 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^<int> -> function<int>
+
+ 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<class T>(int x)
+ // fu^<int>(1) -> function<int>(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<int>(42);
+ // Snippet: function<class T>(int x)
+ // fu^(1,2) -> function<class T>(1, 2)
if (Snippet->front() == '<') {
- // Find matching '>'. Snippet->find('>') will not work in cases like
- // template <typename T=std::vector<int>>. 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 @@ struct CodeCompletionBuilder {
// 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 c6cf5c3d6e9a3..08631a31eda6c 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -233,7 +233,6 @@ opt<bool> EnableFunctionArgSnippets{
"function calls. When enabled, completions also contain "
"placeholders for method parameters"),
init(CodeCompleteOptions().EnableFunctionArgSnippets),
- Hidden,
};
opt<CodeCompleteOptions::IncludeInsertion> HeaderInsertion{
diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index aff6f6cf25ff9..81dfdcb371fb4 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<typename T>(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) {
More information about the cfe-commits
mailing list