Skip to content
2 changes: 2 additions & 0 deletions clang-tools-extra/clangd/ClangdLSPServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,8 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params,
if (const auto &Dir = Params.initializationOptions.compilationDatabasePath)
CDBOpts.CompileCommandsDir = Dir;
CDBOpts.ContextProvider = Opts.ContextProvider;
if (Opts.StrongWorkspaceMode)
CDBOpts.applyWorkingDirectory(std::move(Opts.WorkspaceRoot));
BaseCDB =
std::make_unique<DirectoryBasedGlobalCompilationDatabase>(CDBOpts);
}
Expand Down
5 changes: 5 additions & 0 deletions clang-tools-extra/clangd/ClangdServer.h
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ class ClangdServer {
/// FIXME: If not set, should use the current working directory.
std::optional<std::string> WorkspaceRoot;

/// Sets an alterante mode of operation. Current effects are:
/// - Using the current working directory as the working directory for
/// fallback commands
bool StrongWorkspaceMode;

/// The resource directory is used to find internal headers, overriding
/// defaults and -resource-dir compiler flag).
/// If std::nullopt, ClangdServer calls
Expand Down
36 changes: 26 additions & 10 deletions clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,10 @@ GlobalCompilationDatabase::getFallbackCommand(PathRef File) const {
if (FileExtension.empty() || FileExtension == ".h")
Argv.push_back("-xobjective-c++-header");
Argv.push_back(std::string(File));
tooling::CompileCommand Cmd(llvm::sys::path::parent_path(File),
llvm::sys::path::filename(File), std::move(Argv),
/*Output=*/"");
tooling::CompileCommand Cmd(
WorkingDirectory ? *WorkingDirectory : llvm::sys::path::parent_path(File),
llvm::sys::path::filename(File), std::move(Argv),
/*Output=*/"");
Cmd.Heuristic = "clangd fallback";
return Cmd;
}
Expand Down Expand Up @@ -349,7 +350,8 @@ bool DirectoryBasedGlobalCompilationDatabase::DirectoryCache::load(

DirectoryBasedGlobalCompilationDatabase::
DirectoryBasedGlobalCompilationDatabase(const Options &Opts)
: Opts(Opts), Broadcaster(std::make_unique<BroadcastThread>(*this)) {
: GlobalCompilationDatabase(Opts.WorkingDirectory), Opts(Opts),
Broadcaster(std::make_unique<BroadcastThread>(*this)) {
if (!this->Opts.ContextProvider)
this->Opts.ContextProvider = [](llvm::StringRef) {
return Context::current().clone();
Expand Down Expand Up @@ -460,6 +462,17 @@ DirectoryBasedGlobalCompilationDatabase::lookupCDB(
return Result;
}

void DirectoryBasedGlobalCompilationDatabase::Options::applyWorkingDirectory(
const std::optional<std::string> &&WorkingDirectory) {
if (WorkingDirectory)
this->WorkingDirectory = *WorkingDirectory;
else {
SmallString<256> CWD;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's worth a comment explaining why we need a fallback here. Something like "the user asked for strong workspace mode, but the client didn't provide a workspace path, so use the working directory as a fallback".

llvm::sys::fs::current_path(CWD);
this->WorkingDirectory = std::string(CWD);
}
}

// The broadcast thread announces files with new compile commands to the world.
// Primarily this is used to enqueue them for background indexing.
//
Expand Down Expand Up @@ -759,8 +772,9 @@ DirectoryBasedGlobalCompilationDatabase::getProjectModules(PathRef File) const {

OverlayCDB::OverlayCDB(const GlobalCompilationDatabase *Base,
std::vector<std::string> FallbackFlags,
CommandMangler Mangler)
: DelegatingCDB(Base), Mangler(std::move(Mangler)),
CommandMangler Mangler,
std::optional<std::string> WorkingDirectory)
: DelegatingCDB(Base, WorkingDirectory), Mangler(std::move(Mangler)),
FallbackFlags(std::move(FallbackFlags)) {}

std::optional<tooling::CompileCommand>
Expand Down Expand Up @@ -844,16 +858,18 @@ OverlayCDB::getProjectModules(PathRef File) const {
return MDB;
}

DelegatingCDB::DelegatingCDB(const GlobalCompilationDatabase *Base)
: Base(Base) {
DelegatingCDB::DelegatingCDB(const GlobalCompilationDatabase *Base,
std::optional<std::string> WorkingDirectory)
: GlobalCompilationDatabase(WorkingDirectory), Base(Base) {
if (Base)
BaseChanged = Base->watch([this](const std::vector<std::string> Changes) {
OnCommandChanged.broadcast(Changes);
});
}

DelegatingCDB::DelegatingCDB(std::unique_ptr<GlobalCompilationDatabase> Base)
: DelegatingCDB(Base.get()) {
DelegatingCDB::DelegatingCDB(std::unique_ptr<GlobalCompilationDatabase> Base,
std::optional<std::string> WorkingDirectory)
: DelegatingCDB(Base.get(), WorkingDirectory) {
BaseOwner = std::move(Base);
}

Expand Down
18 changes: 15 additions & 3 deletions clang-tools-extra/clangd/GlobalCompilationDatabase.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ struct ProjectInfo {
/// Provides compilation arguments used for parsing C and C++ files.
class GlobalCompilationDatabase {
public:
GlobalCompilationDatabase(std::optional<std::string> WorkingDirectory)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would give this parameter a default to minimize the disruption caused by this change. (I'm not just talking about test code in this repo; I think this class has out-of-tree subclasses as well.)

: WorkingDirectory(WorkingDirectory) {}
virtual ~GlobalCompilationDatabase() = default;

/// If there are any known-good commands for building this file, returns one.
Expand Down Expand Up @@ -69,14 +71,17 @@ class GlobalCompilationDatabase {
}

protected:
std::optional<std::string> WorkingDirectory;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this only applies to fallback commands, let's call it FallbackWorkingDirectory. Likewise for the field in DirectoryBasedGlobalCompilationDatabase::Options, the method that sets it, etc.

mutable CommandChanged OnCommandChanged;
};

// Helper class for implementing GlobalCompilationDatabases that wrap others.
class DelegatingCDB : public GlobalCompilationDatabase {
public:
DelegatingCDB(const GlobalCompilationDatabase *Base);
DelegatingCDB(std::unique_ptr<GlobalCompilationDatabase> Base);
DelegatingCDB(const GlobalCompilationDatabase *Base,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise here, we may want to give these added parameters a default

std::optional<std::string> WorkingDirectory);
DelegatingCDB(std::unique_ptr<GlobalCompilationDatabase> Base,
std::optional<std::string> WorkingDirectory);

std::optional<tooling::CompileCommand>
getCompileCommand(PathRef File) const override;
Expand Down Expand Up @@ -117,6 +122,12 @@ class DirectoryBasedGlobalCompilationDatabase
// Only look for a compilation database in this one fixed directory.
// FIXME: fold this into config/context mechanism.
std::optional<Path> CompileCommandsDir;
// Working directory for fallback commands
// If unset, parent directory of file should be used
std::optional<std::string> WorkingDirectory;

void
applyWorkingDirectory(const std::optional<std::string> &&WorkingDirectory);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of using const && here?

};

DirectoryBasedGlobalCompilationDatabase(const Options &Opts);
Expand Down Expand Up @@ -196,7 +207,8 @@ class OverlayCDB : public DelegatingCDB {
// Adjuster is applied to all commands, fallback or not.
OverlayCDB(const GlobalCompilationDatabase *Base,
std::vector<std::string> FallbackFlags = {},
CommandMangler Mangler = nullptr);
CommandMangler Mangler = nullptr,
std::optional<std::string> WorkingDirectory = std::nullopt);

std::optional<tooling::CompileCommand>
getCompileCommand(PathRef File) const override;
Expand Down
2 changes: 2 additions & 0 deletions clang-tools-extra/clangd/TUScheduler.h
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,8 @@ class TUScheduler {
/// Typically to inject per-file configuration.
/// If the path is empty, context sholud be "generic".
std::function<Context(PathRef)> ContextProvider;

bool test;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leftover debug code?

};

TUScheduler(const GlobalCompilationDatabase &CDB, const Options &Opts,
Expand Down
8 changes: 6 additions & 2 deletions clang-tools-extra/clangd/tool/Check.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@ class Checker {
bool buildCommand(const ThreadsafeFS &TFS) {
log("Loading compilation database...");
DirectoryBasedGlobalCompilationDatabase::Options CDBOpts(TFS);
if (Opts.StrongWorkspaceMode)
CDBOpts.applyWorkingDirectory(std::move(Opts.WorkspaceRoot));
CDBOpts.CompileCommandsDir =
Config::current().CompileFlags.CDBSearch.FixedCDBPath;
BaseCDB =
Expand All @@ -178,8 +180,10 @@ class Checker {
getSystemIncludeExtractor(llvm::ArrayRef(Opts.QueryDriverGlobs));
if (Opts.ResourceDir)
Mangler.ResourceDir = *Opts.ResourceDir;

CDB = std::make_unique<OverlayCDB>(
BaseCDB.get(), std::vector<std::string>{}, std::move(Mangler));
BaseCDB.get(), std::vector<std::string>{}, std::move(Mangler),
CDBOpts.WorkingDirectory);

if (auto TrueCmd = CDB->getCompileCommand(File)) {
Cmd = std::move(*TrueCmd);
Expand Down Expand Up @@ -502,7 +506,7 @@ bool check(llvm::StringRef File, const ThreadsafeFS &TFS,
config::DiagnosticCallback Diag) const override {
config::Fragment F;
// If we're timing clang-tidy checks, implicitly disabling the slow ones
// is counterproductive!
// is counterproductive!
if (CheckTidyTime.getNumOccurrences())
F.Diagnostics.ClangTidy.FastCheckFilter.emplace("None");
return {std::move(F).compile(Diag)};
Expand Down
11 changes: 11 additions & 0 deletions clang-tools-extra/clangd/tool/ClangdMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,16 @@ opt<bool> EnableConfig{
init(true),
};

opt<bool> StrongWorkspaceMode{
"strong-workspace-mode",
cat(Features),
desc("An alternate mode of operation for clangd, operating more closely to "
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested reword: "operating more closely to the workspace" --> "where the clangd instance is used to edit a single workspace".

"the workspace.\n"
"When enabled, fallback commands use the workspace directory as their "
"working directory instead of the parent folder."),
init(false),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make this option hidden for now (example), until the feature is more fleshed out with other behaviours. (This just means it won't show up in clangd --help, only in clangd --help-hidden.)

};

opt<bool> UseDirtyHeaders{"use-dirty-headers", cat(Misc),
desc("Use files open in the editor when parsing "
"headers instead of reading from the disk"),
Expand Down Expand Up @@ -907,6 +917,7 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
}
if (!ResourceDir.empty())
Opts.ResourceDir = ResourceDir;
Opts.StrongWorkspaceMode = StrongWorkspaceMode;
Opts.BuildDynamicSymbolIndex = true;
#if CLANGD_ENABLE_REMOTE
if (RemoteIndexAddress.empty() != ProjectRoot.empty()) {
Expand Down
3 changes: 2 additions & 1 deletion clang-tools-extra/clangd/unittests/ClangdTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1110,7 +1110,8 @@ TEST(ClangdServerTest, FallbackWhenWaitingForCompileCommand) {
class DelayedCompilationDatabase : public GlobalCompilationDatabase {
public:
DelayedCompilationDatabase(Notification &CanReturnCommand)
: CanReturnCommand(CanReturnCommand) {}
: GlobalCompilationDatabase(std::nullopt),
CanReturnCommand(CanReturnCommand) {}

std::optional<tooling::CompileCommand>
getCompileCommand(PathRef File) const override {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,20 @@ TEST(GlobalCompilationDatabaseTest, FallbackCommand) {
testPath("foo/bar")));
}

TEST(GlobalCompilationDatabaseTest, FallbackWorkingDirectory) {
MockFS TFS;
DirectoryBasedGlobalCompilationDatabase::Options CDBOpts(TFS);
CDBOpts.applyWorkingDirectory(testPath("foo"));
EXPECT_EQ(CDBOpts.WorkingDirectory, testPath("foo"));

DirectoryBasedGlobalCompilationDatabase DB(CDBOpts);
auto Cmd = DB.getFallbackCommand(testPath("foo/src/bar.cc"));
EXPECT_EQ(Cmd.Directory, testPath("foo"));
EXPECT_THAT(Cmd.CommandLine,
ElementsAre("clang", testPath("foo/src/bar.cc")));
EXPECT_EQ(Cmd.Output, "");
}

static tooling::CompileCommand cmd(llvm::StringRef File, llvm::StringRef Arg) {
return tooling::CompileCommand(
testRoot(), File, {"clang", std::string(Arg), std::string(File)}, "");
Expand All @@ -63,6 +77,8 @@ static tooling::CompileCommand cmd(llvm::StringRef File, llvm::StringRef Arg) {
class OverlayCDBTest : public ::testing::Test {
class BaseCDB : public GlobalCompilationDatabase {
public:
BaseCDB() : GlobalCompilationDatabase(std::nullopt) {}

std::optional<tooling::CompileCommand>
getCompileCommand(llvm::StringRef File) const override {
if (File == testPath("foo.cc"))
Expand Down
2 changes: 2 additions & 0 deletions clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1329,6 +1329,8 @@ TEST_F(TUSchedulerTests, IncluderCache) {
OK = testPath("ok.h"),
NotIncluded = testPath("not_included.h");
struct NoHeadersCDB : public GlobalCompilationDatabase {
NoHeadersCDB() : GlobalCompilationDatabase(std::nullopt) {}

std::optional<tooling::CompileCommand>
getCompileCommand(PathRef File) const override {
if (File == NoCmd || File == NotIncluded || FailAll)
Expand Down
3 changes: 2 additions & 1 deletion clang-tools-extra/clangd/unittests/TestFS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ buildTestFS(llvm::StringMap<std::string> const &Files,

MockCompilationDatabase::MockCompilationDatabase(llvm::StringRef Directory,
llvm::StringRef RelPathPrefix)
: ExtraClangFlags({"-ffreestanding"}), Directory(Directory),
: GlobalCompilationDatabase(std::nullopt),
ExtraClangFlags({"-ffreestanding"}), Directory(Directory),
RelPathPrefix(RelPathPrefix) {
// -ffreestanding avoids implicit stdc-predef.h.
}
Expand Down