From 4e9cbd615c56604fbc3c05f677b9da11f30977a2 Mon Sep 17 00:00:00 2001 From: MK-Alias Date: Tue, 10 Sep 2024 13:24:46 +0200 Subject: [PATCH 1/6] issue-63565: community requested small QoL fix for more configurability on --function-arg-placeholders --- clang-tools-extra/clangd/CodeComplete.cpp | 32 ++++++++++-------- clang-tools-extra/clangd/CodeComplete.h | 18 +++++++++-- clang-tools-extra/clangd/tool/ClangdMain.cpp | 34 +++++++++++++++----- 3 files changed, 60 insertions(+), 24 deletions(-) diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 89eee392837af49..9fb264ef9160b3c 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -350,8 +350,7 @@ struct CodeCompletionBuilder { CodeCompletionContext::Kind ContextKind, const CodeCompleteOptions &Opts, bool IsUsingDeclaration, tok::TokenKind NextTokenKind) - : ASTCtx(ASTCtx), - EnableFunctionArgSnippets(Opts.EnableFunctionArgSnippets), + : ASTCtx(ASTCtx), PlaceholderType(Opts.PlaceholderType), IsUsingDeclaration(IsUsingDeclaration), NextTokenKind(NextTokenKind) { Completion.Deprecated = true; // cleared by any non-deprecated overload. add(C, SemaCCS, ContextKind); @@ -561,6 +560,14 @@ struct CodeCompletionBuilder { } std::string summarizeSnippet() const { + /// localize PlaceholderType for better readability + const bool None = PlaceholderType == CodeCompleteOptions::None; + const bool Open = PlaceholderType == CodeCompleteOptions::OpenDelimiter; + const bool Delim = PlaceholderType == CodeCompleteOptions::Delimiters; + const bool Full = + PlaceholderType == CodeCompleteOptions::FullPlaceholders || + (!None && !Open && !Delim); // <-- failsafe + if (IsUsingDeclaration) return ""; auto *Snippet = onlyValue<&BundledEntry::SnippetSuffix>(); @@ -568,7 +575,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 +614,7 @@ struct CodeCompletionBuilder { return ""; } } - if (EnableFunctionArgSnippets) + if (Full) return *Snippet; // Replace argument snippets with a simplified pattern. @@ -622,9 +629,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" : "($0)")); return *Snippet; // Not an arg snippet? } // 'CompletionItemKind::Interface' matches template type aliases. @@ -638,7 +645,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 +661,7 @@ struct CodeCompletionBuilder { ASTContext *ASTCtx; CodeCompletion Completion; llvm::SmallVector Bundled; - bool EnableFunctionArgSnippets; + CodeCompleteOptions::PlaceholderOption PlaceholderType; // No snippets will be generated for using declarations and when the function // arguments are already present. bool IsUsingDeclaration; @@ -797,8 +804,8 @@ SpecifiedScope getQueryScopes(CodeCompletionContext &CCContext, llvm::StringRef SpelledSpecifier = Lexer::getSourceText( CharSourceRange::getCharRange(SemaSpecifier->getRange()), CCSema.SourceMgr, clang::LangOptions()); - if (SpelledSpecifier.consume_front("::")) - Scopes.QueryScopes = {""}; + if (SpelledSpecifier.consume_front("::")) + Scopes.QueryScopes = {""}; Scopes.UnresolvedQualifier = std::string(SpelledSpecifier); // Sema excludes the trailing "::". if (!Scopes.UnresolvedQualifier->empty()) @@ -1591,7 +1598,7 @@ class CodeCompleteFlow { CompletionPrefix HeuristicPrefix; std::optional Filter; // Initialized once Sema runs. Range ReplacedRange; - std::vector QueryScopes; // Initialized once Sema runs. + std::vector QueryScopes; // Initialized once Sema runs. std::vector AccessibleScopes; // Initialized once Sema runs. // Initialized once QueryScopes is initialized, if there are scopes. std::optional ScopeProximity; @@ -2387,8 +2394,7 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const CodeCompletion &C) { llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const CodeCompleteResult &R) { OS << "CodeCompleteResult: " << R.Completions.size() << (R.HasMore ? "+" : "") - << " (" << getCompletionKindString(R.Context) << ")" - << " items:\n"; + << " (" << getCompletionKindString(R.Context) << ")" << " items:\n"; for (const auto &C : R.Completions) OS << C << "\n"; return OS; diff --git a/clang-tools-extra/clangd/CodeComplete.h b/clang-tools-extra/clangd/CodeComplete.h index a7c1ae95dcbf493..fc27a1c7eefc947 100644 --- a/clang-tools-extra/clangd/CodeComplete.h +++ b/clang-tools-extra/clangd/CodeComplete.h @@ -96,9 +96,21 @@ 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; + // requested by community in favour of 'EnableFunctionArgSnippets', + // see here for more info: + // https://github.com/llvm/llvm-project/issues/63565#issuecomment-1975065771 + /// Controls how the delimter/argument-list for callables: "()" + /// and for generics: "<>" are handled + enum PlaceholderOption { + /// nothing, no argument list and also NO Delimiters "()" or "<>" + None = 0, + /// open, only opening delimiter "(" or "<" + OpenDelimiter, + /// empty pair of delimiters "()" or "<>" (or [legacy] alias 0) + Delimiters, + /// full name of both type and variable (or [legacy] alias 1) + FullPlaceholders, + } PlaceholderType = PlaceholderOption::FullPlaceholders; /// Whether to include index symbols that are not defined in the scopes /// visible from the code completion point. This applies in contexts without diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp index 3a5449ac8c79944..a186f3148196647 100644 --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -242,13 +242,31 @@ opt FallbackStyle{ init(clang::format::DefaultFallbackStyle), }; -opt EnableFunctionArgSnippets{ - "function-arg-placeholders", - cat(Features), - desc("When disabled, completions contain only parentheses for " - "function calls. When enabled, completions also contain " - "placeholders for method parameters"), - init(CodeCompleteOptions().EnableFunctionArgSnippets), +opt PlaceholderOption{ + "function-arg-placeholders", cat(Features), + desc("Set the way placeholders and delimiters are implemented."), + init(CodeCompleteOptions().PlaceholderType), + values( + clEnumValN(CodeCompleteOptions::PlaceholderOption::None, "None", + "insert nothing, not even the delimiters \"()\" or " + "\"<>\": void foo"), + clEnumValN(CodeCompleteOptions::PlaceholderOption::OpenDelimiter, + "OpenDelimiter", + "Only insert opening delimiter \"(\" or \"<\", no " + "placeholders: void foo("), + clEnumValN(CodeCompleteOptions::PlaceholderOption::Delimiters, + "Delimiters", + "Only insert delimiters \"()\" and \"<>\", no placeholders: " + "void foo()"), + clEnumValN( + CodeCompleteOptions::PlaceholderOption::FullPlaceholders, + "FullPlaceholders", + "[default] Use full type names and value names: void foo(int x)"), + clEnumValN(CodeCompleteOptions::PlaceholderOption::Delimiters, "0", + "[deprecated] use: Delimiters instead"), + clEnumValN(CodeCompleteOptions::PlaceholderOption::FullPlaceholders, + "1", "[deprecated] use: FullPlaceholders instead")) + }; opt HeaderInsertion{ @@ -916,7 +934,7 @@ 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.PlaceholderType = PlaceholderOption; Opts.CodeComplete.RunParser = CodeCompletionParse; Opts.CodeComplete.RankingModel = RankingModel; From ae6e02e5fb07e2beb115e073b13eb48d343ebc6a Mon Sep 17 00:00:00 2001 From: MK-Alias Date: Sat, 14 Sep 2024 19:48:57 +0200 Subject: [PATCH 2/6] reconfigured: issue-63565 to use .clangd. --- clang-tools-extra/clangd/CodeComplete.cpp | 28 ++++++++---- clang-tools-extra/clangd/CodeComplete.h | 21 +++------ clang-tools-extra/clangd/Config.h | 19 ++++++++ clang-tools-extra/clangd/ConfigCompile.cpp | 33 ++++++++++++-- clang-tools-extra/clangd/ConfigFragment.h | 8 ++++ clang-tools-extra/clangd/ConfigYAML.cpp | 4 ++ clang-tools-extra/clangd/tool/ClangdMain.cpp | 38 +++++++--------- .../clangd/unittests/CodeCompleteTests.cpp | 45 ++++++++++++++----- 8 files changed, 133 insertions(+), 63 deletions(-) diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 9fb264ef9160b3c..375abf6d98b9411 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -32,6 +32,7 @@ #include "Quality.h" #include "SourceCode.h" #include "URI.h" +#include "config.h" #include "index/Index.h" #include "index/Symbol.h" #include "index/SymbolOrigin.h" @@ -350,7 +351,7 @@ struct CodeCompletionBuilder { CodeCompletionContext::Kind ContextKind, const CodeCompleteOptions &Opts, bool IsUsingDeclaration, tok::TokenKind NextTokenKind) - : ASTCtx(ASTCtx), PlaceholderType(Opts.PlaceholderType), + : ASTCtx(ASTCtx), ArgumentLists(Opts.ArgumentLists), IsUsingDeclaration(IsUsingDeclaration), NextTokenKind(NextTokenKind) { Completion.Deprecated = true; // cleared by any non-deprecated overload. add(C, SemaCCS, ContextKind); @@ -560,12 +561,22 @@ struct CodeCompletionBuilder { } std::string summarizeSnippet() const { - /// localize PlaceholderType for better readability - const bool None = PlaceholderType == CodeCompleteOptions::None; - const bool Open = PlaceholderType == CodeCompleteOptions::OpenDelimiter; - const bool Delim = PlaceholderType == CodeCompleteOptions::Delimiters; + + /// localize a ArgList depending on the config. If the config is unset we + /// will use our local (this) version, else use the one of the config + const Config::ArgumentListsOption ArgList = + Config::current().Completion.ArgumentLists != + Config::ArgumentListsOption::UnsetDefault + ? Config::current().Completion.ArgumentLists + : ArgumentLists; + + /// localize ArgList value, tests for better readability + const bool None = ArgList == Config::ArgumentListsOption::None; + const bool Open = ArgList == Config::ArgumentListsOption::OpenDelimiter; + const bool Delim = ArgList == Config::ArgumentListsOption::Delimiters; const bool Full = - PlaceholderType == CodeCompleteOptions::FullPlaceholders || + ArgList == Config::ArgumentListsOption::FullPlaceholders || + ArgList == Config::ArgumentListsOption::UnsetDefault || (!None && !Open && !Delim); // <-- failsafe if (IsUsingDeclaration) @@ -631,7 +642,7 @@ struct CodeCompletionBuilder { if (Snippet->front() == '<') return None ? "" : (Open ? "<" : (EmptyArgs ? "<$1>()$0" : "<$1>($0)")); if (Snippet->front() == '(') - return None ? "" : (Open ? "(" : (EmptyArgs ? "()$0" : "($0)")); + return None ? "" : (Open ? "(" : (EmptyArgs ? "()" : "($0)")); return *Snippet; // Not an arg snippet? } // 'CompletionItemKind::Interface' matches template type aliases. @@ -661,7 +672,8 @@ struct CodeCompletionBuilder { ASTContext *ASTCtx; CodeCompletion Completion; llvm::SmallVector Bundled; - CodeCompleteOptions::PlaceholderOption PlaceholderType; + /// the way argument lists are handled. + Config::ArgumentListsOption 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 fc27a1c7eefc947..9a8db75e30d0093 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,22 +97,6 @@ struct CodeCompleteOptions { /// '->' on member access etc. bool IncludeFixIts = false; - // requested by community in favour of 'EnableFunctionArgSnippets', - // see here for more info: - // https://github.com/llvm/llvm-project/issues/63565#issuecomment-1975065771 - /// Controls how the delimter/argument-list for callables: "()" - /// and for generics: "<>" are handled - enum PlaceholderOption { - /// nothing, no argument list and also NO Delimiters "()" or "<>" - None = 0, - /// open, only opening delimiter "(" or "<" - OpenDelimiter, - /// empty pair of delimiters "()" or "<>" (or [legacy] alias 0) - Delimiters, - /// full name of both type and variable (or [legacy] alias 1) - FullPlaceholders, - } PlaceholderType = PlaceholderOption::FullPlaceholders; - /// 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. @@ -119,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::ArgumentListsOption ArgumentLists = + Config::ArgumentListsOption::UnsetDefault; + /// 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 41143b9ebc8d273..bb58566ba39652b 100644 --- a/clang-tools-extra/clangd/Config.h +++ b/clang-tools-extra/clangd/Config.h @@ -126,11 +126,29 @@ struct Config { std::vector FullyQualifiedNamespaces; } Style; + /// controls the completion options for argument lists. + enum class ArgumentListsOption { + /// the default value. This will imply FullPlaceholders unless overridden by + /// --function-arg-placeholders=0, in which case Delimiters is used. + /// any other use-case of --function-arg-placeholders is ignored + UnsetDefault = 0, + /// nothing, no argument list and also NO Delimiters "()" or "<>" + None, + /// open, only opening delimiter "(" or "<" + OpenDelimiter, + /// empty pair of delimiters "()" or "<>" (or [legacy] alias 0) + Delimiters, + /// full name of both type and variable (or [legacy] alias 1) + 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. + ArgumentListsOption ArgumentLists = ArgumentListsOption::UnsetDefault; } Completion; /// Configures hover feature. @@ -150,6 +168,7 @@ struct Config { bool BlockEnd = false; // Limit the length of type names in inlay hints. (0 means no limit) uint32_t TypeNameLimit = 32; + } InlayHints; struct { diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp index f32f674443ffeb7..9d8439844f97582 100644 --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -504,10 +504,10 @@ struct FragmentCompiler { auto Fast = isFastTidyCheck(Str); if (!Fast.has_value()) { diag(Warning, - llvm::formatv( - "Latency of clang-tidy check '{0}' is not known. " - "It will only run if ClangTidy.FastCheckFilter is Loose or None", - Str) + llvm::formatv("Latency of clang-tidy check '{0}' is not known. " + "It will only run if ClangTidy.FastCheckFilter is " + "Loose or None", + Str) .str(), Arg.Range); } else if (!*Fast) { @@ -622,6 +622,31 @@ struct FragmentCompiler { C.Completion.AllScopes = AllScopes; }); } + if (F.ArgumentLists) { + if (**F.ArgumentLists == "None") { + Out.Apply.push_back( + [ArgumentLists(**F.ArgumentLists)](const Params &, Config &C) { + C.Completion.ArgumentLists = Config::ArgumentListsOption::None; + }); + } else if (**F.ArgumentLists == "OpenDelimiter") { + Out.Apply.push_back( + [ArgumentLists(**F.ArgumentLists)](const Params &, Config &C) { + C.Completion.ArgumentLists = + Config::ArgumentListsOption::OpenDelimiter; + }); + } else if (**F.ArgumentLists == "Delimiters") { + Out.Apply.push_back([ArgumentLists(**F.ArgumentLists)](const Params &, + Config &C) { + C.Completion.ArgumentLists = Config::ArgumentListsOption::Delimiters; + }); + } else if (**F.ArgumentLists == "FullPlaceholders") { + Out.Apply.push_back( + [ArgumentLists(**F.ArgumentLists)](const Params &, Config &C) { + C.Completion.ArgumentLists = + Config::ArgumentListsOption::FullPlaceholders; + }); + } + } } void compile(Fragment::HoverBlock &&F) { diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h index f3e51a9b6dbc4ba..54525e86aa82caa 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> AllScopes; + /// How to present the argument list between '()' and '<>': + /// valid values are enum Config::ArgumentListsOption values: + /// None: Nothing at all + /// OpenDelimiter: only opening delimiter "(" or "<" + /// Delimiters: empty pair of delimiters "()" or "<>" + /// FullPlaceholders: full name of both type and variable + std::optional> ArgumentLists; }; CompletionBlock Completion; diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp index 3e9b6a07d3b325f..bcdda99eeed67a8 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 a186f3148196647..8bac2ce6aacfdd6 100644 --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -242,30 +242,15 @@ opt FallbackStyle{ init(clang::format::DefaultFallbackStyle), }; -opt PlaceholderOption{ +opt PlaceholderOption{ "function-arg-placeholders", cat(Features), desc("Set the way placeholders and delimiters are implemented."), - init(CodeCompleteOptions().PlaceholderType), - values( - clEnumValN(CodeCompleteOptions::PlaceholderOption::None, "None", - "insert nothing, not even the delimiters \"()\" or " - "\"<>\": void foo"), - clEnumValN(CodeCompleteOptions::PlaceholderOption::OpenDelimiter, - "OpenDelimiter", - "Only insert opening delimiter \"(\" or \"<\", no " - "placeholders: void foo("), - clEnumValN(CodeCompleteOptions::PlaceholderOption::Delimiters, - "Delimiters", - "Only insert delimiters \"()\" and \"<>\", no placeholders: " - "void foo()"), - clEnumValN( - CodeCompleteOptions::PlaceholderOption::FullPlaceholders, - "FullPlaceholders", - "[default] Use full type names and value names: void foo(int x)"), - clEnumValN(CodeCompleteOptions::PlaceholderOption::Delimiters, "0", - "[deprecated] use: Delimiters instead"), - clEnumValN(CodeCompleteOptions::PlaceholderOption::FullPlaceholders, - "1", "[deprecated] use: FullPlaceholders instead")) + init(CodeCompleteOptions().ArgumentLists), + values(clEnumValN(Config::ArgumentListsOption::Delimiters, "0", + "Empty pair of delimiters inserted"), + clEnumValN( + Config::ArgumentListsOption::FullPlaceholders, "1", + "[default] Delimiters and placeholder arguments are inserted.")) }; @@ -668,6 +653,7 @@ class FlagsConfigProvider : public config::Provider { std::optional CDBSearch; std::optional IndexSpec; std::optional BGPolicy; + std::optional ArgumentLists; // If --compile-commands-dir arg was invoked, check value and override // default path. @@ -712,6 +698,10 @@ class FlagsConfigProvider : public config::Provider { BGPolicy = Config::BackgroundPolicy::Skip; } + if (PlaceholderOption == Config::ArgumentListsOption::Delimiters) { + ArgumentLists = PlaceholderOption; + } + Frag = [=](const config::Params &, Config &C) { if (CDBSearch) C.CompileFlags.CDBSearch = *CDBSearch; @@ -719,6 +709,9 @@ class FlagsConfigProvider : public config::Provider { C.Index.External = *IndexSpec; if (BGPolicy) C.Index.Background = *BGPolicy; + if (ArgumentLists && C.Completion.ArgumentLists == + Config::ArgumentListsOption::UnsetDefault) + C.Completion.ArgumentLists = *ArgumentLists; if (AllScopesCompletion.getNumOccurrences()) C.Completion.AllScopes = AllScopesCompletion; @@ -934,7 +927,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.PlaceholderType = PlaceholderOption; 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 96d1ee1f0add735..35863e50e4a53d9 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -6,6 +6,7 @@ // //===----------------------------------------------------------------------===// +#include "../config.h" #include "ASTSignals.h" #include "Annotations.h" #include "ClangdServer.h" @@ -1137,8 +1138,8 @@ TEST(CodeCompleteTest, NoColonColonAtTheEnd) { } TEST(CompletionTests, EmptySnippetDoesNotCrash) { - // See https://github.com/clangd/clangd/issues/1216 - auto Results = completions(R"cpp( + // See https://github.com/clangd/clangd/issues/1216 + auto Results = completions(R"cpp( int main() { auto w = [&](auto &&f) { return f(f); }; auto f = w([&](auto &&f) { @@ -1154,18 +1155,18 @@ TEST(CompletionTests, EmptySnippetDoesNotCrash) { } TEST(CompletionTest, Issue1427Crash) { - // Need to provide main file signals to ensure that the branch in - // SymbolRelevanceSignals::computeASTSignals() that tries to - // compute a symbol ID is taken. - ASTSignals MainFileSignals; - CodeCompleteOptions Opts; - Opts.MainFileSignals = &MainFileSignals; - completions(R"cpp( + // Need to provide main file signals to ensure that the branch in + // SymbolRelevanceSignals::computeASTSignals() that tries to + // compute a symbol ID is taken. + ASTSignals MainFileSignals; + CodeCompleteOptions Opts; + Opts.MainFileSignals = &MainFileSignals; + completions(R"cpp( auto f = []() { 1.0_^ }; )cpp", - {}, Opts); + {}, Opts); } TEST(CompletionTest, BacktrackCrashes) { @@ -2595,10 +2596,10 @@ TEST(SignatureHelpTest, DynamicIndexDocumentation) { ElementsAre(AllOf(sig("foo() -> int"), sigDoc("Member doc")))); } -TEST(CompletionTest, CompletionFunctionArgsDisabled) { +TEST(CompletionTest, ArgumentListsNotFullPlaceholders) { CodeCompleteOptions Opts; Opts.EnableSnippets = true; - Opts.EnableFunctionArgSnippets = false; + Opts.ArgumentLists = Config::ArgumentListsOption::Delimiters; { auto Results = completions( @@ -2670,6 +2671,26 @@ TEST(CompletionTest, CompletionFunctionArgsDisabled) { EXPECT_THAT(Results.Completions, UnorderedElementsAre(AllOf( named("FOO"), snippetSuffix("($0)")))); } + { + Opts.ArgumentLists = Config::ArgumentListsOption::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::ArgumentListsOption::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) { From 9e67e06dfd36b9179d53f80ded09438f2e46165a Mon Sep 17 00:00:00 2001 From: MK-Alias Date: Sat, 14 Sep 2024 20:13:54 +0200 Subject: [PATCH 3/6] issue-63565: fixed a case-sensitive #include typo. --- clang-tools-extra/clangd/CodeComplete.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 375abf6d98b9411..a18947a2a9774c2 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" @@ -32,7 +33,6 @@ #include "Quality.h" #include "SourceCode.h" #include "URI.h" -#include "config.h" #include "index/Index.h" #include "index/Symbol.h" #include "index/SymbolOrigin.h" From 6e31a070f087ee6206234afb116ed87e0ee2f7a3 Mon Sep 17 00:00:00 2001 From: MK-Alias Date: Sat, 14 Sep 2024 20:25:52 +0200 Subject: [PATCH 4/6] issue-63565: fixed another case-sensitive #include typo. --- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 35863e50e4a53d9..ab88397a5c05029 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -6,7 +6,7 @@ // //===----------------------------------------------------------------------===// -#include "../config.h" +#include "../Config.h" #include "ASTSignals.h" #include "Annotations.h" #include "ClangdServer.h" From 49ca6ddee1003aaf58f076da5c2ee6767acbf939 Mon Sep 17 00:00:00 2001 From: MK-Alias Date: Wed, 18 Sep 2024 16:29:03 +0200 Subject: [PATCH 5/6] issue-63565: reconfigured to given change requests. --- clang-tools-extra/clangd/ClangdServer.cpp | 3 +- clang-tools-extra/clangd/CodeComplete.cpp | 25 ++++------- clang-tools-extra/clangd/CodeComplete.h | 4 +- clang-tools-extra/clangd/Config.h | 16 +++---- clang-tools-extra/clangd/ConfigCompile.cpp | 42 +++++++------------ clang-tools-extra/clangd/ConfigFragment.h | 4 +- clang-tools-extra/clangd/tool/ClangdMain.cpp | 26 +++++------- .../clangd/unittests/CodeCompleteTests.cpp | 10 ++--- 8 files changed, 52 insertions(+), 78 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index e910a80ba0bae9b..547f988b8731a05 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -64,7 +64,7 @@ namespace { // Tracks number of times a tweak has been offered. static constexpr trace::Metric TweakAvailable( "tweak_available", trace::Metric::Counter, "tweak_id"); - + // Update the FileIndex with new ASTs and plumb the diagnostics responses. struct UpdateIndexCallbacks : public ParsingCallbacks { UpdateIndexCallbacks(FileIndex *FIndex, @@ -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 a18947a2a9774c2..caaed5670cd5c15 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -561,23 +561,14 @@ struct CodeCompletionBuilder { } std::string summarizeSnippet() const { - - /// localize a ArgList depending on the config. If the config is unset we - /// will use our local (this) version, else use the one of the config - const Config::ArgumentListsOption ArgList = - Config::current().Completion.ArgumentLists != - Config::ArgumentListsOption::UnsetDefault - ? Config::current().Completion.ArgumentLists - : ArgumentLists; - - /// localize ArgList value, tests for better readability - const bool None = ArgList == Config::ArgumentListsOption::None; - const bool Open = ArgList == Config::ArgumentListsOption::OpenDelimiter; - const bool Delim = ArgList == Config::ArgumentListsOption::Delimiters; + /// 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 = - ArgList == Config::ArgumentListsOption::FullPlaceholders || - ArgList == Config::ArgumentListsOption::UnsetDefault || - (!None && !Open && !Delim); // <-- failsafe + ArgumentLists == Config::ArgumentListsPolicy::FullPlaceholders || + (!None && !Open && !Delim); // <-- failsafe: Full is default if (IsUsingDeclaration) return ""; @@ -673,7 +664,7 @@ struct CodeCompletionBuilder { CodeCompletion Completion; llvm::SmallVector Bundled; /// the way argument lists are handled. - Config::ArgumentListsOption ArgumentLists; + 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 9a8db75e30d0093..2628f9edafe85db 100644 --- a/clang-tools-extra/clangd/CodeComplete.h +++ b/clang-tools-extra/clangd/CodeComplete.h @@ -105,8 +105,8 @@ struct CodeCompleteOptions { bool AllScopes = false; /// The way argument list on calls '()' and generics '<>' are handled. - Config::ArgumentListsOption ArgumentLists = - Config::ArgumentListsOption::UnsetDefault; + 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). diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h index bb58566ba39652b..d1db9fcbe893376 100644 --- a/clang-tools-extra/clangd/Config.h +++ b/clang-tools-extra/clangd/Config.h @@ -127,18 +127,14 @@ struct Config { } Style; /// controls the completion options for argument lists. - enum class ArgumentListsOption { - /// the default value. This will imply FullPlaceholders unless overridden by - /// --function-arg-placeholders=0, in which case Delimiters is used. - /// any other use-case of --function-arg-placeholders is ignored - UnsetDefault = 0, - /// nothing, no argument list and also NO Delimiters "()" or "<>" + enum class ArgumentListsPolicy { + /// nothing, no argument list and also NO Delimiters "()" or "<>". None, - /// open, only opening delimiter "(" or "<" + /// open, only opening delimiter "(" or "<". OpenDelimiter, - /// empty pair of delimiters "()" or "<>" (or [legacy] alias 0) + /// empty pair of delimiters "()" or "<>". Delimiters, - /// full name of both type and variable (or [legacy] alias 1) + /// full name of both type and variable. FullPlaceholders, }; @@ -148,7 +144,7 @@ struct Config { /// scopes. bool AllScopes = true; /// controls the completion options for argument lists. - ArgumentListsOption ArgumentLists = ArgumentListsOption::UnsetDefault; + 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 9d8439844f97582..58610a5b87922de 100644 --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -504,10 +504,10 @@ struct FragmentCompiler { auto Fast = isFastTidyCheck(Str); if (!Fast.has_value()) { diag(Warning, - llvm::formatv("Latency of clang-tidy check '{0}' is not known. " - "It will only run if ClangTidy.FastCheckFilter is " - "Loose or None", - Str) + llvm::formatv( + "Latency of clang-tidy check '{0}' is not known. " + "It will only run if ClangTidy.FastCheckFilter is Loose or None", + Str) .str(), Arg.Range); } else if (!*Fast) { @@ -623,29 +623,19 @@ struct FragmentCompiler { }); } if (F.ArgumentLists) { - if (**F.ArgumentLists == "None") { - Out.Apply.push_back( - [ArgumentLists(**F.ArgumentLists)](const Params &, Config &C) { - C.Completion.ArgumentLists = Config::ArgumentListsOption::None; - }); - } else if (**F.ArgumentLists == "OpenDelimiter") { - Out.Apply.push_back( - [ArgumentLists(**F.ArgumentLists)](const Params &, Config &C) { - C.Completion.ArgumentLists = - Config::ArgumentListsOption::OpenDelimiter; - }); - } else if (**F.ArgumentLists == "Delimiters") { - Out.Apply.push_back([ArgumentLists(**F.ArgumentLists)](const Params &, - Config &C) { - C.Completion.ArgumentLists = Config::ArgumentListsOption::Delimiters; + if (auto Val = + compileEnum("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; }); - } else if (**F.ArgumentLists == "FullPlaceholders") { - Out.Apply.push_back( - [ArgumentLists(**F.ArgumentLists)](const Params &, Config &C) { - C.Completion.ArgumentLists = - Config::ArgumentListsOption::FullPlaceholders; - }); - } } } diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h index 54525e86aa82caa..fc1b45f5d4c3e9e 100644 --- a/clang-tools-extra/clangd/ConfigFragment.h +++ b/clang-tools-extra/clangd/ConfigFragment.h @@ -310,11 +310,11 @@ struct Fragment { /// not visible. The required scope prefix will be inserted. std::optional> AllScopes; /// How to present the argument list between '()' and '<>': - /// valid values are enum Config::ArgumentListsOption values: + /// 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 variable + /// FullPlaceholders: full name of both type and parameter std::optional> ArgumentLists; }; CompletionBlock Completion; diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp index 8bac2ce6aacfdd6..53ef12b77b8cc3d 100644 --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -242,16 +242,13 @@ opt FallbackStyle{ init(clang::format::DefaultFallbackStyle), }; -opt PlaceholderOption{ - "function-arg-placeholders", cat(Features), - desc("Set the way placeholders and delimiters are implemented."), - init(CodeCompleteOptions().ArgumentLists), - values(clEnumValN(Config::ArgumentListsOption::Delimiters, "0", - "Empty pair of delimiters inserted"), - clEnumValN( - Config::ArgumentListsOption::FullPlaceholders, "1", - "[default] Delimiters and placeholder arguments are inserted.")) - +opt EnableFunctionArgSnippets{ + "function-arg-placeholders", + cat(Features), + desc("When disabled, completions contain only parentheses for " + "function calls. When enabled, completions also contain " + "placeholders for method parameters"), + init(true), }; opt HeaderInsertion{ @@ -653,7 +650,7 @@ class FlagsConfigProvider : public config::Provider { std::optional CDBSearch; std::optional IndexSpec; std::optional BGPolicy; - std::optional ArgumentLists; + std::optional ArgumentLists; // If --compile-commands-dir arg was invoked, check value and override // default path. @@ -698,8 +695,8 @@ class FlagsConfigProvider : public config::Provider { BGPolicy = Config::BackgroundPolicy::Skip; } - if (PlaceholderOption == Config::ArgumentListsOption::Delimiters) { - ArgumentLists = PlaceholderOption; + if (!EnableFunctionArgSnippets) { + ArgumentLists = Config::ArgumentListsPolicy::Delimiters; } Frag = [=](const config::Params &, Config &C) { @@ -709,8 +706,7 @@ class FlagsConfigProvider : public config::Provider { C.Index.External = *IndexSpec; if (BGPolicy) C.Index.Background = *BGPolicy; - if (ArgumentLists && C.Completion.ArgumentLists == - Config::ArgumentListsOption::UnsetDefault) + if (ArgumentLists) C.Completion.ArgumentLists = *ArgumentLists; if (AllScopesCompletion.getNumOccurrences()) C.Completion.AllScopes = AllScopesCompletion; diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index ab88397a5c05029..917431b6520dcf6 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -6,12 +6,12 @@ // //===----------------------------------------------------------------------===// -#include "../Config.h" #include "ASTSignals.h" #include "Annotations.h" #include "ClangdServer.h" #include "CodeComplete.h" #include "Compiler.h" +#include "Config.h" #include "Feature.h" #include "Matchers.h" #include "Protocol.h" @@ -2596,10 +2596,10 @@ TEST(SignatureHelpTest, DynamicIndexDocumentation) { ElementsAre(AllOf(sig("foo() -> int"), sigDoc("Member doc")))); } -TEST(CompletionTest, ArgumentListsNotFullPlaceholders) { +TEST(CompletionTest, ArgumentListsPolicy) { CodeCompleteOptions Opts; Opts.EnableSnippets = true; - Opts.ArgumentLists = Config::ArgumentListsOption::Delimiters; + Opts.ArgumentLists = Config::ArgumentListsPolicy::Delimiters; { auto Results = completions( @@ -2672,7 +2672,7 @@ TEST(CompletionTest, ArgumentListsNotFullPlaceholders) { named("FOO"), snippetSuffix("($0)")))); } { - Opts.ArgumentLists = Config::ArgumentListsOption::None; + Opts.ArgumentLists = Config::ArgumentListsPolicy::None; auto Results = completions( R"cpp( void xfoo(int x, int y); @@ -2682,7 +2682,7 @@ TEST(CompletionTest, ArgumentListsNotFullPlaceholders) { UnorderedElementsAre(AllOf(named("xfoo"), snippetSuffix("")))); } { - Opts.ArgumentLists = Config::ArgumentListsOption::OpenDelimiter; + Opts.ArgumentLists = Config::ArgumentListsPolicy::OpenDelimiter; auto Results = completions( R"cpp( void xfoo(int x, int y); From e222a846376ff5a8676101f34358b021c330f1d8 Mon Sep 17 00:00:00 2001 From: MK-Alias Date: Sun, 22 Sep 2024 09:42:11 +0200 Subject: [PATCH 6/6] issue-63565: using int value on argument & properly formatted. --- clang-tools-extra/clangd/ClangdServer.cpp | 6 +++--- clang-tools-extra/clangd/ConfigCompile.cpp | 8 ++++---- clang-tools-extra/clangd/tool/ClangdMain.cpp | 14 ++++++++------ 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 547f988b8731a05..03f5d133d362e5b 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -62,9 +62,9 @@ namespace clangd { namespace { // Tracks number of times a tweak has been offered. -static constexpr trace::Metric TweakAvailable( - "tweak_available", trace::Metric::Counter, "tweak_id"); - +static constexpr trace::Metric + TweakAvailable("tweak_available", trace::Metric::Counter, "tweak_id"); + // Update the FileIndex with new ASTs and plumb the diagnostics responses. struct UpdateIndexCallbacks : public ParsingCallbacks { UpdateIndexCallbacks(FileIndex *FIndex, diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp index 58610a5b87922de..3551a371124a508 100644 --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -504,10 +504,10 @@ struct FragmentCompiler { auto Fast = isFastTidyCheck(Str); if (!Fast.has_value()) { diag(Warning, - llvm::formatv( - "Latency of clang-tidy check '{0}' is not known. " - "It will only run if ClangTidy.FastCheckFilter is Loose or None", - Str) + llvm::formatv("Latency of clang-tidy check '{0}' is not known. " + "It will only run if ClangTidy.FastCheckFilter is " + "Loose or None", + Str) .str(), Arg.Range); } else if (!*Fast) { diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp index 53ef12b77b8cc3d..0bf2916278b6395 100644 --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -242,13 +242,13 @@ opt FallbackStyle{ init(clang::format::DefaultFallbackStyle), }; -opt EnableFunctionArgSnippets{ +opt 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(true), + init(-1), }; opt HeaderInsertion{ @@ -695,8 +695,10 @@ class FlagsConfigProvider : public config::Provider { BGPolicy = Config::BackgroundPolicy::Skip; } - if (!EnableFunctionArgSnippets) { - ArgumentLists = Config::ArgumentListsPolicy::Delimiters; + if (EnableFunctionArgSnippets >= 0) { + ArgumentLists = EnableFunctionArgSnippets + ? Config::ArgumentListsPolicy::FullPlaceholders + : Config::ArgumentListsPolicy::Delimiters; } Frag = [=](const config::Params &, Config &C) {