[clang][modules] Don't depend on sharing FileManager during module build

Sharing the FileManager between the importer and the module build should
only be an optimization. Add a cc1 option -fno-modules-share-filemanager
to allow us to test this. Fix the path to modulemap files, which
previously depended on the shared FileManager when using path mapped to
an external file in a VFS.

Differential Revision: https://reviews.llvm.org/D131076

6 files changed, 74 insertions, 9 deletions

diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 83dbcfd55d96..eaf8406d306f 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -6032,6 +6032,9 @@ def fallow_pch_with_different_modules_cache_path :
   Flag<["-"], "fallow-pch-with-different-modules-cache-path">,
   HelpText<"Accept a PCH file that was created with a different modules cache path">,
   MarshallingInfoFlag<PreprocessorOpts<"AllowPCHWithDifferentModulesCachePath">>;
+def fno_modules_share_filemanager : Flag<["-"], "fno-modules-share-filemanager">,
+  HelpText<"Disable sharing the FileManager when building a module implicitly">,
+  MarshallingInfoNegativeFlag<FrontendOpts<"ModulesShareFileManager">>;
 def dump_deserialized_pch_decls : Flag<["-"], "dump-deserialized-decls">,
   HelpText<"Dump declarations that are deserialized from PCH, for testing">,
   MarshallingInfoFlag<PreprocessorOpts<"DumpDeserializedPCHDecls">>;
diff --git a/clang/include/clang/Frontend/FrontendOptions.h b/clang/include/clang/Frontend/FrontendOptions.h
index 0c9c2489e448..56e704d8a52f 100644
--- a/clang/include/clang/Frontend/FrontendOptions.h
+++ b/clang/include/clang/Frontend/FrontendOptions.h
@@ -356,6 +356,9 @@ public:
   /// Output (and read) PCM files regardless of compiler errors.
   unsigned AllowPCMWithCompilerErrors : 1;
 
+  /// Whether to share the FileManager when building modules.
+  unsigned ModulesShareFileManager : 1;
+
   CodeCompleteOptions CodeCompleteOpts;
 
   /// Specifies the output format of the AST.
@@ -523,7 +526,7 @@ public:
         BuildingImplicitModuleUsesLock(true), ModulesEmbedAllFiles(false),
         IncludeTimestamps(true), UseTemporary(true),
         OutputPathIndependentPCM(false), AllowPCMWithCompilerErrors(false),
-        TimeTraceGranularity(500) {}
+        ModulesShareFileManager(true), TimeTraceGranularity(500) {}
 
   /// getInputKindForExtension - Return the appropriate input kind for a file
   /// extension. For example, "c" would return Language::C.
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index 2cd7efd862ec..51c7bc237a50 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1213,11 +1213,16 @@ compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc,
                                    ImportingInstance.getDiagnosticClient()),
                              /*ShouldOwnClient=*/true);
 
-  // Note that this module is part of the module build stack, so that we
-  // can detect cycles in the module graph.
-  Instance.setFileManager(&ImportingInstance.getFileManager());
+  if (FrontendOpts.ModulesShareFileManager) {
+    Instance.setFileManager(&ImportingInstance.getFileManager());
+  } else {
+    Instance.createFileManager(&ImportingInstance.getVirtualFileSystem());
+  }
   Instance.createSourceManager(Instance.getFileManager());
   SourceManager &SourceMgr = Instance.getSourceManager();
+
+  // Note that this module is part of the module build stack, so that we
+  // can detect cycles in the module graph.
   SourceMgr.setModuleBuildStack(
     ImportingInstance.getSourceManager().getModuleBuildStack());
   SourceMgr.pushModuleBuildStack(ModuleName,
@@ -1303,12 +1308,16 @@ static bool compileModule(CompilerInstance &ImportingInstance,
             ModuleMapFile, ImportingInstance.getFileManager()))
       ModuleMapFile = PublicMMFile;
 
+    // FIXME: Update header search to keep FileEntryRef rather than rely on
+    // getLastRef().
+    StringRef ModuleMapFilePath =
+        ModuleMapFile->getLastRef().getNameAsRequested();
+
     // Use the module map where this module resides.
     Result = compileModuleImpl(
         ImportingInstance, ImportLoc, Module->getTopLevelModuleName(),
-        FrontendInputFile(ModuleMapFile->getName(), IK, +Module->IsSystem),
-        ModMap.getModuleMapFileForUniquing(Module)->getName(),
-        ModuleFileName);
+        FrontendInputFile(ModuleMapFilePath, IK, +Module->IsSystem),
+        ModMap.getModuleMapFileForUniquing(Module)->getName(), ModuleFileName);
   } else {
     // FIXME: We only need to fake up an input file here as a way of
     // transporting the module's directory to the module map parser. We should
diff --git a/clang/test/ClangScanDeps/modulemap-via-vfs.m b/clang/test/ClangScanDeps/modulemap-via-vfs.m
index c0928909e131..870aa820dacc 100644
--- a/clang/test/ClangScanDeps/modulemap-via-vfs.m
+++ b/clang/test/ClangScanDeps/modulemap-via-vfs.m
@@ -2,13 +2,15 @@
 // RUN: split-file %s %t.dir
 // RUN: sed -e "s|DIR|%/t.dir|g" %t.dir/build/compile-commands.json.in > %t.dir/build/compile-commands.json
 // RUN: sed -e "s|DIR|%/t.dir|g" %t.dir/build/vfs.yaml.in > %t.dir/build/vfs.yaml
-// RUN: clang-scan-deps -compilation-database %t.dir/build/compile-commands.json -j 1 -format experimental-full \
+// RUN: clang-scan-deps -compilation-database %t.dir/build/compile-commands.json \
+// RUN:   -reuse-filemanager=0 -j 1 -format experimental-full \
 // RUN:   -mode preprocess-dependency-directives -generate-modules-path-args > %t.db
 // RUN: %deps-to-rsp %t.db --module-name=A > %t.A.cc1.rsp
 // RUN: cat %t.A.cc1.rsp | sed 's:\\\\\?:/:g' | FileCheck %s
 
 // CHECK-NOT: build/module.modulemap
 // CHECK: A/module.modulemap
+// CHECK-NOT: build/module.modulemap
 
 //--- build/compile-commands.json.in
 
@@ -17,6 +19,11 @@
   "directory": "DIR",
   "command": "clang DIR/main.m -Imodules/A -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-modules -fimplicit-module-maps -ivfsoverlay build/vfs.yaml",
   "file": "DIR/main.m"
+},
+{
+  "directory": "DIR",
+  "command": "clang DIR/main.m -Imodules/A -fmodules -Xclang -fno-modules-share-filemanager -fmodules-cache-path=DIR/module-cache-unshared -fimplicit-modules -fimplicit-module-maps -ivfsoverlay build/vfs.yaml",
+  "file": "DIR/main.m"
 }
 ]
 
@@ -35,6 +42,7 @@ typedef int A_t;
 {
   "version": 0,
   "case-sensitive": "false",
+  "use-external-names": true,
   "roots": [
   {
      "contents": [
diff --git a/clang/test/Modules/submodule-in-private-mmap-vfs.m b/clang/test/Modules/submodule-in-private-mmap-vfs.m
new file mode 100644
index 000000000000..53149f06ddbf
--- /dev/null
+++ b/clang/test/Modules/submodule-in-private-mmap-vfs.m
@@ -0,0 +1,38 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/vfs.json.in > %t/vfs.json
+// RUN: %clang_cc1 -fmodules -fno-modules-share-filemanager -fimplicit-module-maps \
+// RUN:   -fmodules-cache-path=%t -I%t/Virtual -ivfsoverlay %t/vfs.json -fsyntax-only %t/tu.m -verify
+
+//--- Dir1/module.modulemap
+
+//--- Dir2/module.private.modulemap
+module Foo_Private {}
+
+//--- vfs.json.in
+{
+  'version': 0,
+  'use-external-names': true,
+  'roots': [
+    {
+      'name': 'DIR/Virtual',
+      'type': 'directory',
+      'contents': [
+        {
+          'name': 'module.modulemap',
+          'type': 'file',
+          'external-contents': 'DIR/Dir1/module.modulemap'
+        },
+        {
+          'name': 'module.private.modulemap',
+          'type': 'file',
+          'external-contents': 'DIR/Dir2/module.private.modulemap'
+        }
+      ]
+    }
+  ]
+}
+
+//--- tu.m
+@import Foo_Private;
+// expected-no-diagnostics
diff --git a/clang/test/VFS/module-import.m b/clang/test/VFS/module-import.m
index e3f250c55ff9..25fb3cd6c5c7 100644
--- a/clang/test/VFS/module-import.m
+++ b/clang/test/VFS/module-import.m
@@ -1,6 +1,7 @@
-// RUN: rm -rf %t
+// RUN: rm -rf %t %t-unshared
 // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/vfsoverlay.yaml > %t.yaml
 // RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t.yaml -I %t -fsyntax-only %s
+// RUN: %clang_cc1 -Werror -fmodules -fno-modules-share-filemanager -fimplicit-module-maps -fmodules-cache-path=%t-unshared -ivfsoverlay %t.yaml -I %t -fsyntax-only %s
 
 @import not_real;
 
@@ -18,9 +19,12 @@ void foo(void) {
 // Override the module map (vfsoverlay2 on top)
 // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/vfsoverlay2.yaml > %t2.yaml
 // RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t.yaml -ivfsoverlay %t2.yaml -I %t -fsyntax-only %s
+// RUN: %clang_cc1 -Werror -fmodules -fno-modules-share-filemanager -fimplicit-module-maps -fmodules-cache-path=%t-unshared -ivfsoverlay %t.yaml -ivfsoverlay %t2.yaml -I %t -fsyntax-only %s
 
 // vfsoverlay2 not present
 // RUN: not %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t.yaml -I %t -fsyntax-only %s -DIMPORT2 2>&1 | FileCheck -check-prefix=CHECK-VFS2 %s
+// RUN: not %clang_cc1 -Werror -fmodules -fno-modules-share-filemanager -fimplicit-module-maps -fmodules-cache-path=%t-unshared -ivfsoverlay %t.yaml -I %t -fsyntax-only %s -DIMPORT2 2>&1 | FileCheck -check-prefix=CHECK-VFS2 %s
 
 // vfsoverlay2 on the bottom
 // RUN: not %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t2.yaml -ivfsoverlay %t.yaml -I %t -fsyntax-only %s -DIMPORT2 2>&1 | FileCheck -check-prefix=CHECK-VFS2 %s
+// RUN: not %clang_cc1 -Werror -fmodules -fno-modules-share-filemanager -fimplicit-module-maps -fmodules-cache-path=%t-unshared -ivfsoverlay %t2.yaml -ivfsoverlay %t.yaml -I %t -fsyntax-only %s -DIMPORT2 2>&1 | FileCheck -check-prefix=CHECK-VFS2 %s