Revert "[clangd] Filter out non-governed files from broadcast"

This reverts commit d5214dfa7b.

It's causing failures, both in our local CI and the PS4 Windows bot.

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/26872/steps/test/logs/stdio

llvm-svn: 365678
This commit is contained in:
Matthew Voss 2019-07-10 18:16:35 +00:00
parent 89ed2e0a0a
commit 6d1a64e489
8 changed files with 69 additions and 331 deletions

View File

@ -8,18 +8,12 @@
#include "GlobalCompilationDatabase.h"
#include "Logger.h"
#include "Path.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Tooling/ArgumentsAdjusters.h"
#include "clang/Tooling/CompilationDatabase.h"
#include "llvm/ADT/None.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
#include <string>
#include <tuple>
#include <vector>
namespace clang {
namespace clangd {
@ -49,16 +43,6 @@ std::string getStandardResourceDir() {
return CompilerInvocation::GetResourcesPath("clangd", (void *)&Dummy);
}
// Runs the given action on all parent directories of filename, starting from
// deepest directory and going up to root. Stops whenever action succeeds.
void actOnAllParentDirectories(PathRef FileName,
llvm::function_ref<bool(PathRef)> Action) {
for (auto Path = llvm::sys::path::parent_path(FileName);
!Path.empty() && !Action(Path);
Path = llvm::sys::path::parent_path(Path))
;
}
} // namespace
static std::string getFallbackClangPath() {
@ -97,138 +81,60 @@ DirectoryBasedGlobalCompilationDatabase::
~DirectoryBasedGlobalCompilationDatabase() = default;
llvm::Optional<tooling::CompileCommand>
DirectoryBasedGlobalCompilationDatabase::getCompileCommand(PathRef File) const {
CDBLookupRequest Req;
Req.FileName = File;
Req.ShouldBroadcast = true;
auto Res = lookupCDB(Req);
if (!Res) {
DirectoryBasedGlobalCompilationDatabase::getCompileCommand(
PathRef File, ProjectInfo *Project) const {
if (auto CDB = getCDBForFile(File, Project)) {
auto Candidates = CDB->getCompileCommands(File);
if (!Candidates.empty()) {
return std::move(Candidates.front());
}
} else {
log("Failed to find compilation database for {0}", File);
return llvm::None;
}
auto Candidates = Res->CDB->getCompileCommands(File);
if (!Candidates.empty())
return std::move(Candidates.front());
return None;
}
std::pair<tooling::CompilationDatabase *, /*SentBroadcast*/ bool>
std::pair<tooling::CompilationDatabase *, /*Cached*/ bool>
DirectoryBasedGlobalCompilationDatabase::getCDBInDirLocked(PathRef Dir) const {
// FIXME(ibiryukov): Invalidate cached compilation databases on changes
auto CachedIt = CompilationDatabases.find(Dir);
if (CachedIt != CompilationDatabases.end())
return {CachedIt->second.CDB.get(), CachedIt->second.SentBroadcast};
return {CachedIt->second.get(), true};
std::string Error = "";
CachedCDB Entry;
Entry.CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);
auto Result = Entry.CDB.get();
CompilationDatabases[Dir] = std::move(Entry);
auto CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);
auto Result = CDB.get();
CompilationDatabases.insert(std::make_pair(Dir, std::move(CDB)));
return {Result, false};
}
llvm::Optional<DirectoryBasedGlobalCompilationDatabase::CDBLookupResult>
DirectoryBasedGlobalCompilationDatabase::lookupCDB(
CDBLookupRequest Request) const {
assert(llvm::sys::path::is_absolute(Request.FileName) &&
tooling::CompilationDatabase *
DirectoryBasedGlobalCompilationDatabase::getCDBForFile(
PathRef File, ProjectInfo *Project) const {
namespace path = llvm::sys::path;
assert((path::is_absolute(File, path::Style::posix) ||
path::is_absolute(File, path::Style::windows)) &&
"path must be absolute");
CDBLookupResult Result;
bool SentBroadcast = false;
{
std::lock_guard<std::mutex> Lock(Mutex);
if (CompileCommandsDir) {
std::tie(Result.CDB, SentBroadcast) =
getCDBInDirLocked(*CompileCommandsDir);
Result.PI.SourceRoot = *CompileCommandsDir;
} else {
actOnAllParentDirectories(
Request.FileName, [this, &SentBroadcast, &Result](PathRef Path) {
std::tie(Result.CDB, SentBroadcast) = getCDBInDirLocked(Path);
Result.PI.SourceRoot = Path;
return Result.CDB != nullptr;
});
}
if (!Result.CDB)
return llvm::None;
// Mark CDB as broadcasted to make sure discovery is performed once.
if (Request.ShouldBroadcast && !SentBroadcast)
CompilationDatabases[Result.PI.SourceRoot].SentBroadcast = true;
}
// FIXME: Maybe make the following part async, since this can block retrieval
// of compile commands.
if (Request.ShouldBroadcast && !SentBroadcast)
broadcastCDB(Result);
return Result;
}
void DirectoryBasedGlobalCompilationDatabase::broadcastCDB(
CDBLookupResult Result) const {
assert(Result.CDB && "Trying to broadcast an invalid CDB!");
std::vector<std::string> AllFiles = Result.CDB->getAllFiles();
// We assume CDB in CompileCommandsDir owns all of its entries, since we don't
// perform any search in parent paths whenever it is set.
tooling::CompilationDatabase *CDB = nullptr;
bool Cached = false;
std::lock_guard<std::mutex> Lock(Mutex);
if (CompileCommandsDir) {
assert(*CompileCommandsDir == Result.PI.SourceRoot &&
"Trying to broadcast a CDB outside of CompileCommandsDir!");
OnCommandChanged.broadcast(std::move(AllFiles));
return;
}
llvm::StringMap<bool> DirectoryHasCDB;
{
std::lock_guard<std::mutex> Lock(Mutex);
// Uniquify all parent directories of all files.
for (llvm::StringRef File : AllFiles) {
actOnAllParentDirectories(File, [&](PathRef Path) {
auto It = DirectoryHasCDB.try_emplace(Path);
// Already seen this path, and all of its parents.
if (!It.second)
return true;
auto Res = getCDBInDirLocked(Path);
It.first->second = Res.first != nullptr;
return Path == Result.PI.SourceRoot;
});
std::tie(CDB, Cached) = getCDBInDirLocked(*CompileCommandsDir);
if (Project && CDB)
Project->SourceRoot = *CompileCommandsDir;
} else {
for (auto Path = path::parent_path(File); !CDB && !Path.empty();
Path = path::parent_path(Path)) {
std::tie(CDB, Cached) = getCDBInDirLocked(Path);
if (Project && CDB)
Project->SourceRoot = Path;
}
}
std::vector<std::string> GovernedFiles;
for (llvm::StringRef File : AllFiles) {
// A file is governed by this CDB if lookup for the file would find it.
// Independent of whether it has an entry for that file or not.
actOnAllParentDirectories(File, [&](PathRef Path) {
if (DirectoryHasCDB.lookup(Path)) {
if (Path == Result.PI.SourceRoot)
GovernedFiles.push_back(File);
// Stop as soon as we hit a CDB.
return true;
}
return false;
});
}
OnCommandChanged.broadcast(std::move(GovernedFiles));
}
llvm::Optional<ProjectInfo>
DirectoryBasedGlobalCompilationDatabase::getProjectInfo(PathRef File) const {
CDBLookupRequest Req;
Req.FileName = File;
Req.ShouldBroadcast = false;
auto Res = lookupCDB(Req);
if (!Res)
return llvm::None;
return Res->PI;
// FIXME: getAllFiles() may return relative paths, we need absolute paths.
// Hopefully the fix is to change JSONCompilationDatabase and the interface.
if (CDB && !Cached)
OnCommandChanged.broadcast(CDB->getAllFiles());
return CDB;
}
OverlayCDB::OverlayCDB(const GlobalCompilationDatabase *Base,
@ -244,16 +150,19 @@ OverlayCDB::OverlayCDB(const GlobalCompilationDatabase *Base,
}
llvm::Optional<tooling::CompileCommand>
OverlayCDB::getCompileCommand(PathRef File) const {
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 (It != Commands.end()) {
if (Project)
Project->SourceRoot = "";
Cmd = It->second;
}
}
if (!Cmd && Base)
Cmd = Base->getCompileCommand(File);
Cmd = Base->getCompileCommand(File, Project);
if (!Cmd)
return llvm::None;
adjustArguments(*Cmd, ResourceDir);
@ -282,17 +191,5 @@ void OverlayCDB::setCompileCommand(
OnCommandChanged.broadcast({File});
}
llvm::Optional<ProjectInfo> OverlayCDB::getProjectInfo(PathRef File) const {
{
std::lock_guard<std::mutex> Lock(Mutex);
auto It = Commands.find(File);
if (It != Commands.end())
return ProjectInfo{};
}
if (Base)
return Base->getProjectInfo(File);
return llvm::None;
}
} // namespace clangd
} // namespace clang

View File

@ -40,13 +40,9 @@ public:
virtual ~GlobalCompilationDatabase() = default;
/// If there are any known-good commands for building this file, returns one.
/// If the ProjectInfo pointer is set, it will also be populated.
virtual llvm::Optional<tooling::CompileCommand>
getCompileCommand(PathRef File) const = 0;
/// Finds the closest project to \p File.
virtual llvm::Optional<ProjectInfo> getProjectInfo(PathRef File) const {
return llvm::None;
}
getCompileCommand(PathRef File, ProjectInfo * = nullptr) const = 0;
/// Makes a guess at how to build a file.
/// The default implementation just runs clang on the file.
@ -75,40 +71,20 @@ public:
/// Scans File's parents looking for compilation databases.
/// Any extra flags will be added.
/// Might trigger OnCommandChanged, if CDB wasn't broadcasted yet.
llvm::Optional<tooling::CompileCommand>
getCompileCommand(PathRef File) const override;
/// Returns the path to first directory containing a compilation database in
/// \p File's parents.
llvm::Optional<ProjectInfo> getProjectInfo(PathRef File) const override;
getCompileCommand(PathRef File, ProjectInfo * = nullptr) const override;
private:
std::pair<tooling::CompilationDatabase *, /*SentBroadcast*/ bool>
tooling::CompilationDatabase *getCDBForFile(PathRef File,
ProjectInfo *) const;
std::pair<tooling::CompilationDatabase *, /*Cached*/ bool>
getCDBInDirLocked(PathRef File) const;
struct CDBLookupRequest {
PathRef FileName;
// Whether this lookup should trigger discovery of the CDB found.
bool ShouldBroadcast = false;
};
struct CDBLookupResult {
tooling::CompilationDatabase *CDB = nullptr;
ProjectInfo PI;
};
llvm::Optional<CDBLookupResult> lookupCDB(CDBLookupRequest Request) const;
// Performs broadcast on governed files.
void broadcastCDB(CDBLookupResult Res) const;
mutable std::mutex Mutex;
/// Caches compilation databases loaded from directories(keys are
/// directories).
struct CachedCDB {
std::unique_ptr<clang::tooling::CompilationDatabase> CDB = nullptr;
bool SentBroadcast = false;
};
mutable llvm::StringMap<CachedCDB> CompilationDatabases;
mutable llvm::StringMap<std::unique_ptr<clang::tooling::CompilationDatabase>>
CompilationDatabases;
/// Used for command argument pointing to folder where compile_commands.json
/// is located.
@ -133,9 +109,8 @@ public:
llvm::Optional<std::string> ResourceDir = llvm::None);
llvm::Optional<tooling::CompileCommand>
getCompileCommand(PathRef File) const override;
getCompileCommand(PathRef File, ProjectInfo * = nullptr) const override;
tooling::CompileCommand getFallbackCommand(PathRef File) const override;
llvm::Optional<ProjectInfo> getProjectInfo(PathRef File) const override;
/// Sets or clears the compilation command for a particular file.
void

View File

@ -215,8 +215,8 @@ public:
}
llvm::Optional<tooling::CompileCommand>
getCompileCommand(PathRef File) const override {
auto Cmd = Base->getCompileCommand(File);
getCompileCommand(PathRef File, ProjectInfo *PI = nullptr) const override {
auto Cmd = Base->getCompileCommand(File, PI);
if (!Cmd || Cmd->CommandLine.empty())
return Cmd;
@ -240,10 +240,6 @@ public:
return addSystemIncludes(*Cmd, SystemIncludes);
}
llvm::Optional<ProjectInfo> getProjectInfo(PathRef File) const override {
return Base->getProjectInfo(File);
}
private:
mutable std::mutex Mu;
// Caches includes extracted from a driver.

View File

@ -619,15 +619,11 @@ BackgroundIndex::loadShards(std::vector<std::string> ChangedFiles) {
llvm::StringSet<> LoadedShards;
Rebuilder.startLoading();
for (const auto &File : ChangedFiles) {
auto Cmd = CDB.getCompileCommand(File);
ProjectInfo PI;
auto Cmd = CDB.getCompileCommand(File, &PI);
if (!Cmd)
continue;
std::string ProjectRoot;
if (auto PI = CDB.getProjectInfo(File))
ProjectRoot = std::move(PI->SourceRoot);
BackgroundIndexStorage *IndexStorage = IndexStorageFactory(ProjectRoot);
BackgroundIndexStorage *IndexStorage = IndexStorageFactory(PI.SourceRoot);
auto Dependencies = loadShard(*Cmd, IndexStorage, LoadedShards);
for (const auto &Dependency : Dependencies) {
if (!Dependency.NeedsReIndexing || FilesToIndex.count(Dependency.Path))

View File

@ -1064,7 +1064,7 @@ TEST_F(ClangdVFSTest, FallbackWhenPreambleIsNotReady) {
ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
auto FooCpp = testPath("foo.cpp");
Annotations Code(R"cpp(
Annotations Code(R"cpp(
namespace ns { int xyz; }
using namespace ns;
int main() {
@ -1113,7 +1113,7 @@ TEST_F(ClangdVFSTest, FallbackWhenWaitingForCompileCommand) {
: CanReturnCommand(CanReturnCommand) {}
llvm::Optional<tooling::CompileCommand>
getCompileCommand(PathRef File) const override {
getCompileCommand(PathRef File, ProjectInfo * = nullptr) const override {
// FIXME: make this timeout and fail instead of waiting forever in case
// something goes wrong.
CanReturnCommand.wait();

View File

@ -8,21 +8,10 @@
#include "GlobalCompilationDatabase.h"
#include "Path.h"
#include "TestFS.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/raw_ostream.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include <fstream>
#include <string>
namespace clang {
namespace clangd {
@ -31,10 +20,8 @@ using ::testing::AllOf;
using ::testing::Contains;
using ::testing::ElementsAre;
using ::testing::EndsWith;
using ::testing::IsEmpty;
using ::testing::Not;
using ::testing::StartsWith;
using ::testing::UnorderedElementsAre;
TEST(GlobalCompilationDatabaseTest, FallbackCommand) {
DirectoryBasedGlobalCompilationDatabase DB(None);
@ -63,9 +50,13 @@ class OverlayCDBTest : public ::testing::Test {
class BaseCDB : public GlobalCompilationDatabase {
public:
llvm::Optional<tooling::CompileCommand>
getCompileCommand(llvm::StringRef File) const override {
if (File == testPath("foo.cc"))
getCompileCommand(llvm::StringRef File,
ProjectInfo *Project) const override {
if (File == testPath("foo.cc")) {
if (Project)
Project->SourceRoot = testRoot();
return cmd(File, "-DA=1");
}
return None;
}
@ -73,10 +64,6 @@ class OverlayCDBTest : public ::testing::Test {
getFallbackCommand(llvm::StringRef File) const override {
return cmd(File, "-DA=2");
}
llvm::Optional<ProjectInfo> getProjectInfo(PathRef File) const override {
return ProjectInfo{testRoot()};
}
};
protected:
@ -166,109 +153,6 @@ TEST_F(OverlayCDBTest, Adjustments) {
Not(Contains("random-plugin"))));
}
TEST(GlobalCompilationDatabaseTest, DiscoveryWithNestedCDBs) {
const char *const CDBOuter =
R"cdb(
[
{
"file": "a.cc",
"command": "",
"directory": "{0}",
},
{
"file": "build/gen.cc",
"command": "",
"directory": "{0}",
},
{
"file": "build/gen2.cc",
"command": "",
"directory": "{0}",
}
]
)cdb";
const char *const CDBInner =
R"cdb(
[
{
"file": "gen.cc",
"command": "",
"directory": "{0}/build",
}
]
)cdb";
class CleaningFS {
public:
llvm::SmallString<128> Root;
CleaningFS() {
EXPECT_FALSE(
llvm::sys::fs::createUniqueDirectory("clangd-cdb-test", Root))
<< "Failed to create unique directory";
}
~CleaningFS() {
EXPECT_FALSE(llvm::sys::fs::remove_directories(Root))
<< "Failed to cleanup " << Root;
}
void registerFile(PathRef RelativePath, llvm::StringRef Contents) {
llvm::SmallString<128> AbsPath(Root);
llvm::sys::path::append(AbsPath, RelativePath);
EXPECT_FALSE(llvm::sys::fs::create_directories(
llvm::sys::path::parent_path(AbsPath)))
<< "Failed to create directories for: " << AbsPath;
std::error_code EC;
llvm::raw_fd_ostream OS(AbsPath, EC);
EXPECT_FALSE(EC) << "Failed to open " << AbsPath << " for writing";
OS << llvm::formatv(Contents.data(), Root);
OS.close();
EXPECT_FALSE(OS.has_error());
}
};
CleaningFS FS;
FS.registerFile("compile_commands.json", CDBOuter);
FS.registerFile("build/compile_commands.json", CDBInner);
// Note that gen2.cc goes missing with our following model, not sure this
// happens in practice though.
{
DirectoryBasedGlobalCompilationDatabase DB(llvm::None);
std::vector<std::string> DiscoveredFiles;
auto Sub =
DB.watch([&DiscoveredFiles](const std::vector<std::string> Changes) {
DiscoveredFiles = Changes;
});
DB.getCompileCommand((FS.Root + "/a.cc").str());
EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(EndsWith("a.cc")));
DB.getCompileCommand((FS.Root + "/build/gen.cc").str());
EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(EndsWith("gen.cc")));
}
// With a custom compile commands dir.
{
DirectoryBasedGlobalCompilationDatabase DB(FS.Root.str().str());
std::vector<std::string> DiscoveredFiles;
auto Sub =
DB.watch([&DiscoveredFiles](const std::vector<std::string> Changes) {
DiscoveredFiles = Changes;
});
DB.getCompileCommand((FS.Root + "/a.cc").str());
EXPECT_THAT(DiscoveredFiles,
UnorderedElementsAre(EndsWith("a.cc"), EndsWith("gen.cc"),
EndsWith("gen2.cc")));
DiscoveredFiles.clear();
DB.getCompileCommand((FS.Root + "/build/gen.cc").str());
EXPECT_THAT(DiscoveredFiles, IsEmpty());
}
}
} // namespace
} // namespace clangd
} // namespace clang

View File

@ -6,11 +6,7 @@
//
//===----------------------------------------------------------------------===//
#include "TestFS.h"
#include "GlobalCompilationDatabase.h"
#include "Path.h"
#include "URI.h"
#include "llvm/ADT/None.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/Path.h"
@ -40,13 +36,9 @@ MockCompilationDatabase::MockCompilationDatabase(llvm::StringRef Directory,
// -ffreestanding avoids implicit stdc-predef.h.
}
llvm::Optional<ProjectInfo>
MockCompilationDatabase::getProjectInfo(PathRef File) const {
return ProjectInfo{Directory};
};
llvm::Optional<tooling::CompileCommand>
MockCompilationDatabase::getCompileCommand(PathRef File) const {
MockCompilationDatabase::getCompileCommand(PathRef File,
ProjectInfo *Project) const {
if (ExtraClangFlags.empty())
return None;
@ -65,6 +57,8 @@ MockCompilationDatabase::getCompileCommand(PathRef File) const {
CommandLine.push_back(RelativeFilePath.str());
}
if (Project)
Project->SourceRoot = Directory;
return {tooling::CompileCommand(Directory != llvm::StringRef()
? Directory
: llvm::sys::path::parent_path(File),

View File

@ -12,8 +12,6 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTFS_H
#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTFS_H
#include "ClangdServer.h"
#include "GlobalCompilationDatabase.h"
#include "Path.h"
#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/VirtualFileSystem.h"
@ -50,9 +48,7 @@ public:
StringRef RelPathPrefix = StringRef());
llvm::Optional<tooling::CompileCommand>
getCompileCommand(PathRef File) const override;
llvm::Optional<ProjectInfo> getProjectInfo(PathRef File) const override;
getCompileCommand(PathRef File, ProjectInfo * = nullptr) const override;
std::vector<std::string> ExtraClangFlags;