[clang] [llvm] [clang] Simplify usage of FileManager::makeAbsolutePath (PR #182360)
Cyndy Ishida via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 19 17:13:03 PST 2026
https://github.com/cyndyishida updated https://github.com/llvm/llvm-project/pull/182360
>From cd8ee2966aca934ad2c29ecdd879248c0adb81c6 Mon Sep 17 00:00:00 2001
From: Cyndy Ishida <cyndy_ishida at apple.com>
Date: Thu, 19 Feb 2026 12:03:28 -0800
Subject: [PATCH] [clang] Simplify usage of FileManager::makeAbsolutePath
Fold normalization of "."s into `FileManager::makeAbsolutePath` so that
different places that need to see through it can just call it instead of
handling it in wrappers.
---
clang/include/clang/Basic/FileManager.h | 9 ++++--
.../DependencyScanning/ModuleDepCollector.h | 3 +-
clang/lib/Basic/FileManager.cpp | 6 +++-
.../DependencyScanning/ModuleDepCollector.cpp | 28 ++++++++++---------
clang/lib/Serialization/ASTWriter.cpp | 18 +++---------
llvm/include/llvm/Support/Path.h | 8 +++---
llvm/lib/Support/Path.cpp | 2 --
7 files changed, 36 insertions(+), 38 deletions(-)
diff --git a/clang/include/clang/Basic/FileManager.h b/clang/include/clang/Basic/FileManager.h
index fa7552b6ae41d..01ce243e1b5fc 100644
--- a/clang/include/clang/Basic/FileManager.h
+++ b/clang/include/clang/Basic/FileManager.h
@@ -295,9 +295,12 @@ class FileManager : public RefCountedBase<FileManager> {
SmallVectorImpl<char> &Path);
/// Makes \c Path absolute taking into account FileSystemOptions and the
- /// working directory option.
- /// \returns true if \c Path changed to absolute.
- bool makeAbsolutePath(SmallVectorImpl<char> &Path) const;
+ /// working directory option, and canonicalizes through
+ /// `llvm::path::remove_dots` if \c Canonicalize is true.
+ ///
+ /// \returns true if \c Path was changed.
+ bool makeAbsolutePath(SmallVectorImpl<char> &Path,
+ bool Canonicalize = false) const;
/// Produce an array mapping from the unique IDs assigned to each
/// file to the corresponding FileEntryRef.
diff --git a/clang/include/clang/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/DependencyScanning/ModuleDepCollector.h
index 8f665daf03c69..6f6a608f65805 100644
--- a/clang/include/clang/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/DependencyScanning/ModuleDepCollector.h
@@ -200,7 +200,8 @@ struct ModuleDeps {
friend class ModuleDepCollector;
friend class ModuleDepCollectorPP;
- /// The base directory for relative paths in \c FileDeps.
+ /// The absolute directory path that is the base for relative paths
+ /// in \c FileDeps.
std::string FileDepsBaseDir;
/// A collection of paths to files that this module directly depends on, not
diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp
index 6d6ea5b84369b..f737d33009fc2 100644
--- a/clang/lib/Basic/FileManager.cpp
+++ b/clang/lib/Basic/FileManager.cpp
@@ -492,7 +492,8 @@ bool FileManager::fixupRelativePath(const FileSystemOptions &FileSystemOpts,
return true;
}
-bool FileManager::makeAbsolutePath(SmallVectorImpl<char> &Path) const {
+bool FileManager::makeAbsolutePath(SmallVectorImpl<char> &Path,
+ bool Canonicalize) const {
bool Changed = FixupRelativePath(Path);
if (!llvm::sys::path::is_absolute(StringRef(Path.data(), Path.size()))) {
@@ -500,6 +501,9 @@ bool FileManager::makeAbsolutePath(SmallVectorImpl<char> &Path) const {
Changed = true;
}
+ if (Canonicalize)
+ Changed |= llvm::sys::path::remove_dots(Path);
+
return Changed;
}
diff --git a/clang/lib/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/DependencyScanning/ModuleDepCollector.cpp
index 46a0f79bfd38e..e76cd78cb0d2b 100644
--- a/clang/lib/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/DependencyScanning/ModuleDepCollector.cpp
@@ -664,6 +664,16 @@ void ModuleDepCollectorPP::EndOfMainFile() {
MDC.Consumer.handlePrebuiltModuleDependency(I.second);
}
+static StringRef makeAbsoluteAndCanonicalize(CompilerInstance &CI,
+ StringRef Path,
+ SmallVectorImpl<char> &Storage) {
+ // FIXME: Consider skipping if path is already absolute & canonicalized.
+
+ Storage.assign(Path.begin(), Path.end());
+ CI.getFileManager().makeAbsolutePath(Storage, /*Canonicalize=*/true);
+ return StringRef(Storage.data(), Storage.size());
+}
+
std::optional<ModuleID>
ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
assert(M == M->getTopLevelModule() && "Expected top level module!");
@@ -707,7 +717,10 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
serialization::ModuleFile *MF =
MDC.ScanInstance.getASTReader()->getModuleManager().lookup(
*M->getASTFile());
- MD.FileDepsBaseDir = MF->BaseDirectory;
+
+ llvm::SmallString<256> Storage;
+ MD.FileDepsBaseDir =
+ makeAbsoluteAndCanonicalize(MDC.ScanInstance, MF->BaseDirectory, Storage);
MDC.ScanInstance.getASTReader()->visitInputFileInfos(
*MF, /*IncludeSystem=*/true,
[&](const serialization::InputFileInfo &IFI, bool IsSystem) {
@@ -948,17 +961,6 @@ void ModuleDepCollector::addVisibleModules() {
InsertVisibleModules(Import);
}
-static StringRef makeAbsoluteAndPreferred(CompilerInstance &CI, StringRef Path,
- SmallVectorImpl<char> &Storage) {
- if (llvm::sys::path::is_absolute(Path) &&
- !llvm::sys::path::is_style_windows(llvm::sys::path::Style::native))
- return Path;
- Storage.assign(Path.begin(), Path.end());
- CI.getFileManager().makeAbsolutePath(Storage);
- llvm::sys::path::make_preferred(Storage);
- return StringRef(Storage.data(), Storage.size());
-}
-
void ModuleDepCollector::addFileDep(StringRef Path) {
if (Service.getOpts().Format == ScanningOutputFormat::P1689) {
// Within P1689 format, we don't want all the paths to be absolute path
@@ -968,7 +970,7 @@ void ModuleDepCollector::addFileDep(StringRef Path) {
}
llvm::SmallString<256> Storage;
- Path = makeAbsoluteAndPreferred(ScanInstance, Path, Storage);
+ Path = makeAbsoluteAndCanonicalize(ScanInstance, Path, Storage);
FileDeps.emplace_back(Path);
}
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 0368fc2d6e3cc..b36d035eec609 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1163,16 +1163,6 @@ void ASTWriter::WriteBlockInfoBlock() {
Stream.ExitBlock();
}
-/// Prepares a path for being written to an AST file by converting it
-/// to an absolute path and removing nested './'s.
-///
-/// \return \c true if the path was changed.
-static bool cleanPathForOutput(FileManager &FileMgr,
- SmallVectorImpl<char> &Path) {
- bool Changed = FileMgr.makeAbsolutePath(Path);
- return Changed | llvm::sys::path::remove_dots(Path);
-}
-
/// Adjusts the given filename to only write out the portion of the
/// filename that is not part of the system root directory.
///
@@ -1504,7 +1494,7 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) {
return std::nullopt;
}();
if (BaseDir) {
- cleanPathForOutput(FileMgr, *BaseDir);
+ FileMgr.makeAbsolutePath(*BaseDir, /*Canonicalize=*/true);
// If the home of the module is the current working directory, then we
// want to pick up the cwd of the build process loading the module, not
@@ -1530,7 +1520,7 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) {
} else if (!isysroot.empty()) {
// Write out paths relative to the sysroot if possible.
SmallString<128> CleanedSysroot(isysroot);
- cleanPathForOutput(PP.getFileManager(), CleanedSysroot);
+ PP.getFileManager().makeAbsolutePath(CleanedSysroot, /*Canonicalize=*/true);
BaseDirectory.assign(CleanedSysroot.begin(), CleanedSysroot.end());
}
@@ -5370,8 +5360,8 @@ bool ASTWriter::PreparePathForOutput(SmallVectorImpl<char> &Path) {
if (PathStr == "<built-in>" || PathStr == "<command line>")
return false;
- bool Changed = cleanPathForOutput(PP->getFileManager(), Path);
-
+ bool Changed =
+ PP->getFileManager().makeAbsolutePath(Path, /*Canonicalize=*/true);
// Remove a prefix to make the path relative, if relevant.
const char *PathBegin = Path.data();
const char *PathPtr =
diff --git a/llvm/include/llvm/Support/Path.h b/llvm/include/llvm/Support/Path.h
index a8e0f338ec203..84100dcc73a9d 100644
--- a/llvm/include/llvm/Support/Path.h
+++ b/llvm/include/llvm/Support/Path.h
@@ -207,12 +207,12 @@ LLVM_ABI bool replace_path_prefix(SmallVectorImpl<char> &Path,
LLVM_ABI StringRef remove_leading_dotslash(StringRef path LLVM_LIFETIME_BOUND,
Style style = Style::native);
-/// In-place remove any './' and optionally '../' components from a path.
+/// Remove './' and optionally '../' components, and canonicalize separators.
///
-/// @param path processed path
+/// @param path processed path.
/// @param remove_dot_dot specify if '../' (except for leading "../") should be
-/// removed
-/// @result True if path was changed
+/// removed.
+/// @result True if path was changed.
LLVM_ABI bool remove_dots(SmallVectorImpl<char> &path,
bool remove_dot_dot = false,
Style style = Style::native);
diff --git a/llvm/lib/Support/Path.cpp b/llvm/lib/Support/Path.cpp
index a74beca5dcac3..a53beb9359bc0 100644
--- a/llvm/lib/Support/Path.cpp
+++ b/llvm/lib/Support/Path.cpp
@@ -760,8 +760,6 @@ StringRef remove_leading_dotslash(StringRef Path, Style style) {
return Path;
}
-// Remove path traversal components ("." and "..") when possible, and
-// canonicalize slashes.
bool remove_dots(SmallVectorImpl<char> &the_path, bool remove_dot_dot,
Style style) {
style = real_style(style);
More information about the cfe-commits
mailing list