nridge updated this revision to Diff 472235.
nridge added a comment.
Improve handling of config file diagnostics in lit test
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133757/new/
https://reviews.llvm.org/D133757
Files:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/CompileCommands.cpp
clang-tools-extra/clangd/CompileCommands.h
clang-tools-extra/clangd/GlobalCompilationDatabase.h
clang-tools-extra/clangd/QueryDriverDatabase.cpp
clang-tools-extra/clangd/test/system-include-extractor.test
clang-tools-extra/clangd/tool/Check.cpp
Index: clang-tools-extra/clangd/tool/Check.cpp
===================================================================
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -100,9 +100,9 @@
Config::current().CompileFlags.CDBSearch.FixedCDBPath;
std::unique_ptr<GlobalCompilationDatabase> BaseCDB =
std::make_unique<DirectoryBasedGlobalCompilationDatabase>(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<OverlayCDB>(
Index: clang-tools-extra/clangd/test/system-include-extractor.test
===================================================================
--- clang-tools-extra/clangd/test/system-include-extractor.test
+++ clang-tools-extra/clangd/test/system-include-extractor.test
@@ -42,7 +42,9 @@
# 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
+# Note: include clangd's stderr in the FileCheck input with "2>&1" so that we
+# can match output lines like "ASTWorker building file"
+# RUN: clangd -lit-test -query-driver="**.test,**.sh" < %t.test 2>&1 | FileCheck -strict-whitespace %t.test
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}
---
{
@@ -57,10 +59,25 @@
}
}
}
+# Look for the "ASTWorker building file" line so that the subsequent diagnostics
+# that are matches are for the C++ source file and not a config file.
+# CHECK: ASTWorker building file
# CHECK: "method": "textDocument/publishDiagnostics",
# CHECK-NEXT: "params": {
# CHECK-NEXT: "diagnostics": [],
+# CHECK-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
+# 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 2>&1 | FileCheck -strict-whitespace %t.test
Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===================================================================
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ 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<std::string> QueryDriverGlobs,
- std::unique_ptr<GlobalCompilationDatabase> Base)
- : DelegatingCDB(std::move(Base)),
- QueryDriverRegex(convertGlobsToRegex(QueryDriverGlobs)) {}
+ SystemIncludeExtractor(llvm::ArrayRef<std::string> QueryDriverGlobs)
+ : QueryDriverRegex(convertGlobsToRegex(QueryDriverGlobs)) {}
- llvm::Optional<tooling::CompileCommand>
- 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<GlobalCompilationDatabase>
-getQueryDriverDatabase(llvm::ArrayRef<std::string> QueryDriverGlobs,
- std::unique_ptr<GlobalCompilationDatabase> Base) {
- assert(Base && "Null base to SystemIncludeExtractor");
+SystemIncludeExtractorFn
+getSystemIncludeExtractor(llvm::ArrayRef<std::string> QueryDriverGlobs) {
if (QueryDriverGlobs.empty())
- return Base;
- return std::make_unique<QueryDriverDatabase>(QueryDriverGlobs,
- std::move(Base));
+ return nullptr;
+ return SystemIncludeExtractor(QueryDriverGlobs);
}
} // namespace clangd
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.h
===================================================================
--- clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ 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<GlobalCompilationDatabase>
-getQueryDriverDatabase(llvm::ArrayRef<std::string> QueryDriverGlobs,
- std::unique_ptr<GlobalCompilationDatabase> Base);
+/// and adds them to the compile flags.
+/// Returns null when \p QueryDriverGlobs is empty.
+using SystemIncludeExtractorFn = llvm::unique_function<void(
+ tooling::CompileCommand &, llvm::StringRef) const>;
+SystemIncludeExtractorFn
+getSystemIncludeExtractor(llvm::ArrayRef<std::string> QueryDriverGlobs);
/// Wraps another compilation database, and supports overriding the commands
/// using an in-memory mapping.
Index: clang-tools-extra/clangd/CompileCommands.h
===================================================================
--- clang-tools-extra/clangd/CompileCommands.h
+++ clang-tools-extra/clangd/CompileCommands.h
@@ -32,6 +32,7 @@
llvm::Optional<std::string> ResourceDir;
// Root for searching for standard library (passed to -isysroot).
llvm::Optional<std::string> 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.
Index: clang-tools-extra/clangd/CompileCommands.cpp
===================================================================
--- clang-tools-extra/clangd/CompileCommands.cpp
+++ 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) {
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -502,10 +502,10 @@
CDBOpts.ContextProvider = Opts.ContextProvider;
BaseCDB =
std::make_unique<DirectoryBasedGlobalCompilationDatabase>(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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
