https://github.com/MythreyaK updated https://github.com/llvm/llvm-project/pull/128503
>From 60ffbf0067b52cf51b5ce1d8eaf98cd2a2737c6a Mon Sep 17 00:00:00 2001 From: Mythreya <g...@mythreya.dev> Date: Mon, 24 Feb 2025 04:34:38 -0800 Subject: [PATCH 1/2] [clangd] Add `HeaderInsertion` yaml config option This is the yaml config equivalent of `--header-insertion` CLI option --- clang-tools-extra/clangd/ClangdServer.cpp | 1 + clang-tools-extra/clangd/CodeComplete.cpp | 3 ++- clang-tools-extra/clangd/CodeComplete.h | 6 ++---- clang-tools-extra/clangd/Config.h | 9 +++++++++ clang-tools-extra/clangd/ConfigCompile.cpp | 11 +++++++++++ clang-tools-extra/clangd/ConfigFragment.h | 8 ++++++++ clang-tools-extra/clangd/ConfigYAML.cpp | 4 ++++ clang-tools-extra/clangd/tool/ClangdMain.cpp | 4 ++-- .../clangd/unittests/CodeCompleteTests.cpp | 3 ++- 9 files changed, 41 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 49a97da2bfa42..47152b7c36140 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -455,6 +455,7 @@ void ClangdServer::codeComplete(PathRef File, Position Pos, CodeCompleteOpts.MainFileSignals = IP->Signals; CodeCompleteOpts.AllScopes = Config::current().Completion.AllScopes; CodeCompleteOpts.ArgumentLists = Config::current().Completion.ArgumentLists; + CodeCompleteOpts.InsertIncludes = Config::current().HeaderInsertion.Policy; // 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 a8182ce98ebe0..5db8eeaee1027 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -294,7 +294,8 @@ struct CompletionCandidate { std::optional<llvm::StringRef> headerToInsertIfAllowed(const CodeCompleteOptions &Opts, CodeCompletionContext::Kind ContextKind) const { - if (Opts.InsertIncludes == CodeCompleteOptions::NeverInsert || + if (Opts.InsertIncludes == + CodeCompleteOptions::IncludeInsertion::NeverInsert || RankedIncludeHeaders.empty() || !contextAllowsHeaderInsertion(ContextKind)) return std::nullopt; diff --git a/clang-tools-extra/clangd/CodeComplete.h b/clang-tools-extra/clangd/CodeComplete.h index cd41f04e4fb5c..f1e4ebae10ebb 100644 --- a/clang-tools-extra/clangd/CodeComplete.h +++ b/clang-tools-extra/clangd/CodeComplete.h @@ -71,10 +71,8 @@ struct CodeCompleteOptions { /// Whether to present doc comments as plain-text or markdown. MarkupKind DocumentationFormat = MarkupKind::PlainText; - enum IncludeInsertion { - IWYU, - NeverInsert, - } InsertIncludes = IncludeInsertion::IWYU; + using IncludeInsertion = Config::HeaderInsertionPolicy; + Config::HeaderInsertionPolicy InsertIncludes = IncludeInsertion::IWYU; /// Whether include insertions for Objective-C code should use #import instead /// of #include. diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h index 3f8a3c9b060f6..e6e4e446ba3e7 100644 --- a/clang-tools-extra/clangd/Config.h +++ b/clang-tools-extra/clangd/Config.h @@ -156,6 +156,15 @@ struct Config { ArgumentListsPolicy ArgumentLists = ArgumentListsPolicy::FullPlaceholders; } Completion; + enum class HeaderInsertionPolicy { + IWYU, // Include what you use + NeverInsert // Never insert headers as part of code completion + }; + + struct { + HeaderInsertionPolicy Policy = HeaderInsertionPolicy::IWYU; + } HeaderInsertion; + /// Configures hover feature. struct { /// Whether hover show a.k.a type. diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp index 31530c206acd7..d57020eaf7c83 100644 --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -697,6 +697,17 @@ struct FragmentCompiler { C.Completion.ArgumentLists = *Val; }); } + if (F.HeaderInsertion) { + if (auto Val = + compileEnum<Config::HeaderInsertionPolicy>("HeaderInsertion", + *F.HeaderInsertion) + .map("IWYU", Config::HeaderInsertionPolicy::IWYU) + .map("Never", Config::HeaderInsertionPolicy::NeverInsert) + .value()) + Out.Apply.push_back([Val](const Params &, Config &C) { + C.HeaderInsertion.Policy = *Val; + }); + } } void compile(Fragment::HoverBlock &&F) { diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h index 6f95474b9c008..f05ed4d1acdfc 100644 --- a/clang-tools-extra/clangd/ConfigFragment.h +++ b/clang-tools-extra/clangd/ConfigFragment.h @@ -341,6 +341,14 @@ struct Fragment { /// Delimiters: empty pair of delimiters "()" or "<>" /// FullPlaceholders: full name of both type and parameter std::optional<Located<std::string>> ArgumentLists; + /// Add #include directives when accepting code completions. Config + /// equivalent of the CLI option '--header-insertion' + /// Valid values are enum Config::HeaderInsertionPolicy values: + /// "IWYU": Include what you use. Insert the owning header for top-level + /// symbols, unless the header is already directly included or the + /// symbol is forward-declared + /// "NeverInsert": Never insert headers + std::optional<Located<std::string>> HeaderInsertion; }; CompletionBlock Completion; diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp index a46d45df0d319..47c6e1cd0f7e7 100644 --- a/clang-tools-extra/clangd/ConfigYAML.cpp +++ b/clang-tools-extra/clangd/ConfigYAML.cpp @@ -245,6 +245,10 @@ class Parser { if (auto ArgumentLists = scalarValue(N, "ArgumentLists")) F.ArgumentLists = *ArgumentLists; }); + Dict.handle("HeaderInsertion", [&](Node &N) { + if (auto HeaderInsertion = scalarValue(N, "HeaderInsertion")) + F.HeaderInsertion = *HeaderInsertion; + }); Dict.parse(N); } diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp index 714891703b6f3..44fd1d450ec64 100644 --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -257,13 +257,13 @@ opt<CodeCompleteOptions::IncludeInsertion> HeaderInsertion{ desc("Add #include directives when accepting code completions"), init(CodeCompleteOptions().InsertIncludes), values( - clEnumValN(CodeCompleteOptions::IWYU, "iwyu", + clEnumValN(CodeCompleteOptions::IncludeInsertion::IWYU, "iwyu", "Include what you use. " "Insert the owning header for top-level symbols, unless the " "header is already directly included or the symbol is " "forward-declared"), clEnumValN( - CodeCompleteOptions::NeverInsert, "never", + CodeCompleteOptions::IncludeInsertion::NeverInsert, "never", "Never insert #include directives as part of code completion")), }; diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index b12f8275b8a26..042ad73b8a280 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -882,7 +882,8 @@ TEST(CompletionTest, IncludeInsertionPreprocessorIntegrationTests) { ElementsAre(AllOf(named("X"), insertInclude("\"bar.h\"")))); // Can be disabled via option. CodeCompleteOptions NoInsertion; - NoInsertion.InsertIncludes = CodeCompleteOptions::NeverInsert; + NoInsertion.InsertIncludes = + CodeCompleteOptions::IncludeInsertion::NeverInsert; Results = completions(TU, Test.point(), {Sym}, NoInsertion); EXPECT_THAT(Results.Completions, ElementsAre(AllOf(named("X"), Not(insertInclude())))); >From b9710d1f1701613ed41b1b4d5fcda1deab2c6608 Mon Sep 17 00:00:00 2001 From: Mythreya <g...@mythreya.dev> Date: Sun, 16 Mar 2025 22:58:36 -0700 Subject: [PATCH 2/2] Address review comments --- clang-tools-extra/clangd/ClangdServer.cpp | 3 ++- clang-tools-extra/clangd/Config.h | 16 +++++++--------- clang-tools-extra/clangd/ConfigCompile.cpp | 2 +- clang-tools-extra/clangd/tool/ClangdMain.cpp | 8 ++++++++ 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 47152b7c36140..52844129834c3 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -455,7 +455,8 @@ void ClangdServer::codeComplete(PathRef File, Position Pos, CodeCompleteOpts.MainFileSignals = IP->Signals; CodeCompleteOpts.AllScopes = Config::current().Completion.AllScopes; CodeCompleteOpts.ArgumentLists = Config::current().Completion.ArgumentLists; - CodeCompleteOpts.InsertIncludes = Config::current().HeaderInsertion.Policy; + CodeCompleteOpts.InsertIncludes = + Config::current().Completion.HeaderInsertion; // 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/Config.h b/clang-tools-extra/clangd/Config.h index e6e4e446ba3e7..2891a6d1e77b0 100644 --- a/clang-tools-extra/clangd/Config.h +++ b/clang-tools-extra/clangd/Config.h @@ -147,6 +147,11 @@ struct Config { FullPlaceholders, }; + enum class HeaderInsertionPolicy { + IWYU, // Include what you use + NeverInsert // Never insert headers as part of code completion + }; + /// Configures code completion feature. struct { /// Whether code completion includes results that are not visible in current @@ -154,17 +159,10 @@ struct Config { bool AllScopes = true; /// controls the completion options for argument lists. ArgumentListsPolicy ArgumentLists = ArgumentListsPolicy::FullPlaceholders; + /// Controls if headers should be inserted when completions are accepted + HeaderInsertionPolicy HeaderInsertion = HeaderInsertionPolicy::IWYU; } Completion; - enum class HeaderInsertionPolicy { - IWYU, // Include what you use - NeverInsert // Never insert headers as part of code completion - }; - - struct { - HeaderInsertionPolicy Policy = HeaderInsertionPolicy::IWYU; - } HeaderInsertion; - /// Configures hover feature. struct { /// Whether hover show a.k.a type. diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp index d57020eaf7c83..21304a8c0fac7 100644 --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -705,7 +705,7 @@ struct FragmentCompiler { .map("Never", Config::HeaderInsertionPolicy::NeverInsert) .value()) Out.Apply.push_back([Val](const Params &, Config &C) { - C.HeaderInsertion.Policy = *Val; + C.Completion.HeaderInsertion = *Val; }); } } diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp index 44fd1d450ec64..cb271a56185b5 100644 --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -668,6 +668,7 @@ class FlagsConfigProvider : public config::Provider { std::optional<Config::ExternalIndexSpec> IndexSpec; std::optional<Config::BackgroundPolicy> BGPolicy; std::optional<Config::ArgumentListsPolicy> ArgumentLists; + std::optional<Config::HeaderInsertionPolicy> _HeaderInsertion; // If --compile-commands-dir arg was invoked, check value and override // default path. @@ -712,6 +713,11 @@ class FlagsConfigProvider : public config::Provider { BGPolicy = Config::BackgroundPolicy::Skip; } + // If CLI has set never, use that regardless of what the config files have + if (HeaderInsertion == CodeCompleteOptions::IncludeInsertion::NeverInsert) { + _HeaderInsertion = CodeCompleteOptions::IncludeInsertion::NeverInsert; + } + if (std::optional<bool> Enable = shouldEnableFunctionArgSnippets()) { ArgumentLists = *Enable ? Config::ArgumentListsPolicy::FullPlaceholders : Config::ArgumentListsPolicy::Delimiters; @@ -726,6 +732,8 @@ class FlagsConfigProvider : public config::Provider { C.Index.Background = *BGPolicy; if (ArgumentLists) C.Completion.ArgumentLists = *ArgumentLists; + if (_HeaderInsertion) + C.Completion.HeaderInsertion = *_HeaderInsertion; if (AllScopesCompletion.getNumOccurrences()) C.Completion.AllScopes = AllScopesCompletion; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits