[C++20] [Modules] Allow -fmodule-file=<module-name>=<BMI-Path> for implementation...
authorChuanqi Xu <yedeng.yd@linux.alibaba.com>
Wed, 8 Feb 2023 08:24:39 +0000 (16:24 +0800)
committerChuanqi Xu <yedeng.yd@linux.alibaba.com>
Wed, 8 Feb 2023 08:45:00 +0000 (16:45 +0800)
Close https://github.com/llvm/llvm-project/issues/57293.

Previsouly we can't use `-fmodule-file=<module-name>=<BMI-Path>` for
implementation units, it is a bug. Also the behavior of the above option
is not tested nor documented for C++20 Modules. This patch addresses the
2 problems.

clang/docs/StandardCPlusPlusModules.rst
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Sema/SemaModule.cpp
clang/test/CXX/modules-ts/basic/basic.link/module-declaration.cpp
clang/test/CXX/modules-ts/dcl.dcl/dcl.module/p2.cpp
clang/test/Modules/cxx20-impl-module-conditionally-load.cppm [new file with mode: 0644]
clang/test/Modules/cxx20-named-conditionally-load.cppm [new file with mode: 0644]

index 44762fa..c8dbaa8 100644 (file)
@@ -329,6 +329,12 @@ https://gcc.gnu.org/onlinedocs/gcc-3.0.2/cpp_9.html.
 How to specify the dependent BMIs
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
+There are 3 methods to specify the dependent BMIs:
+
+* (1) ``-fprebuilt-module-path=<path/to/direcotry>``.
+* (2) ``-fmodule-file=<path/to/BMI>``.
+* (3) ``-fmodule-file=<module-name>=<path/to/BMI>``.
+
 The option ``-fprebuilt-module-path`` tells the compiler the path where to search for dependent BMIs.
 It may be used multiple times just like ``-I`` for specifying paths for header files. The look up rule here is:
 
@@ -337,16 +343,21 @@ It may be used multiple times just like ``-I`` for specifying paths for header f
 * (2) When we import partition module unit M:P. The compiler would look up M-P.pcm in the
   directories specified by ``-fprebuilt-module-path``.
 
-Another way to specify the dependent BMIs is to use ``-fmodule-file``. The main difference
-is that ``-fprebuilt-module-path`` takes a directory, whereas ``-fmodule-file`` requires a
-specific file. In case both the ``-fprebuilt-module-path`` and ``-fmodule-file`` exist, the 
-``-fmodule-file`` option takes higher precedence. In another word, if the compiler finds the wanted
-BMI specified by ``-fmodule-file``, the compiler wouldn't look up again in the directories specified
-by ``-fprebuilt-module-path``.
-
-When we compile a ``module implementation unit``, we must pass the BMI of the corresponding
-``primary module interface unit`` by ``-fmodule-file``
-since the language specification says a module implementation unit implicitly imports
+The option ``-fmodule-file=<path/to/BMI>`` tells the compiler to load the specified BMI directly.
+The option ``-fmodule-file=<module-name>=<path/to/BMI>`` tells the compiler to load the specified BMI
+for the module specified by ``<module-name>`` when necessary. The main difference is that
+``-fmodule-file=<path/to/BMI>`` will load the BMI eagerly, whereas
+``-fmodule-file=<module-name>=<path/to/BMI>`` will only load the BMI lazily, which is similar
+with ``-fprebuilt-module-path``.
+
+In case all ``-fprebuilt-module-path=<path/to/direcotry>``, ``-fmodule-file=<path/to/BMI>`` and
+``-fmodule-file=<module-name>=<path/to/BMI>`` exist, the ``-fmodule-file=<path/to/BMI>`` option
+takes highest precedence and ``-fmodule-file=<module-name>=<path/to/BMI>`` will take the second
+highest precedence.
+
+When we compile a ``module implementation unit``, we must specify the BMI of the corresponding
+``primary module interface unit``.
+Since the language specification says a module implementation unit implicitly imports
 the primary module interface unit.
 
   [module.unit]p8
@@ -354,13 +365,15 @@ the primary module interface unit.
   A module-declaration that contains neither an export-keyword nor a module-partition implicitly
   imports the primary module interface unit of the module as if by a module-import-declaration.
 
-Again, the option ``-fmodule-file`` may occur multiple times.
+All of the 3 options ``-fprebuilt-module-path=<path/to/direcotry>``, ``-fmodule-file=<path/to/BMI>``
+and ``-fmodule-file=<module-name>=<path/to/BMI>`` may occur multiple times.
 For example, the command line to compile ``M.cppm`` in
 the above example could be rewritten into:
 
 .. code-block:: console
 
   $ clang++ -std=c++20 M.cppm --precompile -fmodule-file=M-interface_part.pcm -fmodule-file=M-impl_part.pcm -o M.pcm
+  $ clang++ -std=c++20 M.cppm --precompile -fmodule-file=M:interface_part=M-interface_part.pcm -fmodule-file=M:impl_part=M-impl_part.pcm -o M.pcm
 
 ``-fprebuilt-module-path`` is more convenient and ``-fmodule-file`` is faster since
 it saves time for file lookup.
index 9c79e85..4a8a6b6 100644 (file)
@@ -1986,14 +1986,7 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
     Module = PP->getHeaderSearchInfo().lookupModule(
         ModuleName, ImportLoc, /*AllowSearch*/ true,
         /*AllowExtraModuleMapSearch*/ !IsInclusionDirective);
-    /// FIXME: perhaps we should (a) look for a module using the module name
-    //  to file map (PrebuiltModuleFiles) and (b) diagnose if still not found?
-    //if (Module == nullptr) {
-    //  getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_found)
-    //    << ModuleName;
-    //  DisableGeneratingGlobalModuleIndex = true;
-    //  return ModuleLoadResult();
-    //}
+
     MM.cacheModuleLoad(*Path[0].first, Module);
   } else {
     ModuleLoadResult Result = findOrCompileModuleAndReadAST(
index f52c024..194239a 100644 (file)
@@ -337,20 +337,29 @@ Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
   }
 
   case ModuleDeclKind::Implementation: {
-    std::pair<IdentifierInfo *, SourceLocation> ModuleNameLoc(
-        PP.getIdentifierInfo(ModuleName), Path[0].second);
     // C++20 A module-declaration that contains neither an export-
     // keyword nor a module-partition implicitly imports the primary
     // module interface unit of the module as if by a module-import-
     // declaration.
+    std::pair<IdentifierInfo *, SourceLocation> ModuleNameLoc(
+        PP.getIdentifierInfo(ModuleName), Path[0].second);
+
+    // The module loader will assume we're trying to import the module that
+    // we're building if `LangOpts.CurrentModule` equals to 'ModuleName'.
+    // Change the value for `LangOpts.CurrentModule` temporarily to make the
+    // module loader work properly.
+    const_cast<LangOptions&>(getLangOpts()).CurrentModule = "";
     Mod = getModuleLoader().loadModule(ModuleLoc, {ModuleNameLoc},
                                        Module::AllVisible,
                                        /*IsInclusionDirective=*/false);
+    const_cast<LangOptions&>(getLangOpts()).CurrentModule = ModuleName;
+
     if (!Mod) {
       Diag(ModuleLoc, diag::err_module_not_defined) << ModuleName;
       // Create an empty module interface unit for error recovery.
       Mod = Map.createModuleForInterfaceUnit(ModuleLoc, ModuleName);
     }
+
   } break;
 
   case ModuleDeclKind::PartitionImplementation:
index 09e1097..6cdf04a 100644 (file)
@@ -42,7 +42,7 @@ EXPORT module MODULE_NAME;
 #elif TEST == 10
 // expected-error-re@-9 {{'maybe_unused' attribute cannot be applied to a module{{$}}}}
 #elif TEST == 1
-// expected-error@-11 {{definition of module 'z' is not available}}
+// expected-error@-11 {{module 'z' not found}}
 #else
 // expected-no-diagnostics
 #endif
index 992715e..53ab19e 100644 (file)
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -fmodules-ts -verify %s
 
 // A named module shall contain exactly one module interface unit.
-module M; // expected-error {{definition of module 'M' is not available; use -fmodule-file= to specify path to precompiled module interface}}
+module M; // expected-error {{module 'M' not found}}
 
 // FIXME: How do we ensure there is not more than one?
diff --git a/clang/test/Modules/cxx20-impl-module-conditionally-load.cppm b/clang/test/Modules/cxx20-impl-module-conditionally-load.cppm
new file mode 100644 (file)
index 0000000..d7dc896
--- /dev/null
@@ -0,0 +1,15 @@
+// From https://github.com/llvm/llvm-project/issues/57293
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/m.cppm -emit-module-interface -o %t/m.pcm
+// RUN: %clang_cc1 -std=c++20 %t/m.cpp -fmodule-file=m=%t/m.pcm -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 %t/m.cpp -fprebuilt-module-path=%t -verify -fsyntax-only
+
+//--- m.cppm
+export module m;
+
+//--- m.cpp
+// expected-no-diagnostics
+module m;
diff --git a/clang/test/Modules/cxx20-named-conditionally-load.cppm b/clang/test/Modules/cxx20-named-conditionally-load.cppm
new file mode 100644 (file)
index 0000000..85254d0
--- /dev/null
@@ -0,0 +1,37 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 %t/b.cppm -fmodule-file=a=%t/a.pcm -fsyntax-only -verify
+// RUN: %clang_cc1 -std=c++20 %t/use.cpp -fmodule-file=a=%t/a.pcm -fsyntax-only -verify
+//
+// RUN: %clang_cc1 -std=c++20 %t/M-Part.cppm -emit-module-interface -o %t/M-Part.pcm
+// RUN: %clang_cc1 -std=c++20 %t/M.cppm -fmodule-file=M:Part=%t/M-Part.pcm -fsyntax-only -verify
+
+//--- a.cppm
+export module a;
+export int foo() { return 43; }
+
+//--- b.cppm
+// expected-no-diagnostics
+export module b;
+import a;
+export int b() {
+    return foo();
+}
+
+//--- use.cpp
+// expected-no-diagnostics
+import a;
+int Use() {
+    return foo();
+}
+
+//--- M-Part.cppm
+export module M:Part;
+
+//--- M.cppm
+// expected-no-diagnostics
+export module M;
+import :Part;