diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -11,6 +11,7 @@ #include "../clang-tidy/ClangTidyOptions.h" #include "CodeComplete.h" +#include "ConfigProvider.h" #include "GlobalCompilationDatabase.h" #include "Hover.h" #include "Protocol.h" @@ -113,6 +114,9 @@ /// If set, use this index to augment code completion results. SymbolIndex *StaticIndex = nullptr; + /// If set, queried to obtain the configuration to handle each request. + config::Provider *ConfigProvider = nullptr; + /// If set, enable clang-tidy in clangd and use to it get clang-tidy /// configurations for a particular file. /// Clangd supports only a small subset of ClangTidyOptions, these options @@ -326,6 +330,15 @@ ArrayRef Ranges, Callback CB); + /// Derives a context for a task processing the specified source file. + /// This includes the current configuration (see Options::ConfigProvider). + /// The empty string means no particular file is the target. + /// Rather than called by each feature, this is exposed to the components + /// that control worker threads, like TUScheduler and BackgroundIndex. + /// This means it's OK to do some IO here, and it cuts across all features. + Context createProcessingContext(PathRef) const; + config::Provider *ConfigProvider = nullptr; + const ThreadsafeFS &TFS; Path ResourceDir; diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -8,6 +8,7 @@ #include "ClangdServer.h" #include "CodeComplete.h" +#include "Config.h" #include "FindSymbols.h" #include "Format.h" #include "HeaderSourceSwitch.h" @@ -132,7 +133,7 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB, const ThreadsafeFS &TFS, const Options &Opts, Callbacks *Callbacks) - : TFS(TFS), + : ConfigProvider(Opts.ConfigProvider), TFS(TFS), DynamicIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex(Opts.HeavyweightDynamicSymbolIndex) : nullptr), @@ -147,7 +148,14 @@ // FIXME(ioeric): this can be slow and we may be able to index on less // critical paths. WorkScheduler( - CDB, TUScheduler::Options(Opts), + CDB, + [&, this] { + TUScheduler::Options O(Opts); + O.ContextProvider = [this](PathRef P) { + return createProcessingContext(P); + }; + return O; + }(), std::make_unique( DynamicIdx.get(), Callbacks, Opts.TheiaSemanticHighlighting)) { // Adds an index to the stack, at higher priority than existing indexes. @@ -170,7 +178,8 @@ [Callbacks](BackgroundQueue::Stats S) { if (Callbacks) Callbacks->onBackgroundIndexProgress(S); - }); + }, + [this](PathRef P) { return createProcessingContext(P); }); AddIndex(BackgroundIdx.get()); } if (DynamicIdx) @@ -335,7 +344,7 @@ Result.push_back(replacementToEdit(Code, R)); return CB(Result); }; - WorkScheduler.run("FormatOnType", std::move(Action)); + WorkScheduler.run("FormatOnType", File, std::move(Action)); } void ClangdServer::prepareRename(PathRef File, Position Pos, @@ -585,7 +594,7 @@ tooling::calculateRangesAfterReplacements(IncludeReplaces, Ranges), File))); }; - WorkScheduler.run("Format", std::move(Action)); + WorkScheduler.run("Format", File, std::move(Action)); } void ClangdServer::findDocumentHighlights( @@ -646,7 +655,7 @@ llvm::StringRef Query, int Limit, Callback> CB) { WorkScheduler.run( - "getWorkspaceSymbols", + "getWorkspaceSymbols", /*Path=*/"", [Query = Query.str(), Limit, CB = std::move(CB), this]() mutable { CB(clangd::getWorkspaceSymbols(Query, Limit, Index, WorkspaceRoot.getValueOr(""))); @@ -736,6 +745,31 @@ return WorkScheduler.fileStats(); } +Context ClangdServer::createProcessingContext(PathRef File) const { + if (!ConfigProvider) + return Context::current().clone(); + + config::Params Params; + if (!File.empty()) { + assert(llvm::sys::path::is_absolute(File)); + llvm::SmallString<256> PosixPath = File; + llvm::sys::path::native(PosixPath, llvm::sys::path::Style::posix); + Params.Path = PosixPath.str(); + } + + auto DiagnosticHandler = [](const llvm::SMDiagnostic &Diag) { + if (Diag.getKind() == llvm::SourceMgr::DK_Error) { + elog("config error at {0}:{1}:{2}: {3}", Diag.getFilename(), + Diag.getLineNo(), Diag.getColumnNo(), Diag.getMessage()); + } else { + log("config warning at {0}:{1}:{2}: {3}", Diag.getFilename(), + Diag.getLineNo(), Diag.getColumnNo(), Diag.getMessage()); + } + }; + Config C = ConfigProvider->getConfig(Params, DiagnosticHandler); + return Context::current().derive(Config::Key, std::move(C)); +} + LLVM_NODISCARD bool ClangdServer::blockUntilIdleForTest(llvm::Optional TimeoutSeconds) { return WorkScheduler.blockUntilIdle(timeoutSeconds(TimeoutSeconds)) && diff --git a/clang-tools-extra/clangd/TUScheduler.h b/clang-tools-extra/clangd/TUScheduler.h --- a/clang-tools-extra/clangd/TUScheduler.h +++ b/clang-tools-extra/clangd/TUScheduler.h @@ -195,6 +195,11 @@ /// Whether to run PreamblePeer asynchronously. /// No-op if AsyncThreadsCount is 0. bool AsyncPreambleBuilds = false; + + /// Used to create a context that wraps each single operation. + /// Typically to inject per-file configuration. + /// If the path is empty, context sholud be "generic". + std::function ContextProvider; }; TUScheduler(const GlobalCompilationDatabase &CDB, const Options &Opts, @@ -233,7 +238,9 @@ llvm::StringMap getAllFileContents() const; /// Schedule an async task with no dependencies. - void run(llvm::StringRef Name, llvm::unique_function Action); + /// Path may be empty (it is used only to set the Context). + void run(llvm::StringRef Name, llvm::StringRef Path, + llvm::unique_function Action); /// Defines how a runWithAST action is implicitly cancelled by other actions. enum ASTActionInvalidation { @@ -301,6 +308,7 @@ // this inside clangd. // FIXME: remove this when there is proper index support via build system // integration. + // FIXME: move to ClangdServer via createProcessingContext. static llvm::Optional getFileBeingProcessedInContext(); private: diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp --- a/clang-tools-extra/clangd/TUScheduler.cpp +++ b/clang-tools-extra/clangd/TUScheduler.cpp @@ -270,6 +270,10 @@ { WithContext Guard(std::move(CurrentReq->Ctx)); + // Note that we don't make use of the ContextProvider here. + // Preamble tasks are always scheduled by ASTWorker tasks, and we + // reuse the context/config that was created at that level. + // Build the preamble and let the waiters know about it. build(std::move(*CurrentReq)); } @@ -456,6 +460,8 @@ const DebouncePolicy UpdateDebounce; /// File that ASTWorker is responsible for. const Path FileName; + /// Callback to create processing contexts for tasks. + const std::function ContextProvider; const GlobalCompilationDatabase &CDB; /// Callback invoked when preamble or main file AST is built. ParsingCallbacks &Callbacks; @@ -569,8 +575,9 @@ bool RunSync, const TUScheduler::Options &Opts, ParsingCallbacks &Callbacks) : IdleASTs(LRUCache), RunSync(RunSync), UpdateDebounce(Opts.UpdateDebounce), - FileName(FileName), CDB(CDB), Callbacks(Callbacks), Barrier(Barrier), - Done(false), Status(FileName, Callbacks), + FileName(FileName), ContextProvider(Opts.ContextProvider), CDB(CDB), + Callbacks(Callbacks), Barrier(Barrier), Done(false), + Status(FileName, Callbacks), PreamblePeer(FileName, Callbacks, Opts.StorePreamblesInMemory, RunSync || !Opts.AsyncPreambleBuilds, Status, *this) { // Set a fallback command because compile command can be accessed before @@ -1055,6 +1062,9 @@ Status.ASTActivity.K = ASTAction::RunningAction; Status.ASTActivity.Name = CurrentRequest->Name; }); + llvm::Optional WithProvidedContext; + if (ContextProvider) + WithProvidedContext.emplace(ContextProvider(FileName)); CurrentRequest->Action(); } @@ -1288,14 +1298,18 @@ return Results; } -void TUScheduler::run(llvm::StringRef Name, +void TUScheduler::run(llvm::StringRef Name, llvm::StringRef Path, llvm::unique_function Action) { if (!PreambleTasks) return Action(); PreambleTasks->runAsync(Name, [this, Ctx = Context::current().clone(), + Path(Path.str()), Action = std::move(Action)]() mutable { std::lock_guard BarrierLock(Barrier); WithContext WC(std::move(Ctx)); + llvm::Optional WithProvidedContext; + if (Opts.ContextProvider) + WithProvidedContext.emplace(Opts.ContextProvider(Path)); Action(); }); } @@ -1356,6 +1370,9 @@ WithContext Guard(std::move(Ctx)); trace::Span Tracer(Name); SPAN_ATTACH(Tracer, "file", File); + llvm::Optional WithProvidedContext; + if (Opts.ContextProvider) + WithProvidedContext.emplace(Opts.ContextProvider(File)); Action(InputsAndPreamble{Contents, Command, Preamble.get()}); }; diff --git a/clang-tools-extra/clangd/index/Background.h b/clang-tools-extra/clangd/index/Background.h --- a/clang-tools-extra/clangd/index/Background.h +++ b/clang-tools-extra/clangd/index/Background.h @@ -138,7 +138,8 @@ // Arbitrary value to ensure some concurrency in tests. // In production an explicit value is passed. size_t ThreadPoolSize = 4, - std::function OnProgress = nullptr); + std::function OnProgress = nullptr, + std::function ContextProvider = nullptr); ~BackgroundIndex(); // Blocks while the current task finishes. // Enqueue translation units for indexing. @@ -183,6 +184,7 @@ const ThreadsafeFS &TFS; const GlobalCompilationDatabase &CDB; Context BackgroundContext; + std::function ContextProvider; llvm::Error index(tooling::CompileCommand); @@ -193,12 +195,11 @@ BackgroundIndexStorage::Factory IndexStorageFactory; // Tries to load shards for the MainFiles and their dependencies. - std::vector - loadProject(std::vector MainFiles); + std::vector loadProject(std::vector MainFiles); BackgroundQueue::Task changedFilesTask(const std::vector &ChangedFiles); - BackgroundQueue::Task indexFileTask(tooling::CompileCommand Cmd); + BackgroundQueue::Task indexFileTask(std::string Path); // from lowest to highest priority enum QueuePriority { diff --git a/clang-tools-extra/clangd/index/Background.cpp b/clang-tools-extra/clangd/index/Background.cpp --- a/clang-tools-extra/clangd/index/Background.cpp +++ b/clang-tools-extra/clangd/index/Background.cpp @@ -93,9 +93,11 @@ Context BackgroundContext, const ThreadsafeFS &TFS, const GlobalCompilationDatabase &CDB, BackgroundIndexStorage::Factory IndexStorageFactory, size_t ThreadPoolSize, - std::function OnProgress) + std::function OnProgress, + std::function ContextProvider) : SwapIndex(std::make_unique()), TFS(TFS), CDB(CDB), BackgroundContext(std::move(BackgroundContext)), + ContextProvider(std::move(ContextProvider)), Rebuilder(this, &IndexedSymbols, ThreadPoolSize), IndexStorageFactory(std::move(IndexStorageFactory)), Queue(std::move(OnProgress)), @@ -122,6 +124,11 @@ const std::vector &ChangedFiles) { BackgroundQueue::Task T([this, ChangedFiles] { trace::Span Tracer("BackgroundIndexEnqueue"); + + llvm::Optional WithProvidedContext; + if (ContextProvider) + WithProvidedContext.emplace(ContextProvider(/*Path=*/"")); + // We're doing this asynchronously, because we'll read shards here too. log("Enqueueing {0} commands for indexing", ChangedFiles.size()); SPAN_ATTACH(Tracer, "files", int64_t(ChangedFiles.size())); @@ -147,17 +154,20 @@ return Path.drop_back(llvm::sys::path::extension(Path).size()); } -BackgroundQueue::Task -BackgroundIndex::indexFileTask(tooling::CompileCommand Cmd) { - BackgroundQueue::Task T([this, Cmd] { - // We can't use llvm::StringRef here since we are going to - // move from Cmd during the call below. - const std::string FileName = Cmd.Filename; - if (auto Error = index(std::move(Cmd))) - elog("Indexing {0} failed: {1}", FileName, std::move(Error)); +BackgroundQueue::Task BackgroundIndex::indexFileTask(std::string Path) { + std::string Tag = filenameWithoutExtension(Path).str(); + BackgroundQueue::Task T([this, Path(std::move(Path))] { + llvm::Optional WithProvidedContext; + if (ContextProvider) + WithProvidedContext.emplace(ContextProvider(Path)); + auto Cmd = CDB.getCompileCommand(Path); + if (!Cmd) + return; + if (auto Error = index(std::move(*Cmd))) + elog("Indexing {0} failed: {1}", Path, std::move(Error)); }); T.QueuePri = IndexFile; - T.Tag = std::string(filenameWithoutExtension(Cmd.Filename)); + T.Tag = std::move(Tag); return T; } @@ -342,10 +352,8 @@ // Restores shards for \p MainFiles from index storage. Then checks staleness of // those shards and returns a list of TUs that needs to be indexed to update // staleness. -std::vector +std::vector BackgroundIndex::loadProject(std::vector MainFiles) { - std::vector NeedsReIndexing; - Rebuilder.startLoading(); // Load shards for all of the mainfiles. const std::vector Result = @@ -398,14 +406,7 @@ TUsToIndex.insert(TUForFile); } - for (PathRef TU : TUsToIndex) { - auto Cmd = CDB.getCompileCommand(TU); - if (!Cmd) - continue; - NeedsReIndexing.emplace_back(std::move(*Cmd)); - } - - return NeedsReIndexing; + return {TUsToIndex.begin(), TUsToIndex.end()}; } } // namespace clangd diff --git a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp --- a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp @@ -1,3 +1,5 @@ +#include "CompileCommands.h" +#include "Config.h" #include "Headers.h" #include "SyncAPI.h" #include "TestFS.h" @@ -5,6 +7,7 @@ #include "TestTU.h" #include "index/Background.h" #include "index/BackgroundRebuild.h" +#include "clang/Tooling/ArgumentsAdjusters.h" #include "clang/Tooling/CompilationDatabase.h" #include "llvm/Support/ScopedPrinter.h" #include "llvm/Support/Threading.h" @@ -24,6 +27,7 @@ namespace clangd { MATCHER_P(Named, N, "") { return arg.Name == N; } +MATCHER_P(QName, N, "") { return (arg.Scope + arg.Name).str() == N; } MATCHER(Declared, "") { return !StringRef(arg.CanonicalDeclaration.FileURI).empty(); } @@ -104,6 +108,51 @@ ASSERT_TRUE(Idx.blockUntilIdleForTest()); } +TEST_F(BackgroundIndexTest, Config) { + MockFS FS; + // Set up two identical TUs, foo and bar. + // They define foo::one and bar::one. + std::vector Cmds; + for (std::string Name : {"foo", "bar"}) { + std::string Filename = Name + ".cpp"; + std::string Header = Name + ".h"; + FS.Files[Filename] = "#include \"" + Header + "\""; + FS.Files[Header] = "namespace " + Name + " { int one; }"; + tooling::CompileCommand Cmd; + Cmd.Filename = Filename; + Cmd.Directory = testRoot(); + Cmd.CommandLine = {"clang++", Filename}; + Cmds.push_back(std::move(Cmd)); + } + // Context provider that installs a configuration mutating foo's command. + // This causes it to define foo::two instead of foo::one. + auto ContextProvider = [](PathRef P) { + Config C; + if (P.endswith("foo.cpp")) + C.CompileFlags.Edits.push_back( + [](std::vector &Argv) { Argv.push_back("-Done=two"); }); + return Context::current().derive(Config::Key, std::move(C)); + }; + // Create the background index. + llvm::StringMap Storage; + size_t CacheHits = 0; + MemoryShardStorage MSS(Storage, CacheHits); + // We need the CommandMangler, because that applies the config we're testing. + OverlayCDB CDB(/*Base=*/nullptr, /*FallbackFlags=*/{}, + tooling::ArgumentsAdjuster(CommandMangler::forTests())); + BackgroundIndex Idx( + Context::empty(), FS, CDB, [&](llvm::StringRef) { return &MSS; }, + /*ThreadPoolSize=*/4, /*OnProgress=*/nullptr, std::move(ContextProvider)); + // Index the two files. + for (auto &Cmd : Cmds) + CDB.setCompileCommand(testPath(Cmd.Filename), std::move(Cmd)); + // Wait for both files to be indexed. + ASSERT_TRUE(Idx.blockUntilIdleForTest()); + EXPECT_THAT(runFuzzyFind(Idx, ""), + UnorderedElementsAre(QName("foo"), QName("foo::two"), + QName("bar"), QName("bar::one"))); +} + TEST_F(BackgroundIndexTest, IndexTwoFiles) { MockFS FS; // a.h yields different symbols when included by A.cc vs B.cc. diff --git a/clang-tools-extra/clangd/unittests/ClangdTests.cpp b/clang-tools-extra/clangd/unittests/ClangdTests.cpp --- a/clang-tools-extra/clangd/unittests/ClangdTests.cpp +++ b/clang-tools-extra/clangd/unittests/ClangdTests.cpp @@ -10,6 +10,7 @@ #include "ClangdLSPServer.h" #include "ClangdServer.h" #include "CodeComplete.h" +#include "ConfigFragment.h" #include "GlobalCompilationDatabase.h" #include "Matchers.h" #include "SyncAPI.h" @@ -19,6 +20,7 @@ #include "support/Threading.h" #include "clang/Config/config.h" #include "clang/Sema/CodeCompleteConsumer.h" +#include "clang/Tooling/ArgumentsAdjusters.h" #include "llvm/ADT/None.h" #include "llvm/ADT/Optional.h" #include "llvm/ADT/SmallVector.h" @@ -47,6 +49,7 @@ using ::testing::Gt; using ::testing::IsEmpty; using ::testing::Pair; +using ::testing::SizeIs; using ::testing::UnorderedElementsAre; MATCHER_P2(DeclAt, File, Range, "") { @@ -301,6 +304,48 @@ EXPECT_EQ(Callbacks.Got, 42); } +TEST(ClangdServerTest, RespectsConfig) { + // Go-to-definition will resolve as marked if FOO is defined. + Annotations Example(R"cpp( + #ifdef FOO + int [[x]]; + #else + int x; + #endif + int y = ^x; + )cpp"); + // Provide conditional config that defines FOO for foo.cc. + class ConfigProvider : public config::Provider { + std::vector + getFragments(const config::Params &, + config::DiagnosticCallback DC) const override { + config::Fragment F; + F.If.PathMatch.emplace_back(".*foo.cc"); + F.CompileFlags.Add.emplace_back("-DFOO=1"); + return {std::move(F).compile(DC)}; + } + } CfgProvider; + + auto Opts = ClangdServer::optsForTest(); + Opts.ConfigProvider = &CfgProvider; + OverlayCDB CDB(/*Base=*/nullptr, /*FallbackFlags=*/{}, + tooling::ArgumentsAdjuster(CommandMangler::forTests())); + MockFS FS; + ClangdServer Server(CDB, FS, Opts); + // foo.cc sees the expected definition, as FOO is defined. + Server.addDocument(testPath("foo.cc"), Example.code()); + auto Result = runLocateSymbolAt(Server, testPath("foo.cc"), Example.point()); + ASSERT_TRUE(bool(Result)) << Result.takeError(); + ASSERT_THAT(*Result, SizeIs(1)); + EXPECT_EQ(Result->front().PreferredDeclaration.range, Example.range()); + // bar.cc gets a different result, as FOO is not defined. + Server.addDocument(testPath("bar.cc"), Example.code()); + Result = runLocateSymbolAt(Server, testPath("bar.cc"), Example.point()); + ASSERT_TRUE(bool(Result)) << Result.takeError(); + ASSERT_THAT(*Result, SizeIs(1)); + EXPECT_NE(Result->front().PreferredDeclaration.range, Example.range()); +} + TEST_F(ClangdVFSTest, PropagatesVersion) { MockCompilationDatabase CDB; MockFS FS; diff --git a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp --- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp +++ b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp @@ -65,8 +65,20 @@ return true; } +// Dummy ContextProvider to verify the provider is invoked & contexts are used. +static Key BoundPath; +Context bindPath(PathRef F) { + return Context::current().derive(BoundPath, F.str()); +} +llvm::StringRef boundPath() { + const std::string *V = Context::current().get(BoundPath); + return V ? *V : llvm::StringRef(""); +} + TUScheduler::Options optsForTest() { - return TUScheduler::Options(ClangdServer::optsForTest()); + TUScheduler::Options Opts(ClangdServer::optsForTest()); + Opts.ContextProvider = bindPath; + return Opts; } class TUSchedulerTests : public ::testing::Test { @@ -454,6 +466,7 @@ [File, Nonce, Version(Inputs.Version), &Mut, &TotalUpdates, &LatestDiagVersion](std::vector) { EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce)); + EXPECT_EQ(File, boundPath()); std::lock_guard Lock(Mut); ++TotalUpdates; @@ -474,6 +487,7 @@ [File, Inputs, Nonce, &Mut, &TotalASTReads](Expected AST) { EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce)); + EXPECT_EQ(File, boundPath()); ASSERT_TRUE((bool)AST); EXPECT_EQ(AST->Inputs.Contents, Inputs.Contents); @@ -493,6 +507,7 @@ [File, Inputs, Nonce, &Mut, &TotalPreambleReads](Expected Preamble) { EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce)); + EXPECT_EQ(File, boundPath()); ASSERT_TRUE((bool)Preamble); EXPECT_EQ(Preamble->Contents, Inputs.Contents); @@ -849,18 +864,22 @@ } TEST_F(TUSchedulerTests, Run) { - TUScheduler S(CDB, optsForTest()); + auto Opts = optsForTest(); + Opts.ContextProvider = bindPath; + TUScheduler S(CDB, Opts); std::atomic Counter(0); - S.run("add 1", [&] { ++Counter; }); - S.run("add 2", [&] { Counter += 2; }); + S.run("add 1", /*Path=*/"", [&] { ++Counter; }); + S.run("add 2", /*Path=*/"", [&] { Counter += 2; }); ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10))); EXPECT_EQ(Counter.load(), 3); Notification TaskRun; Key TestKey; WithContextValue CtxWithKey(TestKey, 10); - S.run("props context", [&] { + const char *Path = "somepath"; + S.run("props context", Path, [&] { EXPECT_EQ(Context::current().getExisting(TestKey), 10); + EXPECT_EQ(Path, boundPath()); TaskRun.notify(); }); TaskRun.wait();