llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-modules Author: Jan Svoboda (jansvoboda11) <details> <summary>Changes</summary> PR https://github.com/llvm/llvm-project/pull/150123 changed how we normalize the modules cache path. Unfortunately, empty path would get normalized to the current working directory. This means that even explicitly-built PCMs that don't rely on the CWD now embed it, leading to surprising behavior. This PR fixes that by normalizing an empty modules cache path to an empty string. --- Full diff: https://github.com/llvm/llvm-project/pull/164840.diff 4 Files Affected: - (modified) clang/lib/Frontend/CompilerInstance.cpp (+1-4) - (modified) clang/lib/Lex/HeaderSearch.cpp (+4-2) - (modified) clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp (+5-6) - (added) clang/test/Modules/explicit-build-cwd.c (+17) ``````````diff diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 374138fe4cf8f..e3bf0eaa3c391 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -546,14 +546,11 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) { std::string CompilerInstance::getSpecificModuleCachePath(StringRef ModuleHash) { assert(FileMgr && "Specific module cache path requires a FileManager"); - if (getHeaderSearchOpts().ModuleCachePath.empty()) - return ""; - // Set up the module path, including the hash for the module-creation options. SmallString<256> SpecificModuleCache; normalizeModuleCachePath(*FileMgr, getHeaderSearchOpts().ModuleCachePath, SpecificModuleCache); - if (!getHeaderSearchOpts().DisableModuleHash) + if (!SpecificModuleCache.empty() && !getHeaderSearchOpts().DisableModuleHash) llvm::sys::path::append(SpecificModuleCache, ModuleHash); return std::string(SpecificModuleCache); } diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp index 238c5e2f2d9a5..65c324c10ca5d 100644 --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -2186,6 +2186,8 @@ std::string HeaderSearch::suggestPathToFileForDiagnostics( void clang::normalizeModuleCachePath(FileManager &FileMgr, StringRef Path, SmallVectorImpl<char> &NormalizedPath) { NormalizedPath.assign(Path.begin(), Path.end()); - FileMgr.makeAbsolutePath(NormalizedPath); - llvm::sys::path::remove_dots(NormalizedPath); + if (!NormalizedPath.empty()) { + FileMgr.makeAbsolutePath(NormalizedPath); + llvm::sys::path::remove_dots(NormalizedPath); + } } diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp index 05d566922a441..42f52d0ff6241 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp @@ -524,13 +524,12 @@ bool initializeScanCompilerInstance( // Use the dependency scanning optimized file system if requested to do so. if (DepFS) { DepFS->resetBypassedPathPrefix(); - if (!ScanInstance.getHeaderSearchOpts().ModuleCachePath.empty()) { - SmallString<256> ModulesCachePath; - normalizeModuleCachePath( - ScanInstance.getFileManager(), - ScanInstance.getHeaderSearchOpts().ModuleCachePath, ModulesCachePath); + SmallString<256> ModulesCachePath; + normalizeModuleCachePath(ScanInstance.getFileManager(), + ScanInstance.getHeaderSearchOpts().ModuleCachePath, + ModulesCachePath); + if (!ModulesCachePath.empty()) DepFS->setBypassedPathPrefix(ModulesCachePath); - } ScanInstance.setDependencyDirectivesGetter( std::make_unique<ScanningDependencyDirectivesGetter>( diff --git a/clang/test/Modules/explicit-build-cwd.c b/clang/test/Modules/explicit-build-cwd.c new file mode 100644 index 0000000000000..af8b74361d685 --- /dev/null +++ b/clang/test/Modules/explicit-build-cwd.c @@ -0,0 +1,17 @@ +// This test checks that explicitly building the same module from different +// working directories results in the same PCM contents. + +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: mkdir %t/one +// RUN: mkdir %t/two + +//--- module.modulemap +module M { header "M.h" } + +//--- M.h + +// RUN: cd %t/one && %clang_cc1 -fmodules -emit-module %t/module.modulemap -fmodule-name=M -o %t/M_one.pcm +// RUN: cd %t/two && %clang_cc1 -fmodules -emit-module %t/module.modulemap -fmodule-name=M -o %t/M_two.pcm + +// RUN: diff %t/M_one.pcm %t/M_two.pcm `````````` </details> https://github.com/llvm/llvm-project/pull/164840 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
