[clang] b4c6dc2 - [clang] Update code that assumes FileEntry::getName is absolute NFC
Ben Langmuir via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 1 14:48:57 PDT 2022
Author: Ben Langmuir
Date: 2022-08-01T14:48:37-07:00
New Revision: b4c6dc2e66370d94ff52075c2710a674d8e1e0f8
URL: https://github.com/llvm/llvm-project/commit/b4c6dc2e66370d94ff52075c2710a674d8e1e0f8
DIFF: https://github.com/llvm/llvm-project/commit/b4c6dc2e66370d94ff52075c2710a674d8e1e0f8.diff
LOG: [clang] Update code that assumes FileEntry::getName is absolute NFC
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.
Differential Revision: https://reviews.llvm.org/D130934
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp b/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
index cb5caf1ca92a1..9757d99cd789b 100644
--- a/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ b/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -156,11 +156,15 @@ groupReplacements(const TUReplacements &TUs, const TUDiagnostics &TUDs,
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
diff erent
+ // 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 @@ groupReplacements(const TUReplacements &TUs, const TUDiagnostics &TUDs,
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)
diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index 377dcd5a68fec..f4f13a34b1746 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -233,6 +233,11 @@ class ModuleDepCollector final : public DependencyCollector {
/// 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.
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index 474808d888ec2..c84919138612a 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -28,8 +28,9 @@ namespace {
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 @@ class DependencyConsumerForwarder : public DependencyFileGenerator {
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 @@ class DependencyScanningAction : public tooling::ToolAction {
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>(
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 8d58a53a5971c..b3726b09a4400 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -240,8 +240,7 @@ void ModuleDepCollectorPP::FileChanged(SourceLocation Loc,
// 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 @@ void ModuleDepCollectorPP::InclusionDirective(
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 @@ void ModuleDepCollectorPP::EndOfMainFile() {
->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 @@ ModuleID ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
// 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 @@ bool ModuleDepCollector::isPrebuiltModule(const Module *M) {
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);
+}
More information about the cfe-commits
mailing list