benlangmuir created this revision.
benlangmuir added reviewers: bnbarham, jansvoboda11.
Herald added a project: All.
benlangmuir requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.
It's an accident that we started return asbolute paths from FileEntry::getName
for all relative paths. Prepare for getName to get (closer to) return the
requested path. Note: conceptually it might make sense for the dependency
scanner to allow relative paths and have the DependencyConsumer decide if it
wants to make them absolute, but we currently document that it's absolute and I
didn't want to change behaviour here.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D130934
Files:
clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -240,8 +240,7 @@
// We do not want #line markers to affect dependency generation!
if (Optional<StringRef> Filename =
SM.getNonBuiltinFilenameForID(SM.getFileID(SM.getExpansionLoc(Loc))))
- MDC.FileDeps.push_back(
- std::string(llvm::sys::path::remove_leading_dotslash(*Filename)));
+ MDC.addFileDep(llvm::sys::path::remove_leading_dotslash(*Filename));
}
void ModuleDepCollectorPP::InclusionDirective(
@@ -252,7 +251,7 @@
if (!File && !Imported) {
// This is a non-modular include that HeaderSearch failed to find. Add it
// here as `FileChanged` will never see it.
- MDC.FileDeps.push_back(std::string(FileName));
+ MDC.addFileDep(FileName);
}
handleImport(Imported);
}
@@ -282,8 +281,7 @@
->getName());
if (!MDC.ScanInstance.getPreprocessorOpts().ImplicitPCHInclude.empty())
- MDC.FileDeps.push_back(
- MDC.ScanInstance.getPreprocessorOpts().ImplicitPCHInclude);
+ MDC.addFileDep(MDC.ScanInstance.getPreprocessorOpts().ImplicitPCHInclude);
for (const Module *M : DirectModularDeps) {
// A top-level module might not be actually imported as a module when
@@ -346,10 +344,10 @@
// handle it like normal. With explicitly built modules we don't need
// to play VFS tricks, so replace it with the correct module map.
if (IF.getFile()->getName().endswith("__inferred_module.map")) {
- MD.FileDeps.insert(ModuleMap->getName());
+ MDC.addFileDep(MD, ModuleMap->getName());
return;
}
- MD.FileDeps.insert(IF.getFile()->getName());
+ MDC.addFileDep(MD, IF.getFile()->getName());
});
// We usually don't need to list the module map files of our dependencies when
@@ -494,3 +492,24 @@
PrebuiltModuleFileIt->second == M->getASTFile()->getName());
return true;
}
+
+static StringRef makeAbsolute(CompilerInstance &CI, StringRef Path,
+ SmallVectorImpl<char> &Storage) {
+ if (llvm::sys::path::is_absolute(Path))
+ return Path;
+ Storage.assign(Path.begin(), Path.end());
+ CI.getFileManager().makeAbsolutePath(Storage);
+ return StringRef(Storage.data(), Storage.size());
+}
+
+void ModuleDepCollector::addFileDep(StringRef Path) {
+ llvm::SmallString<256> Storage;
+ Path = makeAbsolute(ScanInstance, Path, Storage);
+ FileDeps.push_back(std::string(Path));
+}
+
+void ModuleDepCollector::addFileDep(ModuleDeps &MD, StringRef Path) {
+ llvm::SmallString<256> Storage;
+ Path = makeAbsolute(ScanInstance, Path, Storage);
+ MD.FileDeps.insert(Path);
+}
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -28,8 +28,9 @@
class DependencyConsumerForwarder : public DependencyFileGenerator {
public:
DependencyConsumerForwarder(std::unique_ptr<DependencyOutputOptions> Opts,
- DependencyConsumer &C)
- : DependencyFileGenerator(*Opts), Opts(std::move(Opts)), C(C) {}
+ StringRef WorkingDirectory, DependencyConsumer &C)
+ : DependencyFileGenerator(*Opts), WorkingDirectory(WorkingDirectory),
+ Opts(std::move(Opts)), C(C) {}
void finishedMainFile(DiagnosticsEngine &Diags) override {
C.handleDependencyOutputOpts(*Opts);
@@ -37,11 +38,13 @@
for (const auto &File : getDependencies()) {
CanonPath = File;
llvm::sys::path::remove_dots(CanonPath, /*remove_dot_dot=*/true);
+ llvm::sys::fs::make_absolute(WorkingDirectory, CanonPath);
C.handleFileDependency(CanonPath);
}
}
private:
+ StringRef WorkingDirectory;
std::unique_ptr<DependencyOutputOptions> Opts;
DependencyConsumer &C;
};
@@ -221,8 +224,8 @@
switch (Format) {
case ScanningOutputFormat::Make:
ScanInstance.addDependencyCollector(
- std::make_shared<DependencyConsumerForwarder>(std::move(Opts),
- Consumer));
+ std::make_shared<DependencyConsumerForwarder>(
+ std::move(Opts), WorkingDirectory, Consumer));
break;
case ScanningOutputFormat::Full:
ScanInstance.addDependencyCollector(std::make_shared<ModuleDepCollector>(
Index: clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
===================================================================
--- clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -233,6 +233,11 @@
/// Checks whether the module is known as being prebuilt.
bool isPrebuiltModule(const Module *M);
+ /// Adds \p Path to \c FileDeps, making it absolute if necessary.
+ void addFileDep(StringRef Path);
+ /// Adds \p Path to \c MD.FileDeps, making it absolute if necessary.
+ void addFileDep(ModuleDeps &MD, StringRef Path);
+
/// Constructs a CompilerInvocation that can be used to build the given
/// module, excluding paths to discovered modular dependencies that are yet to
/// be built.
Index: clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===================================================================
--- clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -156,11 +156,15 @@
const tooling::TranslationUnitDiagnostics *SourceTU,
const llvm::Optional<std::string> BuildDir) {
// Use the file manager to deduplicate paths. FileEntries are
- // automatically canonicalized.
- auto PrevWorkingDir = SM.getFileManager().getFileSystemOpts().WorkingDir;
+ // automatically canonicalized. Since relative paths can come from different
+ // build directories, make them absolute immediately.
+ SmallString<128> Path = R.getFilePath();
if (BuildDir)
- SM.getFileManager().getFileSystemOpts().WorkingDir = std::move(*BuildDir);
- if (auto Entry = SM.getFileManager().getFile(R.getFilePath())) {
+ llvm::sys::fs::make_absolute(*BuildDir, Path);
+ else
+ SM.getFileManager().makeAbsolutePath(Path);
+
+ if (auto Entry = SM.getFileManager().getFile(Path)) {
if (SourceTU) {
auto &Replaces = DiagReplacements[*Entry];
auto It = Replaces.find(R);
@@ -171,11 +175,10 @@
return;
}
GroupedReplacements[*Entry].push_back(R);
- } else if (Warned.insert(R.getFilePath()).second) {
+ } else if (Warned.insert(Path).second) {
errs() << "Described file '" << R.getFilePath()
<< "' doesn't exist. Ignoring...\n";
}
- SM.getFileManager().getFileSystemOpts().WorkingDir = PrevWorkingDir;
};
for (const auto &TU : TUs)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
