https://github.com/Fil-Den updated https://github.com/llvm/llvm-project/pull/230635
>From 0aa974e98315bf8d505dc60a3b5d236dd57bc8ca Mon Sep 17 00:00:00 2001 From: Fil-Den <[email protected]> Date: Fri, 9 Oct 2026 22:19:00 +0300 Subject: [PATCH 1/2] clangd-indexer .clangd support for relative file paths --- .../clangd/indexer/IndexerMain.cpp | 32 ++--- .../indexer-clangd-config-relative-path.test | 6 +- clang/include/clang/Tooling/AllTUsExecution.h | 5 + clang/include/clang/Tooling/Execution.h | 7 + .../clang/Tooling/StandaloneExecution.h | 4 + clang/include/clang/Tooling/Tooling.h | 29 +++++ clang/lib/Tooling/AllTUsExecution.cpp | 1 + clang/lib/Tooling/Tooling.cpp | 122 ++++++++++-------- clang/unittests/Tooling/ExecutionTest.cpp | 2 + clang/unittests/Tooling/ToolingTest.cpp | 43 ++++++ 10 files changed, 170 insertions(+), 81 deletions(-) diff --git a/clang-tools-extra/clangd/indexer/IndexerMain.cpp b/clang-tools-extra/clangd/indexer/IndexerMain.cpp index f081299552064..c034c2f2ce83c 100644 --- a/clang-tools-extra/clangd/indexer/IndexerMain.cpp +++ b/clang-tools-extra/clangd/indexer/IndexerMain.cpp @@ -26,6 +26,7 @@ #include "clang/Tooling/ArgumentsAdjusters.h" #include "clang/Tooling/Execution.h" #include "clang/Tooling/Tooling.h" +#include "llvm/ADT/SmallString.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/Path.h" #include "llvm/Support/Signals.h" @@ -183,28 +184,21 @@ int main(int argc, const char **argv) { Mangler->SystemIncludeExtractor = clang::clangd::getSystemIncludeExtractor( static_cast<llvm::ArrayRef<std::string>>( clang::clangd::QueryDriverGlobs)); + Executor->get()->setInvocationWrapper( + [ContextProvider = std::move(ContextProvider)]( + const clang::tooling::CompileCommand &Cmd, + llvm::function_ref<bool()> Invoke) { + // Cmd.Filename may be relative to the compile command's directory. + llvm::SmallString<256> AbsFile(Cmd.Filename); + llvm::sys::path::make_absolute(Cmd.Directory, AbsFile); + clang::clangd::WithContext WithCfg(ContextProvider(AbsFile)); + return Invoke(); + }); auto Err = Executor->get()->execute( std::make_unique<clang::clangd::IndexActionFactory>(Data), clang::tooling::ArgumentsAdjuster( - [Mangler = std::move(Mangler), - ContextProvider = std::move(ContextProvider)]( - const std::vector<std::string> &Args, llvm::StringRef File) { - // FIXME: If File is relative, it's relative to the compile - // command's "directory", not our CWD, but ToolExecutor doesn't - // expose "directory" here. We don't have enough information to - // correctly determine the absolute path, so we don't apply the - // config for relative paths. See - // indexer-clangd-config-relative-path.test. - // FIXME: WithCfg only lives for this ArgumentsAdjuster call, so - // it's visible to Mangler below but not to the parse that follows. - // That's harmless today since clangd-indexer doesn't consult - // config during the parse, but a real fix would need libTooling - // changes to keep the context alive for the whole invocation. - std::optional<clang::clangd::WithContext> WithCfg; - if (llvm::sys::path::is_absolute(File)) { - WithCfg.emplace(ContextProvider(File)); - } - + [Mangler = std::move(Mangler)](const std::vector<std::string> &Args, + llvm::StringRef File) { clang::tooling::CompileCommand Cmd; Cmd.CommandLine = Args; Mangler->operator()(Cmd, File); diff --git a/clang-tools-extra/clangd/test/indexer-clangd-config-relative-path.test b/clang-tools-extra/clangd/test/indexer-clangd-config-relative-path.test index 8f4f22c50a132..da309fb3d4906 100644 --- a/clang-tools-extra/clangd/test/indexer-clangd-config-relative-path.test +++ b/clang-tools-extra/clangd/test/indexer-clangd-config-relative-path.test @@ -5,10 +5,6 @@ # systems, so that the developer's own user config can't affect the result. # UNSUPPORTED: system-windows # UNSUPPORTED: system-darwin -# -# FIXME: clangd-indexer does not yet find the .clangd file for relative "file" -# entries, so the REL check below fails. Remove this XFAIL once that is fixed. -# XFAIL: * # RUN: rm -rf %t.dir && mkdir -p %t.dir # RUN: split-file %s %t.dir @@ -32,7 +28,7 @@ # REL: Processing file {{.*}}test.cpp # REL-NOT: error: "FROM_CLANGD_FILE not defined -# Absolute "file" entry: same project, same config, and this one works today. +# Absolute "file" entry: same project, same config. # RUN: cd %t.dir/elsewhere && env XDG_CONFIG_HOME=%t.dir/xdg \ # RUN: clangd-indexer --enable-config --executor=all-TUs \ # RUN: %t.dir/abs/compile_commands.json > %t.dir/abs.idx 2> %t.dir/abs.log diff --git a/clang/include/clang/Tooling/AllTUsExecution.h b/clang/include/clang/Tooling/AllTUsExecution.h index 03cfc9c67f5c5..974fe746502e3 100644 --- a/clang/include/clang/Tooling/AllTUsExecution.h +++ b/clang/include/clang/Tooling/AllTUsExecution.h @@ -60,6 +60,10 @@ class AllTUsToolExecutor : public ToolExecutor { OverlayFiles[FilePath] = std::string(Content); } + void setInvocationWrapper(InvocationWrapper Wrapper) override { + InvocationWrap = std::move(Wrapper); + } + private: // Used to store the parser when the executor is initialized with parser. std::optional<CommonOptionsParser> OptionsParser; @@ -67,6 +71,7 @@ class AllTUsToolExecutor : public ToolExecutor { std::unique_ptr<ToolResults> Results; ExecutionContext Context; llvm::StringMap<std::string> OverlayFiles; + InvocationWrapper InvocationWrap; unsigned ThreadCount; }; diff --git a/clang/include/clang/Tooling/Execution.h b/clang/include/clang/Tooling/Execution.h index ca6f22c5da3bf..a0ba49e37124f 100644 --- a/clang/include/clang/Tooling/Execution.h +++ b/clang/include/clang/Tooling/Execution.h @@ -144,6 +144,13 @@ class ToolExecutor { /// \param FilePath The path at which the content will be mapped. /// \param Content A buffer of the file's content. virtual void mapVirtualFile(StringRef FilePath, StringRef Content) = 0; + + /// Set a hook that runs around the processing of each compile command. + /// + /// See \c InvocationWrapper. Implementations must forward the wrapper to + /// every \c ClangTool they use (or otherwise honor it). Depending on the + /// executor, the wrapper may be called concurrently from multiple threads. + virtual void setInvocationWrapper(InvocationWrapper Wrapper) = 0; }; /// Interface for factories that create specific executors. This is also diff --git a/clang/include/clang/Tooling/StandaloneExecution.h b/clang/include/clang/Tooling/StandaloneExecution.h index cdbe65a95b9d2..8ce273ca8993f 100644 --- a/clang/include/clang/Tooling/StandaloneExecution.h +++ b/clang/include/clang/Tooling/StandaloneExecution.h @@ -77,6 +77,10 @@ class StandaloneToolExecutor : public ToolExecutor { Tool.mapVirtualFile(FilePath, Content); } + void setInvocationWrapper(InvocationWrapper Wrapper) override { + Tool.setInvocationWrapper(std::move(Wrapper)); + } + /// Returns the file manager used in the tool. /// /// The file manager is shared between all translation units. diff --git a/clang/include/clang/Tooling/Tooling.h b/clang/include/clang/Tooling/Tooling.h index 9909394495496..59f8b0e27b3c2 100644 --- a/clang/include/clang/Tooling/Tooling.h +++ b/clang/include/clang/Tooling/Tooling.h @@ -38,12 +38,14 @@ #include "clang/Tooling/ArgumentsAdjusters.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" +#include "llvm/ADT/STLFunctionalExtras.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/StringSet.h" #include "llvm/ADT/Twine.h" #include "llvm/Option/Option.h" #include "llvm/Support/VirtualFileSystem.h" +#include <functional> #include <memory> #include <string> #include <utility> @@ -65,6 +67,7 @@ class Compilation; namespace tooling { class CompilationDatabase; +struct CompileCommand; /// Retrieves the flags of the `-cc1` job in `Compilation` that has only source /// files as its inputs. @@ -309,6 +312,23 @@ class ToolInvocation { DiagnosticOptions *DiagOpts = nullptr; }; +/// A hook that runs around each compile command processed by a \c ClangTool. +/// +/// \p Cmd is the compile command about to be processed. Its \c Directory is +/// the working directory the command will run in, and its \c Filename may be +/// relative to it. \p Invoke does the actual work (applying arguments +/// adjusters and running the tool action) and returns whether it succeeded. +/// The wrapper should normally call \p Invoke exactly once and return its +/// result; the wrapper's return value is used as the success status for +/// \p Cmd. +/// +/// This allows, for example, setting up file-specific state for the whole +/// duration of the invocation, e.g. using an RAII object. +/// +/// Executors may call the wrapper concurrently from multiple threads. +using InvocationWrapper = std::function<bool( + const CompileCommand &Cmd, llvm::function_ref<bool()> Invoke)>; + /// Utility to run a FrontendAction over a set of files. /// /// This class is written to be usable for command line utilities. @@ -360,6 +380,13 @@ class ClangTool { /// Clear the command line arguments adjuster chain. void clearArgumentsAdjusters(); + /// Set a hook that runs around the processing of each compile command. + /// + /// See \c InvocationWrapper. Passing an empty wrapper removes the hook. + void setInvocationWrapper(InvocationWrapper Wrapper) { + InvocationWrap = std::move(Wrapper); + } + /// Runs an action over all files specified in the command line. /// /// \param Action Tool action. @@ -399,6 +426,8 @@ class ClangTool { ArgumentsAdjuster ArgsAdjuster; + InvocationWrapper InvocationWrap; + DiagnosticConsumer *DiagConsumer = nullptr; bool PrintErrorMessage = true; diff --git a/clang/lib/Tooling/AllTUsExecution.cpp b/clang/lib/Tooling/AllTUsExecution.cpp index 9cad8680447be..1feb288266ccc 100644 --- a/clang/lib/Tooling/AllTUsExecution.cpp +++ b/clang/lib/Tooling/AllTUsExecution.cpp @@ -129,6 +129,7 @@ llvm::Error AllTUsToolExecutor::execute( std::make_shared<PCHContainerOperations>(), FS); Tool.appendArgumentsAdjuster(Action.second); Tool.appendArgumentsAdjuster(getDefaultArgumentsAdjusters()); + Tool.setInvocationWrapper(InvocationWrap); for (const auto &FileAndContent : OverlayFiles) Tool.mapVirtualFile(FileAndContent.first(), FileAndContent.second); diff --git a/clang/lib/Tooling/Tooling.cpp b/clang/lib/Tooling/Tooling.cpp index 71307d1fe7307..c9de2778c07e3 100644 --- a/clang/lib/Tooling/Tooling.cpp +++ b/clang/lib/Tooling/Tooling.cpp @@ -567,68 +567,76 @@ int ClangTool::run(ToolAction *Action) { for (CompileCommand &CompileCommand : CompileCommandsForFile) { // If the 'directory' field of the compilation database is empty, display // an error and use the working directory instead. - StringRef Directory = CompileCommand.Directory; - if (Directory.empty()) { + if (CompileCommand.Directory.empty()) { llvm::errs() << "'directory' field of compilation database is empty; " "using the current working directory instead.\n"; - Directory = InitialWorkingDir; + CompileCommand.Directory = InitialWorkingDir; } - // FIXME: chdir is thread hostile; on the other hand, creating the same - // behavior as chdir is complex: chdir resolves the path once, thus - // guaranteeing that all subsequent relative path operations work - // on the same path the original chdir resulted in. This makes a - // difference for example on network filesystems, where symlinks might be - // switched during runtime of the tool. Fixing this depends on having a - // file system abstraction that allows openat() style interactions. - if (OverlayFileSystem->setCurrentWorkingDirectory(Directory)) - llvm::report_fatal_error("Cannot chdir into \"" + Twine(Directory) + - "\"!"); - - // Now fill the in-memory VFS with the relative file mappings so it will - // have the correct relative paths. We never remove mappings but that - // should be fine. - if (SeenWorkingDirectories.insert(Directory).second) - for (const auto &MappedFile : MappedFileContents) - if (!llvm::sys::path::is_absolute(MappedFile.first)) - InMemoryFileSystem->addFile( - MappedFile.first, 0, - llvm::MemoryBuffer::getMemBuffer(MappedFile.second)); - - std::vector<std::string> CommandLine = CompileCommand.CommandLine; - if (ArgsAdjuster) - CommandLine = ArgsAdjuster(CommandLine, CompileCommand.Filename); - assert(!CommandLine.empty()); - - // Add the resource dir based on the binary of this tool. argv[0] in the - // compilation database may refer to a different compiler and we want to - // pick up the very same standard library that compiler is using. The - // builtin headers in the resource dir need to match the exact clang - // version the tool is using. - // FIXME: On linux, GetMainExecutable is independent of the value of the - // first argument, thus allowing ClangTool and runToolOnCode to just - // pass in made-up names here. Make sure this works on other platforms. - injectResourceDir(CommandLine, "clang_tool", &StaticSymbol); - - ++CurrentCommandIndexForFile; - - // FIXME: We need a callback mechanism for the tool writer to output a - // customized message for each file. - if (NumOfTotalFiles > 1 || CompileCommandsForFile.size() > 1) { - llvm::errs() << "[" << std::to_string(CurrentFileIndex) << "/" - << std::to_string(NumOfTotalFiles) << "]"; - if (CompileCommandsForFile.size() > 1) { - llvm::errs() << " (" << std::to_string(CurrentCommandIndexForFile) - << "/" << std::to_string(CompileCommandsForFile.size()) - << ")"; + auto Invoke = [&]() -> bool { + StringRef Directory = CompileCommand.Directory; + + // FIXME: chdir is thread hostile; on the other hand, creating the same + // behavior as chdir is complex: chdir resolves the path once, thus + // guaranteeing that all subsequent relative path operations work + // on the same path the original chdir resulted in. This makes a + // difference for example on network filesystems, where symlinks might + // be switched during runtime of the tool. Fixing this depends on + // having a file system abstraction that allows openat() style + // interactions. + if (OverlayFileSystem->setCurrentWorkingDirectory(Directory)) + llvm::report_fatal_error("Cannot chdir into \"" + Twine(Directory) + + "\"!"); + + // Now fill the in-memory VFS with the relative file mappings so it + // will have the correct relative paths. We never remove mappings but + // that should be fine. + if (SeenWorkingDirectories.insert(Directory).second) + for (const auto &MappedFile : MappedFileContents) + if (!llvm::sys::path::is_absolute(MappedFile.first)) + InMemoryFileSystem->addFile( + MappedFile.first, 0, + llvm::MemoryBuffer::getMemBuffer(MappedFile.second)); + + std::vector<std::string> CommandLine = CompileCommand.CommandLine; + if (ArgsAdjuster) + CommandLine = ArgsAdjuster(CommandLine, CompileCommand.Filename); + assert(!CommandLine.empty()); + + // Add the resource dir based on the binary of this tool. argv[0] in + // the compilation database may refer to a different compiler and we + // want to pick up the very same standard library that compiler is + // using. The builtin headers in the resource dir need to match the + // exact clang version the tool is using. + // FIXME: On linux, GetMainExecutable is independent of the value of + // the first argument, thus allowing ClangTool and runToolOnCode to + // just pass in made-up names here. Make sure this works on other + // platforms. + injectResourceDir(CommandLine, "clang_tool", &StaticSymbol); + + ++CurrentCommandIndexForFile; + + // FIXME: We need a callback mechanism for the tool writer to output a + // customized message for each file. + if (NumOfTotalFiles > 1 || CompileCommandsForFile.size() > 1) { + llvm::errs() << "[" << std::to_string(CurrentFileIndex) << "/" + << std::to_string(NumOfTotalFiles) << "]"; + if (CompileCommandsForFile.size() > 1) { + llvm::errs() << " (" << std::to_string(CurrentCommandIndexForFile) + << "/" << std::to_string(CompileCommandsForFile.size()) + << ")"; + } + llvm::errs() << " Processing file " << File << ".\n"; } - llvm::errs() << " Processing file " << File << ".\n"; - } - ToolInvocation Invocation(std::move(CommandLine), Action, Files.get(), - PCHContainerOps); - Invocation.setDiagnosticConsumer(DiagConsumer); - - if (!Invocation.run()) { + ToolInvocation Invocation(std::move(CommandLine), Action, Files.get(), + PCHContainerOps); + Invocation.setDiagnosticConsumer(DiagConsumer); + return Invocation.run(); + }; + + bool Success = + InvocationWrap ? InvocationWrap(CompileCommand, Invoke) : Invoke(); + if (!Success) { // FIXME: Diagnostics should be used instead. if (PrintErrorMessage) llvm::errs() << "Error while processing " << File << ".\n"; diff --git a/clang/unittests/Tooling/ExecutionTest.cpp b/clang/unittests/Tooling/ExecutionTest.cpp index c81049308f706..8b19fdc903349 100644 --- a/clang/unittests/Tooling/ExecutionTest.cpp +++ b/clang/unittests/Tooling/ExecutionTest.cpp @@ -112,6 +112,8 @@ class TestToolExecutor : public ToolExecutor { VFS[std::string(FilePath)] = std::string(Content); } + void setInvocationWrapper(InvocationWrapper) override {} + private: CommonOptionsParser OptionsParser; std::string SourcePaths; diff --git a/clang/unittests/Tooling/ToolingTest.cpp b/clang/unittests/Tooling/ToolingTest.cpp index 11a5491dd35fb..788afeda9b97b 100644 --- a/clang/unittests/Tooling/ToolingTest.cpp +++ b/clang/unittests/Tooling/ToolingTest.cpp @@ -744,6 +744,49 @@ TEST(ClangToolTest, ArgumentAdjusters) { EXPECT_FALSE(Found); } +TEST(ClangToolTest, InvocationWrapper) { + FixedCompilationDatabase Compilations("/", std::vector<std::string>()); + + ClangTool Tool(Compilations, std::vector<std::string>(1, "/a.cc")); + Tool.mapVirtualFile("/a.cc", "void a() {}"); + + std::unique_ptr<FrontendActionFactory> Action( + newFrontendActionFactory<SyntaxOnlyAction>()); + + bool InWrapper = false; + bool AdjusterRanInWrapper = false; + bool AdjusterRan = false; + Tool.appendArgumentsAdjuster( + [&](const CommandLineArguments &Args, StringRef /*unused*/) { + AdjusterRan = true; + AdjusterRanInWrapper = InWrapper; + return Args; + }); + + unsigned WrapperCalls = 0; + Tool.setInvocationWrapper( + [&](const CompileCommand &Cmd, llvm::function_ref<bool()> Invoke) { + ++WrapperCalls; + EXPECT_TRUE(StringRef(Cmd.Filename).ends_with("a.cc")); + EXPECT_EQ("/", Cmd.Directory); + InWrapper = true; + bool Result = Invoke(); + InWrapper = false; + return Result; + }); + EXPECT_EQ(0, Tool.run(Action.get())); + EXPECT_EQ(1u, WrapperCalls); + EXPECT_TRUE(AdjusterRan); + EXPECT_TRUE(AdjusterRanInWrapper); + + // A wrapper that doesn't invoke the action fails the run. + AdjusterRan = false; + Tool.setInvocationWrapper( + [](const CompileCommand &, llvm::function_ref<bool()>) { return false; }); + EXPECT_EQ(1, Tool.run(Action.get())); + EXPECT_FALSE(AdjusterRan); +} + TEST(ClangToolTest, NoDoubleSyntaxOnly) { FixedCompilationDatabase Compilations("/", {"-fsyntax-only"}); >From 7cc2b34719edbc32e18671a76a1b3c1a890acd5a Mon Sep 17 00:00:00 2001 From: Fil-Den <[email protected]> Date: Sat, 10 Oct 2026 16:47:34 +0300 Subject: [PATCH 2/2] Fix the test on Windows --- clang/unittests/Tooling/ToolingTest.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/clang/unittests/Tooling/ToolingTest.cpp b/clang/unittests/Tooling/ToolingTest.cpp index 788afeda9b97b..ec8805f2e2e72 100644 --- a/clang/unittests/Tooling/ToolingTest.cpp +++ b/clang/unittests/Tooling/ToolingTest.cpp @@ -774,7 +774,14 @@ TEST(ClangToolTest, InvocationWrapper) { InWrapper = false; return Result; }); - EXPECT_EQ(0, Tool.run(Action.get())); + int Result = Tool.run(Action.get()); +#ifndef _WIN32 + // "/a.cc" is not an absolute path on Windows, so the mapped virtual file is + // not found there and the parse fails. The wrapper still runs either way. + EXPECT_EQ(0, Result); +#else + (void)Result; +#endif EXPECT_EQ(1u, WrapperCalls); EXPECT_TRUE(AdjusterRan); EXPECT_TRUE(AdjusterRanInWrapper); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
