[clangd] Add ArgumentLists config option under Completion #111322
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The new config option is a more flexible version of --function-arg-placeholders, allowing users more detailed control of what is inserted in argument list position when clangd completes the name of a function in a function call context. Fixes llvm#63565
|
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clangd Author: Nathan Ridge (HighCommander4) ChangesThe new config option is a more flexible version of --function-arg-placeholders, allowing users more detailed control of what is inserted in argument list position when clangd completes the name of a function in a function call context. Fixes #63565 Full diff: https://github.com/llvm/llvm-project/pull/111322.diff 9 Files Affected:
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index e910a80ba0bae9..9b38be04e7ddd7 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -451,6 +451,7 @@ void ClangdServer::codeComplete(PathRef File, Position Pos,
CodeCompleteOpts.MainFileSignals = IP->Signals;
CodeCompleteOpts.AllScopes = Config::current().Completion.AllScopes;
+ CodeCompleteOpts.ArgumentLists = Config::current().Completion.ArgumentLists;
// FIXME(ibiryukov): even if Preamble is non-null, we may want to check
// both the old and the new version in case only one of them matches.
CodeCompleteResult Result = clangd::codeComplete(
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
index 89eee392837af4..adca462b53f0a0 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -21,6 +21,7 @@
#include "AST.h"
#include "CodeCompletionStrings.h"
#include "Compiler.h"
+#include "Config.h"
#include "ExpectedTypes.h"
#include "Feature.h"
#include "FileDistance.h"
@@ -350,8 +351,7 @@ struct CodeCompletionBuilder {
CodeCompletionContext::Kind ContextKind,
const CodeCompleteOptions &Opts,
bool IsUsingDeclaration, tok::TokenKind NextTokenKind)
- : ASTCtx(ASTCtx),
- EnableFunctionArgSnippets(Opts.EnableFunctionArgSnippets),
+ : ASTCtx(ASTCtx), ArgumentLists(Opts.ArgumentLists),
IsUsingDeclaration(IsUsingDeclaration), NextTokenKind(NextTokenKind) {
Completion.Deprecated = true; // cleared by any non-deprecated overload.
add(C, SemaCCS, ContextKind);
@@ -561,6 +561,15 @@ struct CodeCompletionBuilder {
}
std::string summarizeSnippet() const {
+ /// localize ArgumentLists tests for better readability
+ const bool None = ArgumentLists == Config::ArgumentListsPolicy::None;
+ const bool Open =
+ ArgumentLists == Config::ArgumentListsPolicy::OpenDelimiter;
+ const bool Delim = ArgumentLists == Config::ArgumentListsPolicy::Delimiters;
+ const bool Full =
+ ArgumentLists == Config::ArgumentListsPolicy::FullPlaceholders ||
+ (!None && !Open && !Delim); // <-- failsafe: Full is default
+
if (IsUsingDeclaration)
return "";
auto *Snippet = onlyValue<&BundledEntry::SnippetSuffix>();
@@ -568,7 +577,7 @@ struct CodeCompletionBuilder {
// All bundles are function calls.
// FIXME(ibiryukov): sometimes add template arguments to a snippet, e.g.
// we need to complete 'forward<$1>($0)'.
- return "($0)";
+ return None ? "" : (Open ? "(" : "($0)");
if (Snippet->empty())
return "";
@@ -607,7 +616,7 @@ struct CodeCompletionBuilder {
return "";
}
}
- if (EnableFunctionArgSnippets)
+ if (Full)
return *Snippet;
// Replace argument snippets with a simplified pattern.
@@ -622,9 +631,9 @@ struct CodeCompletionBuilder {
bool EmptyArgs = llvm::StringRef(*Snippet).ends_with("()");
if (Snippet->front() == '<')
- return EmptyArgs ? "<$1>()$0" : "<$1>($0)";
+ return None ? "" : (Open ? "<" : (EmptyArgs ? "<$1>()$0" : "<$1>($0)"));
if (Snippet->front() == '(')
- return EmptyArgs ? "()" : "($0)";
+ return None ? "" : (Open ? "(" : (EmptyArgs ? "()" : "($0)"));
return *Snippet; // Not an arg snippet?
}
// 'CompletionItemKind::Interface' matches template type aliases.
@@ -638,7 +647,7 @@ struct CodeCompletionBuilder {
// e.g. Foo<${1:class}>.
if (llvm::StringRef(*Snippet).ends_with("<>"))
return "<>"; // can happen with defaulted template arguments.
- return "<$0>";
+ return None ? "" : (Open ? "<" : "<$0>");
}
return *Snippet;
}
@@ -654,7 +663,8 @@ struct CodeCompletionBuilder {
ASTContext *ASTCtx;
CodeCompletion Completion;
llvm::SmallVector<BundledEntry, 1> Bundled;
- bool EnableFunctionArgSnippets;
+ /// the way argument lists are handled.
+ Config::ArgumentListsPolicy ArgumentLists;
// No snippets will be generated for using declarations and when the function
// arguments are already present.
bool IsUsingDeclaration;
diff --git a/clang-tools-extra/clangd/CodeComplete.h b/clang-tools-extra/clangd/CodeComplete.h
index a7c1ae95dcbf49..2628f9edafe85d 100644
--- a/clang-tools-extra/clangd/CodeComplete.h
+++ b/clang-tools-extra/clangd/CodeComplete.h
@@ -17,6 +17,7 @@
#include "ASTSignals.h"
#include "Compiler.h"
+#include "Config.h"
#include "Protocol.h"
#include "Quality.h"
#include "index/Index.h"
@@ -96,10 +97,6 @@ struct CodeCompleteOptions {
/// '->' on member access etc.
bool IncludeFixIts = false;
- /// Whether to generate snippets for function arguments on code-completion.
- /// Needs snippets to be enabled as well.
- bool EnableFunctionArgSnippets = true;
-
/// Whether to include index symbols that are not defined in the scopes
/// visible from the code completion point. This applies in contexts without
/// explicit scope qualifiers.
@@ -107,6 +104,10 @@ struct CodeCompleteOptions {
/// Such completions can insert scope qualifiers.
bool AllScopes = false;
+ /// The way argument list on calls '()' and generics '<>' are handled.
+ Config::ArgumentListsPolicy ArgumentLists =
+ Config::ArgumentListsPolicy::FullPlaceholders;
+
/// Whether to use the clang parser, or fallback to text-based completion
/// (using identifiers in the current file and symbol indexes).
enum CodeCompletionParse {
diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h
index 41143b9ebc8d27..8fcbc5c33469fa 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -126,11 +126,25 @@ struct Config {
std::vector<std::string> FullyQualifiedNamespaces;
} Style;
+ /// controls the completion options for argument lists.
+ enum class ArgumentListsPolicy {
+ /// nothing, no argument list and also NO Delimiters "()" or "<>".
+ None,
+ /// open, only opening delimiter "(" or "<".
+ OpenDelimiter,
+ /// empty pair of delimiters "()" or "<>".
+ Delimiters,
+ /// full name of both type and variable.
+ FullPlaceholders,
+ };
+
/// Configures code completion feature.
struct {
/// Whether code completion includes results that are not visible in current
/// scopes.
bool AllScopes = true;
+ /// controls the completion options for argument lists.
+ ArgumentListsPolicy ArgumentLists = ArgumentListsPolicy::FullPlaceholders;
} Completion;
/// Configures hover feature.
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp
index f32f674443ffeb..58610a5b87922d 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -622,6 +622,21 @@ struct FragmentCompiler {
C.Completion.AllScopes = AllScopes;
});
}
+ if (F.ArgumentLists) {
+ if (auto Val =
+ compileEnum<Config::ArgumentListsPolicy>("ArgumentLists",
+ *F.ArgumentLists)
+ .map("None", Config::ArgumentListsPolicy::None)
+ .map("OpenDelimiter",
+ Config::ArgumentListsPolicy::OpenDelimiter)
+ .map("Delimiters", Config::ArgumentListsPolicy::Delimiters)
+ .map("FullPlaceholders",
+ Config::ArgumentListsPolicy::FullPlaceholders)
+ .value())
+ Out.Apply.push_back([Val](const Params &, Config &C) {
+ C.Completion.ArgumentLists = *Val;
+ });
+ }
}
void compile(Fragment::HoverBlock &&F) {
diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h
index f3e51a9b6dbc4b..fc1b45f5d4c3e9 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -32,6 +32,7 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIGFRAGMENT_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIGFRAGMENT_H
+#include "Config.h"
#include "ConfigProvider.h"
#include "llvm/Support/SMLoc.h"
#include "llvm/Support/SourceMgr.h"
@@ -308,6 +309,13 @@ struct Fragment {
/// Whether code completion should include suggestions from scopes that are
/// not visible. The required scope prefix will be inserted.
std::optional<Located<bool>> AllScopes;
+ /// How to present the argument list between '()' and '<>':
+ /// valid values are enum Config::ArgumentListsPolicy values:
+ /// None: Nothing at all
+ /// OpenDelimiter: only opening delimiter "(" or "<"
+ /// Delimiters: empty pair of delimiters "()" or "<>"
+ /// FullPlaceholders: full name of both type and parameter
+ std::optional<Located<std::string>> ArgumentLists;
};
CompletionBlock Completion;
diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp
index 3e9b6a07d3b325..bcdda99eeed67a 100644
--- a/clang-tools-extra/clangd/ConfigYAML.cpp
+++ b/clang-tools-extra/clangd/ConfigYAML.cpp
@@ -230,6 +230,10 @@ class Parser {
if (auto AllScopes = boolValue(N, "AllScopes"))
F.AllScopes = *AllScopes;
});
+ Dict.handle("ArgumentLists", [&](Node &N) {
+ if (auto ArgumentLists = scalarValue(N, "ArgumentLists"))
+ F.ArgumentLists = *ArgumentLists;
+ });
Dict.parse(N);
}
diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index 3a5449ac8c7994..0bf2916278b639 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -242,13 +242,13 @@ opt<std::string> FallbackStyle{
init(clang::format::DefaultFallbackStyle),
};
-opt<bool> EnableFunctionArgSnippets{
+opt<int> EnableFunctionArgSnippets{
"function-arg-placeholders",
cat(Features),
- desc("When disabled, completions contain only parentheses for "
- "function calls. When enabled, completions also contain "
+ desc("When disabled (0), completions contain only parentheses for "
+ "function calls. When enabled (1), completions also contain "
"placeholders for method parameters"),
- init(CodeCompleteOptions().EnableFunctionArgSnippets),
+ init(-1),
};
opt<CodeCompleteOptions::IncludeInsertion> HeaderInsertion{
@@ -650,6 +650,7 @@ class FlagsConfigProvider : public config::Provider {
std::optional<Config::CDBSearchSpec> CDBSearch;
std::optional<Config::ExternalIndexSpec> IndexSpec;
std::optional<Config::BackgroundPolicy> BGPolicy;
+ std::optional<Config::ArgumentListsPolicy> ArgumentLists;
// If --compile-commands-dir arg was invoked, check value and override
// default path.
@@ -694,6 +695,12 @@ class FlagsConfigProvider : public config::Provider {
BGPolicy = Config::BackgroundPolicy::Skip;
}
+ if (EnableFunctionArgSnippets >= 0) {
+ ArgumentLists = EnableFunctionArgSnippets
+ ? Config::ArgumentListsPolicy::FullPlaceholders
+ : Config::ArgumentListsPolicy::Delimiters;
+ }
+
Frag = [=](const config::Params &, Config &C) {
if (CDBSearch)
C.CompileFlags.CDBSearch = *CDBSearch;
@@ -701,6 +708,8 @@ class FlagsConfigProvider : public config::Provider {
C.Index.External = *IndexSpec;
if (BGPolicy)
C.Index.Background = *BGPolicy;
+ if (ArgumentLists)
+ C.Completion.ArgumentLists = *ArgumentLists;
if (AllScopesCompletion.getNumOccurrences())
C.Completion.AllScopes = AllScopesCompletion;
@@ -916,7 +925,6 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
Opts.CodeComplete.IncludeIndicator.Insert.clear();
Opts.CodeComplete.IncludeIndicator.NoInsert.clear();
}
- Opts.CodeComplete.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
Opts.CodeComplete.RunParser = CodeCompletionParse;
Opts.CodeComplete.RankingModel = RankingModel;
diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 96d1ee1f0add73..a89f4997362265 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -11,6 +11,7 @@
#include "ClangdServer.h"
#include "CodeComplete.h"
#include "Compiler.h"
+#include "Config.h"
#include "Feature.h"
#include "Matchers.h"
#include "Protocol.h"
@@ -2595,10 +2596,10 @@ TEST(SignatureHelpTest, DynamicIndexDocumentation) {
ElementsAre(AllOf(sig("foo() -> int"), sigDoc("Member doc"))));
}
-TEST(CompletionTest, CompletionFunctionArgsDisabled) {
+TEST(CompletionTest, ArgumentListsPolicy) {
CodeCompleteOptions Opts;
Opts.EnableSnippets = true;
- Opts.EnableFunctionArgSnippets = false;
+ Opts.ArgumentLists = Config::ArgumentListsPolicy::Delimiters;
{
auto Results = completions(
@@ -2670,6 +2671,26 @@ TEST(CompletionTest, CompletionFunctionArgsDisabled) {
EXPECT_THAT(Results.Completions, UnorderedElementsAre(AllOf(
named("FOO"), snippetSuffix("($0)"))));
}
+ {
+ Opts.ArgumentLists = Config::ArgumentListsPolicy::None;
+ auto Results = completions(
+ R"cpp(
+ void xfoo(int x, int y);
+ void f() { xfo^ })cpp",
+ {}, Opts);
+ EXPECT_THAT(Results.Completions,
+ UnorderedElementsAre(AllOf(named("xfoo"), snippetSuffix(""))));
+ }
+ {
+ Opts.ArgumentLists = Config::ArgumentListsPolicy::OpenDelimiter;
+ auto Results = completions(
+ R"cpp(
+ void xfoo(int x, int y);
+ void f() { xfo^ })cpp",
+ {}, Opts);
+ EXPECT_THAT(Results.Completions,
+ UnorderedElementsAre(AllOf(named("xfoo"), snippetSuffix("("))));
+ }
}
TEST(CompletionTest, SuggestOverrides) {
|
|
This was originally submitted by @MK-Alias and reviewed by me at #108005. The only changes I've made relative to #108005 is to remove unrelated formatting changes (per my last outstanding review comment there) and reword the commit message. I will therefore go ahead and merge this without additional review. |
Kyvangka1610
added a commit
to Kyvangka1610/llvm-project
that referenced
this pull request
Oct 7, 2024
* commit 'FETCH_HEAD': [X86] getIntImmCostInst - pull out repeated Imm.getBitWidth() calls. NFC. [X86] Add test coverage for llvm#111323 [Driver] Use empty multilib file in another test (llvm#111352) [clang][OpenMP][test] Use x86_64-linux-gnu triple for test referencing avx512f feature (llvm#111337) [doc] Fix Kaleidoscope tutorial chapter 3 code snippet and full listing discrepancies (llvm#111289) [Flang][OpenMP] Improve entry block argument creation and binding (llvm#110267) [x86] combineMul - handle 0/-1 KnownBits cases before MUL_IMM logic (REAPPLIED) [llvm-dis] Fix non-deterministic disassembly across multiple inputs (llvm#110988) [lldb][test] TestDataFormatterLibcxxOptionalSimulator.py: change order of ifdefs [lldb][test] Add libcxx-simulators test for std::optional (llvm#111133) [x86] combineMul - use computeKnownBits directly to find MUL_IMM constant splat. (REAPPLIED) Reland "[lldb][test] TestDataFormatterLibcxxStringSimulator.py: add new padding layout" (llvm#111123) Revert "[x86] combineMul - use computeKnownBits directly to find MUL_IMM constant splat." update_test_checks: fix a simple regression (llvm#111347) [LegalizeVectorTypes] Always widen fabs (llvm#111298) [lsan] Make ReportUnsuspendedThreads return bool also for Fuchsia [mlir][vector] Add more tests for ConvertVectorToLLVM (6/n) (llvm#111121) [bazel] port 9144fed [SystemZ] Remove inlining threshold multiplier. (llvm#106058) [LegalizeVectorTypes] When widening don't check for libcalls if promoted (llvm#111297) [clang][Driver] Improve multilib custom error reporting (llvm#110804) [clang][Driver] Rename "FatalError" key to "Error" in multilib.yaml (llvm#110804) [LLVM][Maintainers] Update release managers (llvm#111164) [Clang][Driver] Add option to provide path for multilib's YAML config file (llvm#109640) [LoopVectorize] Remove redundant code in emitSCEVChecks (llvm#111132) [AMDGPU] Only emit SCOPE_SYS global_wb (llvm#110636) [ELF] Change Ctx::target to unique_ptr (llvm#111260) [ELF] Pass Ctx & to some free functions [RISCV] Only disassemble fcvtmod.w.d if the rounding mode is rtz. (llvm#111308) [Clang] Remove the special-casing for RequiresExprBodyDecl in BuildResolvedCallExpr() after fd87d76 (llvm#111277) [ELF] Pass Ctx & to InputFile [clang-format] Add AlignFunctionDeclarations to AlignConsecutiveDeclarations (llvm#108241) [AMDGPU] Support preloading hidden kernel arguments (llvm#98861) [ELF] Move static nextGroupId isInGroup to LinkerDriver [clangd] Add ArgumentLists config option under Completion (llvm#111322) [ELF] Pass Ctx & to SyntheticSections [ELF] Pass Ctx & to Symbols [ELF] Pass Ctx & to Symbols [ELF] getRelocTargetVA: pass Ctx and Relocation. NFC [clang-tidy] Avoid capturing a local variable in a static lambda in UseRangesCheck (llvm#111282) [VPlan] Use pointer to member 0 as VPInterleaveRecipe's pointer arg. (llvm#106431) [clangd] Simplify ternary expressions with std::optional::value_or (NFC) (llvm#111309) [libc++][format][2/3] Optimizes c-string arguments. (llvm#101805) [RISCV] Combine RVBUnary and RVKUnary into classes that are more similar to ALU(W)_r(r/i). NFC (llvm#111279) [ELF] Pass Ctx & to InputFiles [libc] GPU RPC interface: add return value to `rpc_host_call` (llvm#111288) Signed-off-by: kyvangka1610 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The new config option is a more flexible version of --function-arg-placeholders, allowing users more detailed control of what is inserted in argument list position when clangd completes the name of a function in a function call context.
Fixes #63565