[clang-tools-extra] r351788 - [clangd] Filter out plugin related flags and move all commandline manipulations into OverlayCDB.
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 22 01:10:21 PST 2019
Author: kadircet
Date: Tue Jan 22 01:10:20 2019
New Revision: 351788
URL: http://llvm.org/viewvc/llvm-project?rev=351788&view=rev
Log:
[clangd] Filter out plugin related flags and move all commandline manipulations into OverlayCDB.
Summary:
Some projects make use of clang plugins when building, but clangd is
not aware of those plugins therefore can't work with the same compile command
arguments.
There were multiple places clangd performed commandline manipulations,
this one also moves them all into OverlayCDB.
Reviewers: ilya-biryukov
Subscribers: klimek, sammccall, ioeric, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D56841
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/clangd/index/Background.h
clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=351788&r1=351787&r2=351788&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Tue Jan 22 01:10:20 2019
@@ -289,7 +289,8 @@ void ClangdLSPServer::onInitialize(const
if (UseDirBasedCDB)
BaseCDB = llvm::make_unique<DirectoryBasedGlobalCompilationDatabase>(
CompileCommandsDir);
- CDB.emplace(BaseCDB.get(), Params.initializationOptions.fallbackFlags);
+ CDB.emplace(BaseCDB.get(), Params.initializationOptions.fallbackFlags,
+ ClangdServerOpts.ResourceDir);
Server.emplace(*CDB, FSProvider, static_cast<DiagnosticsConsumer &>(*this),
ClangdServerOpts);
applyConfiguration(Params.initializationOptions.ConfigSettings);
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=351788&r1=351787&r2=351788&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Jan 22 01:10:20 2019
@@ -37,11 +37,6 @@ namespace clang {
namespace clangd {
namespace {
-std::string getStandardResourceDir() {
- static int Dummy; // Just an address in this process.
- return CompilerInvocation::GetResourcesPath("clangd", (void *)&Dummy);
-}
-
class RefactoringResultCollector final
: public tooling::RefactoringResultConsumer {
public:
@@ -107,8 +102,6 @@ ClangdServer::ClangdServer(const GlobalC
DiagnosticsConsumer &DiagConsumer,
const Options &Opts)
: CDB(CDB), FSProvider(FSProvider),
- ResourceDir(Opts.ResourceDir ? *Opts.ResourceDir
- : getStandardResourceDir()),
DynamicIdx(Opts.BuildDynamicSymbolIndex
? new FileIndex(Opts.HeavyweightDynamicSymbolIndex)
: nullptr),
@@ -136,7 +129,7 @@ ClangdServer::ClangdServer(const GlobalC
AddIndex(Opts.StaticIndex);
if (Opts.BackgroundIndex) {
BackgroundIdx = llvm::make_unique<BackgroundIndex>(
- Context::current().clone(), ResourceDir, FSProvider, CDB,
+ Context::current().clone(), FSProvider, CDB,
BackgroundIndexStorage::createDiskBackedStorageFactory(),
Opts.BackgroundIndexRebuildPeriodMs);
AddIndex(BackgroundIdx.get());
@@ -461,10 +454,6 @@ tooling::CompileCommand ClangdServer::ge
llvm::Optional<tooling::CompileCommand> C = CDB.getCompileCommand(File);
if (!C) // FIXME: Suppress diagnostics? Let the user know?
C = CDB.getFallbackCommand(File);
-
- // Inject the resource dir.
- // FIXME: Don't overwrite it if it's already there.
- C->CommandLine.push_back("-resource-dir=" + ResourceDir);
return std::move(*C);
}
Modified: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp?rev=351788&r1=351787&r2=351788&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp (original)
+++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp Tue Jan 22 01:10:20 2019
@@ -8,12 +8,36 @@
#include "GlobalCompilationDatabase.h"
#include "Logger.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Tooling/ArgumentsAdjusters.h"
#include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
namespace clang {
namespace clangd {
+namespace {
+
+void adjustArguments(tooling::CompileCommand &Cmd,
+ llvm::StringRef ResourceDir) {
+ // Strip plugin related command line arguments. Clangd does
+ // not support plugins currently. Therefore it breaks if
+ // compiler tries to load plugins.
+ Cmd.CommandLine =
+ tooling::getStripPluginsAdjuster()(Cmd.CommandLine, Cmd.Filename);
+ // Inject the resource dir.
+ // FIXME: Don't overwrite it if it's already there.
+ if (!ResourceDir.empty())
+ Cmd.CommandLine.push_back(("-resource-dir=" + ResourceDir).str());
+}
+
+std::string getStandardResourceDir() {
+ static int Dummy; // Just an address in this process.
+ return CompilerInvocation::GetResourcesPath("clangd", (void *)&Dummy);
+}
+
+} // namespace
static std::string getFallbackClangPath() {
static int Dummy;
@@ -105,8 +129,11 @@ DirectoryBasedGlobalCompilationDatabase:
}
OverlayCDB::OverlayCDB(const GlobalCompilationDatabase *Base,
- std::vector<std::string> FallbackFlags)
- : Base(Base), FallbackFlags(std::move(FallbackFlags)) {
+ std::vector<std::string> FallbackFlags,
+ llvm::Optional<std::string> ResourceDir)
+ : Base(Base), ResourceDir(ResourceDir ? std::move(*ResourceDir)
+ : getStandardResourceDir()),
+ FallbackFlags(std::move(FallbackFlags)) {
if (Base)
BaseChanged = Base->watch([this](const std::vector<std::string> Changes) {
OnCommandChanged.broadcast(Changes);
@@ -115,16 +142,22 @@ OverlayCDB::OverlayCDB(const GlobalCompi
llvm::Optional<tooling::CompileCommand>
OverlayCDB::getCompileCommand(PathRef File, ProjectInfo *Project) const {
+ llvm::Optional<tooling::CompileCommand> Cmd;
{
std::lock_guard<std::mutex> Lock(Mutex);
auto It = Commands.find(File);
if (It != Commands.end()) {
if (Project)
Project->SourceRoot = "";
- return It->second;
+ Cmd = It->second;
}
}
- return Base ? Base->getCompileCommand(File, Project) : None;
+ if (!Cmd && Base)
+ Cmd = Base->getCompileCommand(File, Project);
+ if (!Cmd)
+ return llvm::None;
+ adjustArguments(*Cmd, ResourceDir);
+ return Cmd;
}
tooling::CompileCommand OverlayCDB::getFallbackCommand(PathRef File) const {
Modified: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h?rev=351788&r1=351787&r2=351788&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h (original)
+++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h Tue Jan 22 01:10:20 2019
@@ -11,6 +11,7 @@
#include "Function.h"
#include "Path.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringMap.h"
#include <memory>
#include <mutex>
@@ -97,7 +98,8 @@ public:
// Base may be null, in which case no entries are inherited.
// FallbackFlags are added to the fallback compile command.
OverlayCDB(const GlobalCompilationDatabase *Base,
- std::vector<std::string> FallbackFlags = {});
+ std::vector<std::string> FallbackFlags = {},
+ llvm::Optional<std::string> ResourceDir = llvm::None);
llvm::Optional<tooling::CompileCommand>
getCompileCommand(PathRef File, ProjectInfo * = nullptr) const override;
@@ -112,6 +114,7 @@ private:
mutable std::mutex Mutex;
llvm::StringMap<tooling::CompileCommand> Commands; /* GUARDED_BY(Mut) */
const GlobalCompilationDatabase *Base;
+ std::string ResourceDir;
std::vector<std::string> FallbackFlags;
CommandChanged::Subscription BaseChanged;
};
Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=351788&r1=351787&r2=351788&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Tue Jan 22 01:10:20 2019
@@ -126,13 +126,12 @@ llvm::SmallString<128> getAbsolutePath(c
} // namespace
BackgroundIndex::BackgroundIndex(
- Context BackgroundContext, llvm::StringRef ResourceDir,
- const FileSystemProvider &FSProvider, const GlobalCompilationDatabase &CDB,
+ Context BackgroundContext, const FileSystemProvider &FSProvider,
+ const GlobalCompilationDatabase &CDB,
BackgroundIndexStorage::Factory IndexStorageFactory,
size_t BuildIndexPeriodMs, size_t ThreadPoolSize)
- : SwapIndex(llvm::make_unique<MemIndex>()), ResourceDir(ResourceDir),
- FSProvider(FSProvider), CDB(CDB),
- BackgroundContext(std::move(BackgroundContext)),
+ : SwapIndex(llvm::make_unique<MemIndex>()), FSProvider(FSProvider),
+ CDB(CDB), BackgroundContext(std::move(BackgroundContext)),
BuildIndexPeriodMs(BuildIndexPeriodMs),
SymbolsUpdatedSinceLastIndex(false),
IndexStorageFactory(std::move(IndexStorageFactory)),
@@ -229,7 +228,6 @@ void BackgroundIndex::enqueue(tooling::C
BackgroundIndexStorage *Storage) {
enqueueTask(Bind(
[this, Storage](tooling::CompileCommand Cmd) {
- Cmd.CommandLine.push_back("-resource-dir=" + ResourceDir);
// 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;
Modified: clang-tools-extra/trunk/clangd/index/Background.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.h?rev=351788&r1=351787&r2=351788&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Background.h (original)
+++ clang-tools-extra/trunk/clangd/index/Background.h Tue Jan 22 01:10:20 2019
@@ -67,9 +67,7 @@ public:
/// If BuildIndexPeriodMs is greater than 0, the symbol index will only be
/// rebuilt periodically (one per \p BuildIndexPeriodMs); otherwise, index is
/// rebuilt for each indexed file.
- // FIXME: resource-dir injection should be hoisted somewhere common.
- BackgroundIndex(Context BackgroundContext, llvm::StringRef ResourceDir,
- const FileSystemProvider &,
+ BackgroundIndex(Context BackgroundContext, const FileSystemProvider &,
const GlobalCompilationDatabase &CDB,
BackgroundIndexStorage::Factory IndexStorageFactory,
size_t BuildIndexPeriodMs = 0,
@@ -98,7 +96,6 @@ private:
BackgroundIndexStorage *IndexStorage);
// configuration
- std::string ResourceDir;
const FileSystemProvider &FSProvider;
const GlobalCompilationDatabase &CDB;
Context BackgroundContext;
Modified: clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp?rev=351788&r1=351787&r2=351788&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp Tue Jan 22 01:10:20 2019
@@ -76,7 +76,7 @@ TEST_F(BackgroundIndexTest, NoCrashOnErr
size_t CacheHits = 0;
MemoryShardStorage MSS(Storage, CacheHits);
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), "", FS, CDB,
+ BackgroundIndex Idx(Context::empty(), FS, CDB,
[&](llvm::StringRef) { return &MSS; });
tooling::CompileCommand Cmd;
@@ -113,7 +113,7 @@ TEST_F(BackgroundIndexTest, IndexTwoFile
size_t CacheHits = 0;
MemoryShardStorage MSS(Storage, CacheHits);
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), "", FS, CDB,
+ BackgroundIndex Idx(Context::empty(), FS, CDB,
[&](llvm::StringRef) { return &MSS; });
tooling::CompileCommand Cmd;
@@ -168,7 +168,7 @@ TEST_F(BackgroundIndexTest, ShardStorage
// Check nothing is loaded from Storage, but A.cc and A.h has been stored.
{
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), "", FS, CDB,
+ BackgroundIndex Idx(Context::empty(), FS, CDB,
[&](llvm::StringRef) { return &MSS; });
CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
ASSERT_TRUE(Idx.blockUntilIdleForTest());
@@ -178,7 +178,7 @@ TEST_F(BackgroundIndexTest, ShardStorage
{
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), "", FS, CDB,
+ BackgroundIndex Idx(Context::empty(), FS, CDB,
[&](llvm::StringRef) { return &MSS; });
CDB.setCompileCommand(testPath("root"), Cmd);
ASSERT_TRUE(Idx.blockUntilIdleForTest());
@@ -224,7 +224,7 @@ TEST_F(BackgroundIndexTest, DirectInclud
Cmd.CommandLine = {"clang++", testPath("root/A.cc")};
{
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), "", FS, CDB,
+ BackgroundIndex Idx(Context::empty(), FS, CDB,
[&](llvm::StringRef) { return &MSS; });
CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
ASSERT_TRUE(Idx.blockUntilIdleForTest());
@@ -262,7 +262,7 @@ TEST_F(BackgroundIndexTest, DISABLED_Per
MemoryShardStorage MSS(Storage, CacheHits);
OverlayCDB CDB(/*Base=*/nullptr);
BackgroundIndex Idx(
- Context::empty(), "", FS, CDB, [&](llvm::StringRef) { return &MSS; },
+ Context::empty(), FS, CDB, [&](llvm::StringRef) { return &MSS; },
/*BuildIndexPeriodMs=*/500);
FS.Files[testPath("root/A.cc")] = "#include \"A.h\"";
@@ -310,7 +310,7 @@ TEST_F(BackgroundIndexTest, ShardStorage
// Check nothing is loaded from Storage, but A.cc and A.h has been stored.
{
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), "", FS, CDB,
+ BackgroundIndex Idx(Context::empty(), FS, CDB,
[&](llvm::StringRef) { return &MSS; });
CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
ASSERT_TRUE(Idx.blockUntilIdleForTest());
@@ -325,7 +325,7 @@ TEST_F(BackgroundIndexTest, ShardStorage
)cpp";
{
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), "", FS, CDB,
+ BackgroundIndex Idx(Context::empty(), FS, CDB,
[&](llvm::StringRef) { return &MSS; });
CDB.setCompileCommand(testPath("root"), Cmd);
ASSERT_TRUE(Idx.blockUntilIdleForTest());
@@ -343,7 +343,7 @@ TEST_F(BackgroundIndexTest, ShardStorage
{
CacheHits = 0;
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), "", FS, CDB,
+ BackgroundIndex Idx(Context::empty(), FS, CDB,
[&](llvm::StringRef) { return &MSS; });
CDB.setCompileCommand(testPath("root"), Cmd);
ASSERT_TRUE(Idx.blockUntilIdleForTest());
@@ -384,7 +384,7 @@ TEST_F(BackgroundIndexTest, ShardStorage
// Check that A.cc, A.h and B.h has been stored.
{
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), "", FS, CDB,
+ BackgroundIndex Idx(Context::empty(), FS, CDB,
[&](llvm::StringRef) { return &MSS; });
CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
ASSERT_TRUE(Idx.blockUntilIdleForTest());
@@ -400,7 +400,7 @@ TEST_F(BackgroundIndexTest, ShardStorage
{
CacheHits = 0;
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), "", FS, CDB,
+ BackgroundIndex Idx(Context::empty(), FS, CDB,
[&](llvm::StringRef) { return &MSS; });
CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
ASSERT_TRUE(Idx.blockUntilIdleForTest());
@@ -416,7 +416,7 @@ TEST_F(BackgroundIndexTest, ShardStorage
{
CacheHits = 0;
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), "", FS, CDB,
+ BackgroundIndex Idx(Context::empty(), FS, CDB,
[&](llvm::StringRef) { return &MSS; });
CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
ASSERT_TRUE(Idx.blockUntilIdleForTest());
Modified: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp?rev=351788&r1=351787&r2=351788&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Tue Jan 22 01:10:20 2019
@@ -9,6 +9,7 @@
#include "Annotations.h"
#include "ClangdLSPServer.h"
#include "ClangdServer.h"
+#include "GlobalCompilationDatabase.h"
#include "Matchers.h"
#include "SyncAPI.h"
#include "TestFS.h"
@@ -1036,6 +1037,28 @@ TEST(ClangdTests, PreambleVFSStatCache)
}
#endif
+TEST_F(ClangdVFSTest, FlagsWithPlugins) {
+ MockFSProvider FS;
+ ErrorCheckingDiagConsumer DiagConsumer;
+ MockCompilationDatabase CDB;
+ CDB.ExtraClangFlags = {
+ "-Xclang",
+ "-add-plugin",
+ "-Xclang",
+ "random-plugin",
+ };
+ OverlayCDB OCDB(&CDB);
+ ClangdServer Server(OCDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+ auto FooCpp = testPath("foo.cpp");
+ const auto SourceContents = "int main() { return 0; }";
+ FS.Files[FooCpp] = FooCpp;
+ Server.addDocument(FooCpp, SourceContents);
+ auto Result = dumpASTWithoutMemoryLocs(Server, FooCpp);
+ EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
+ EXPECT_NE(Result, "<no-ast>");
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Modified: clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp?rev=351788&r1=351787&r2=351788&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp Tue Jan 22 01:10:20 2019
@@ -64,7 +64,7 @@ protected:
};
TEST_F(OverlayCDBTest, GetCompileCommand) {
- OverlayCDB CDB(Base.get());
+ OverlayCDB CDB(Base.get(), {}, std::string(""));
EXPECT_EQ(CDB.getCompileCommand(testPath("foo.cc")),
Base->getCompileCommand(testPath("foo.cc")));
EXPECT_EQ(CDB.getCompileCommand(testPath("missing.cc")), llvm::None);
@@ -84,7 +84,7 @@ TEST_F(OverlayCDBTest, GetFallbackComman
}
TEST_F(OverlayCDBTest, NoBase) {
- OverlayCDB CDB(nullptr, {"-DA=6"});
+ OverlayCDB CDB(nullptr, {"-DA=6"}, std::string(""));
EXPECT_EQ(CDB.getCompileCommand(testPath("bar.cc")), None);
auto Override = cmd(testPath("bar.cc"), "-DA=5");
CDB.setCompileCommand(testPath("bar.cc"), Override);
More information about the cfe-commits
mailing list