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 <[email protected]> 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); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
