llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-modules Author: Cyndy Ishida (cyndyishida) <details> <summary>Changes</summary> 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. There shouldn't be a functional impact. --- Full diff: https://github.com/llvm/llvm-project/pull/182360.diff 5 Files Affected: - (modified) clang/include/clang/Basic/FileManager.h (+5-3) - (modified) clang/include/clang/DependencyScanning/ModuleDepCollector.h (+2-1) - (modified) clang/lib/Basic/FileManager.cpp (+5-1) - (modified) clang/lib/DependencyScanning/ModuleDepCollector.cpp (+15-12) - (modified) clang/lib/Serialization/ASTWriter.cpp (+4-14) ``````````diff diff --git a/clang/include/clang/Basic/FileManager.h b/clang/include/clang/Basic/FileManager.h index fa7552b6ae41d..02f6d59199d24 100644 --- a/clang/include/clang/Basic/FileManager.h +++ b/clang/include/clang/Basic/FileManager.h @@ -295,9 +295,11 @@ 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 resolves through "."s if + /// \c RemoveDots is true. + /// \returns true if \c Path was changed. + bool makeAbsolutePath(SmallVectorImpl<char> &Path, + bool RemoveDots = 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..48725d1ee734c 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 RemoveDots) 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 (RemoveDots) + 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..ab1e05725de9d 100644 --- a/clang/lib/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/DependencyScanning/ModuleDepCollector.cpp @@ -664,6 +664,17 @@ void ModuleDepCollectorPP::EndOfMainFile() { MDC.Consumer.handlePrebuiltModuleDependency(I.second); } +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, /*RemoveDots=*/true); + llvm::sys::path::make_preferred(Storage); + return StringRef(Storage.data(), Storage.size()); +} + std::optional<ModuleID> ModuleDepCollectorPP::handleTopLevelModule(const Module *M) { assert(M == M->getTopLevelModule() && "Expected top level module!"); @@ -707,7 +718,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 = + makeAbsoluteAndPreferred(MDC.ScanInstance, MF->BaseDirectory, Storage); MDC.ScanInstance.getASTReader()->visitInputFileInfos( *MF, /*IncludeSystem=*/true, [&](const serialization::InputFileInfo &IFI, bool IsSystem) { @@ -948,17 +962,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 diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 0368fc2d6e3cc..8218944383e70 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, /*RemoveDots=*/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, /*RemoveDots=*/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, /*RemoveDots=*/true); // Remove a prefix to make the path relative, if relevant. const char *PathBegin = Path.data(); const char *PathPtr = `````````` </details> https://github.com/llvm/llvm-project/pull/182360 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
