Author: Volodymyr Sapsai Date: 2023-08-10T10:47:51-07:00 New Revision: 97dfaf4cd27814bdf9aa9d2eafc21fdb4f76c56d
URL: https://github.com/llvm/llvm-project/commit/97dfaf4cd27814bdf9aa9d2eafc21fdb4f76c56d DIFF: https://github.com/llvm/llvm-project/commit/97dfaf4cd27814bdf9aa9d2eafc21fdb4f76c56d.diff LOG: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays. Fix errors like > module 'MultiPath' is defined in both > 'path/to/modules.cache/3JR48BPRU7BCG/MultiPath-1352QHUF8RNMU.pcm' and > 'path/to/modules.cache/3JR48BPRU7BCG/MultiPath-20HNSLLIUDDV1.pcm' To avoid building extra identical modules `-ivfsoverlay` option is not a part of the hash like "/3JR48BPRU7BCG/". And it is build system's responsibility to provide `-ivfsoverlay` options that don't cause observable differences. We also need to make sure the hash like "-1352QHUF8RNMU" is not affected by `-ivfsoverlay`. As this hash is defined by the module map path, use the path prior to any VFS remappings. rdar://111921464 Differential Revision: https://reviews.llvm.org/D156749 Added: clang/test/VFS/module-map-path.m Modified: clang/lib/Lex/HeaderSearch.cpp clang/lib/Serialization/ASTWriter.cpp Removed: ################################################################################ diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp index f42d51d65dcdf3..80e1bad72ce9b7 100644 --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -177,7 +177,7 @@ std::string HeaderSearch::getCachedModuleFileName(Module *Module) { // *.modulemap file. In this case, just return an empty string. if (!ModuleMap) return {}; - return getCachedModuleFileName(Module->Name, ModuleMap->getName()); + return getCachedModuleFileName(Module->Name, ModuleMap->getNameAsRequested()); } std::string HeaderSearch::getPrebuiltModuleFileName(StringRef ModuleName, diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 8b0015f1ee9f6c..289c0383cd4b0e 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1327,7 +1327,8 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, ASTContext &Context, auto &Map = PP.getHeaderSearchInfo().getModuleMap(); AddPath(WritingModule->PresumedModuleMapFile.empty() - ? Map.getModuleMapFileForUniquing(WritingModule)->getName() + ? Map.getModuleMapFileForUniquing(WritingModule) + ->getNameAsRequested() : StringRef(WritingModule->PresumedModuleMapFile), Record); diff --git a/clang/test/VFS/module-map-path.m b/clang/test/VFS/module-map-path.m new file mode 100644 index 00000000000000..0f3c0a7aa499eb --- /dev/null +++ b/clang/test/VFS/module-map-path.m @@ -0,0 +1,110 @@ +// Test the module map path is consistent between clang invocations when using VFS overlays. + +// RUN: rm -rf %t +// RUN: split-file %s %t + +// Pre-populate the module cache with the modules that don't use VFS overlays. +// RUN: %clang_cc1 -fsyntax-only -F%t/Frameworks -I%t/include %t/prepopulate_module_cache.m \ +// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache + +// Execute a compilation with VFS overlay. .pcm file path looks like <hash1>/ModuleName-<hash2>.pcm. +// <hash1> corresponds to the compilation settings like language options. +// <hash2> corresponds to the module map path. So if any of those change, we should use a diff erent module. +// But for VFS overlay we make an exception that it's not a part of <hash1> to reduce the number of built .pcm files. +// Test that paths in overlays don't leak into <hash2> and don't cause using 2 .pcm files for the same module. +// DEFINE: %{command} = %clang_cc1 -fsyntax-only -verify -F%t/Frameworks -I%t/include %t/test.m \ +// DEFINE: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache +// RUN: sed -e "s@TMP_DIR@%{/t:regex_replacement}@g" -e "s@USE_EXTERNAL_NAMES_OPTION@@g" %t/overlay.yaml.template > %t/external-names-default.yaml +// RUN: %{command} -ivfsoverlay %t/external-names-default.yaml + +// RUN: sed -e "s@TMP_DIR@%{/t:regex_replacement}@g" -e "s@USE_EXTERNAL_NAMES_OPTION@'use-external-names': true,@g" %t/overlay.yaml.template > %t/external-names-true.yaml +// RUN: %{command} -ivfsoverlay %t/external-names-true.yaml + +// RUN: sed -e "s@TMP_DIR@%{/t:regex_replacement}@g" -e "s@USE_EXTERNAL_NAMES_OPTION@'use-external-names': false,@g" %t/overlay.yaml.template > %t/external-names-false.yaml +// RUN: %{command} -ivfsoverlay %t/external-names-false.yaml + +//--- prepopulate_module_cache.m +#import <Redirecting/Redirecting.h> + +//--- test.m +// At first import multi-path modules directly, so clang decides which .pcm file they should belong to. +#import <MultiPath/MultiPath.h> +#import <MultiPathHeader.h> + +// Then import a module from the module cache and all its transitive dependencies. +// Make sure the .pcm files loaded directly are the same as 'Redirecting' is referencing. +#import <Redirecting/Redirecting.h> +// expected-no-diagnostics + + +//--- Frameworks/MultiPath.framework/Headers/MultiPath.h +void multiPathFramework(void); + +//--- Frameworks/MultiPath.framework/Modules/module.modulemap +framework module MultiPath { + header "MultiPath.h" + export * +} + + +//--- include/MultiPathHeader.h +void multiPathHeader(void); + +//--- include/module.modulemap +module MultiPathHeader { + header "MultiPathHeader.h" + export * +} + + +//--- Frameworks/Redirecting.framework/Headers/Redirecting.h +#import <MultiPath/MultiPath.h> +#import <MultiPathHeader.h> + +//--- Frameworks/Redirecting.framework/Modules/module.modulemap +framework module Redirecting { + header "Redirecting.h" + export * +} + + +//--- BuildTemporaries/MultiPath.h +void multiPathFramework(void); +//--- BuildTemporaries/module.modulemap +framework module MultiPath { + header "MultiPath.h" + export * +} +//--- BuildTemporaries/header.h +void multiPathHeader(void); +//--- BuildTemporaries/include.modulemap +module MultiPathHeader { + header "MultiPathHeader.h" + export * +} + +//--- overlay.yaml.template +{ + 'version': 0, + USE_EXTERNAL_NAMES_OPTION + 'roots': [ + { 'name': 'TMP_DIR/Frameworks/MultiPath.framework/Headers', 'type': 'directory', + 'contents': [ + { 'name': 'MultiPath.h', 'type': 'file', + 'external-contents': 'TMP_DIR/BuildTemporaries/MultiPath.h'} + ]}, + { 'name': 'TMP_DIR/Frameworks/MultiPath.framework/Modules', 'type': 'directory', + 'contents': [ + { 'name': 'module.modulemap', 'type': 'file', + 'external-contents': 'TMP_DIR/BuildTemporaries/module.modulemap'} + ]}, + { 'name': 'TMP_DIR/include', 'type': 'directory', + 'contents': [ + { 'name': 'MultiPathHeader.h', 'type': 'file', + 'external-contents': 'TMP_DIR/BuildTemporaries/header.h'}, + { 'name': 'module.modulemap', 'type': 'file', + 'external-contents': 'TMP_DIR/BuildTemporaries/include.modulemap'} + ]} + ] +} + _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
