diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -502,10 +502,10 @@ CDBOpts.ContextProvider = Opts.ContextProvider; BaseCDB = std::make_unique(CDBOpts); - BaseCDB = getQueryDriverDatabase(llvm::makeArrayRef(Opts.QueryDriverGlobs), - std::move(BaseCDB)); } auto Mangler = CommandMangler::detect(); + Mangler.SystemIncludeExtractor = + getSystemIncludeExtractor(llvm::makeArrayRef(Opts.QueryDriverGlobs)); if (Opts.ResourceDir) Mangler.ResourceDir = *Opts.ResourceDir; CDB.emplace(BaseCDB.get(), Params.initializationOptions.fallbackFlags, @@ -1815,5 +1815,6 @@ }); } } + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/CompileCommands.h b/clang-tools-extra/clangd/CompileCommands.h --- a/clang-tools-extra/clangd/CompileCommands.h +++ b/clang-tools-extra/clangd/CompileCommands.h @@ -32,6 +32,7 @@ llvm::Optional ResourceDir; // Root for searching for standard library (passed to -isysroot). llvm::Optional Sysroot; + SystemIncludeExtractorFn SystemIncludeExtractor; // A command-mangler that doesn't know anything about the system. // This is hermetic for unit-tests, but won't work well in production. diff --git a/clang-tools-extra/clangd/CompileCommands.cpp b/clang-tools-extra/clangd/CompileCommands.cpp --- a/clang-tools-extra/clangd/CompileCommands.cpp +++ b/clang-tools-extra/clangd/CompileCommands.cpp @@ -301,6 +301,17 @@ for (auto &Edit : Config::current().CompileFlags.Edits) Edit(Cmd); + // The system include extractor needs to run: + // - AFTER transferCompileCommand(), because the -x flag it adds may be + // necessary for the system include extractor to identify the file type + // - AFTER applying CompileFlags.Edits, because the name of the compiler + // that needs to be invoked may come from the CompileFlags->Compiler key + // - BEFORE resolveDriver() because that can mess up the driver path, + // e.g. changing gcc to /path/to/clang/bin/gcc + if (SystemIncludeExtractor) { + SystemIncludeExtractor(Command, File); + } + // Check whether the flag exists, either as -flag or -flag=* auto Has = [&](llvm::StringRef Flag) { for (llvm::StringRef Arg : Cmd) { diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.h b/clang-tools-extra/clangd/GlobalCompilationDatabase.h --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.h +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.h @@ -162,11 +162,12 @@ }; /// Extracts system include search path from drivers matching QueryDriverGlobs -/// and adds them to the compile flags. Base may not be nullptr. -/// Returns Base when \p QueryDriverGlobs is empty. -std::unique_ptr -getQueryDriverDatabase(llvm::ArrayRef QueryDriverGlobs, - std::unique_ptr Base); +/// and adds them to the compile flags. +/// Returns null when \p QueryDriverGlobs is empty. +using SystemIncludeExtractorFn = llvm::unique_function; +SystemIncludeExtractorFn +getSystemIncludeExtractor(llvm::ArrayRef QueryDriverGlobs); /// Wraps another compilation database, and supports overriding the commands /// using an in-memory mapping. diff --git a/clang-tools-extra/clangd/QueryDriverDatabase.cpp b/clang-tools-extra/clangd/QueryDriverDatabase.cpp --- a/clang-tools-extra/clangd/QueryDriverDatabase.cpp +++ b/clang-tools-extra/clangd/QueryDriverDatabase.cpp @@ -315,24 +315,20 @@ /// Extracts system includes from a trusted driver by parsing the output of /// include search path and appends them to the commands coming from underlying /// compilation database. -class QueryDriverDatabase : public DelegatingCDB { +class SystemIncludeExtractor { public: - QueryDriverDatabase(llvm::ArrayRef QueryDriverGlobs, - std::unique_ptr Base) - : DelegatingCDB(std::move(Base)), - QueryDriverRegex(convertGlobsToRegex(QueryDriverGlobs)) {} + SystemIncludeExtractor(llvm::ArrayRef QueryDriverGlobs) + : QueryDriverRegex(convertGlobsToRegex(QueryDriverGlobs)) {} - llvm::Optional - getCompileCommand(PathRef File) const override { - auto Cmd = DelegatingCDB::getCompileCommand(File); - if (!Cmd || Cmd->CommandLine.empty()) - return Cmd; + void operator()(tooling::CompileCommand &Cmd, llvm::StringRef File) const { + if (Cmd.CommandLine.empty()) + return; llvm::StringRef Lang; - for (size_t I = 0, E = Cmd->CommandLine.size(); I < E; ++I) { - llvm::StringRef Arg = Cmd->CommandLine[I]; + for (size_t I = 0, E = Cmd.CommandLine.size(); I < E; ++I) { + llvm::StringRef Arg = Cmd.CommandLine[I]; if (Arg == "-x" && I + 1 < E) - Lang = Cmd->CommandLine[I + 1]; + Lang = Cmd.CommandLine[I + 1]; else if (Arg.startswith("-x")) Lang = Arg.drop_front(2).trim(); } @@ -341,26 +337,25 @@ auto Type = driver::types::lookupTypeForExtension(Ext); if (Type == driver::types::TY_INVALID) { elog("System include extraction: invalid file type for {0}", Ext); - return Cmd; + return; } Lang = driver::types::getTypeName(Type); } - llvm::SmallString<128> Driver(Cmd->CommandLine.front()); + llvm::SmallString<128> Driver(Cmd.CommandLine.front()); if (llvm::any_of(Driver, [](char C) { return llvm::sys::path::is_separator(C); })) // Driver is a not a single executable name but instead a path (either // relative or absolute). - llvm::sys::fs::make_absolute(Cmd->Directory, Driver); + llvm::sys::fs::make_absolute(Cmd.Directory, Driver); if (auto Info = QueriedDrivers.get(/*Key=*/(Driver + ":" + Lang).str(), [&] { return extractSystemIncludesAndTarget( - Driver, Lang, Cmd->CommandLine, QueryDriverRegex); + Driver, Lang, Cmd.CommandLine, QueryDriverRegex); })) { - setTarget(addSystemIncludes(*Cmd, Info->SystemIncludes), Info->Target); + setTarget(addSystemIncludes(Cmd, Info->SystemIncludes), Info->Target); } - return Cmd; } private: @@ -370,14 +365,11 @@ }; } // namespace -std::unique_ptr -getQueryDriverDatabase(llvm::ArrayRef QueryDriverGlobs, - std::unique_ptr Base) { - assert(Base && "Null base to SystemIncludeExtractor"); +SystemIncludeExtractorFn +getSystemIncludeExtractor(llvm::ArrayRef QueryDriverGlobs) { if (QueryDriverGlobs.empty()) - return Base; - return std::make_unique(QueryDriverGlobs, - std::move(Base)); + return nullptr; + return SystemIncludeExtractor(QueryDriverGlobs); } } // namespace clangd diff --git a/clang-tools-extra/clangd/test/system-include-extractor.test b/clang-tools-extra/clangd/test/system-include-extractor.test --- a/clang-tools-extra/clangd/test/system-include-extractor.test +++ b/clang-tools-extra/clangd/test/system-include-extractor.test @@ -42,7 +42,7 @@ # RUN: sed -E -e 's|"file://([A-Z]):/|"file:///\1:/|g' %t.test.1 > %t.test # Bless the mock driver we've just created so that clangd can execute it. -# RUN: clangd -lit-test -query-driver="**.test,**.sh" < %t.test | FileCheck -strict-whitespace %t.test +# RUN: clangd -lit-test -query-driver="**.test,**.sh" < %t.test | FileCheck -strict-whitespace %t.test --check-prefix=CHECK1 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{}} --- { @@ -57,10 +57,38 @@ } } } -# CHECK: "method": "textDocument/publishDiagnostics", -# CHECK-NEXT: "params": { -# CHECK-NEXT: "diagnostics": [], +# CHECK1: "method": "textDocument/publishDiagnostics", +# CHECK1-NEXT: "params": { +# CHECK1-NEXT: "diagnostics": [], +# CHECK1-NEXT: "uri": "file://INPUT_DIR/the-file.cpp", --- {"jsonrpc":"2.0","id":10000,"method":"shutdown"} --- {"jsonrpc":"2.0","method":"exit"} + +# Generate a different compile_commands.json which does not point to the mock driver +# RUN: echo '[{"directory": "%/t.dir", "command": "gcc the-file.cpp -nostdinc --sysroot /my/sysroot/path -isysroot=/isysroot", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json + +# Generate a clangd config file which points to the mock driver instead +# RUN: echo 'CompileFlags:' > %t.dir/.clangd +# RUN: echo ' Compiler: my_driver.sh' >> %t.dir/.clangd + +# Run clangd a second time, to make sure it picks up the driver name from the config file +# Reuse the same input as above, but make different CHECKs because this time we'll +# get publishDiagnostics for the config files too. +# Note, we need to pass -enable-config because -lit-test otherwise disables it +# RUN: clangd -lit-test -enable-config -query-driver="**.test,**.sh" < %t.test | FileCheck -strict-whitespace %t.test --check-prefix=CHECK2 + +# Skip past the lack of diagnostics in the workspace and user config files... +# CHECK2: "method": "textDocument/publishDiagnostics", +# CHECK2-NEXT: "params": { +# CHECK2-NEXT: "diagnostics": [], +# CHECK2: "method": "textDocument/publishDiagnostics", +# CHECK2-NEXT: "params": { +# CHECK2-NEXT: "diagnostics": [], + +# ... to get to the interesting check: no diagnostics in the C++ file +# CHECK2: "method": "textDocument/publishDiagnostics", +# CHECK2-NEXT: "params": { +# CHECK2-NEXT: "diagnostics": [], +# CHECK2-NEXT: "uri": "file://INPUT_DIR/the-file.cpp", diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp --- a/clang-tools-extra/clangd/tool/Check.cpp +++ b/clang-tools-extra/clangd/tool/Check.cpp @@ -100,9 +100,9 @@ Config::current().CompileFlags.CDBSearch.FixedCDBPath; std::unique_ptr BaseCDB = std::make_unique(CDBOpts); - BaseCDB = getQueryDriverDatabase(llvm::makeArrayRef(Opts.QueryDriverGlobs), - std::move(BaseCDB)); auto Mangler = CommandMangler::detect(); + Mangler.SystemIncludeExtractor = + getSystemIncludeExtractor(llvm::makeArrayRef(Opts.QueryDriverGlobs)); if (Opts.ResourceDir) Mangler.ResourceDir = *Opts.ResourceDir; auto CDB = std::make_unique(