[clang] 1782e8f - [C++20] [Modules] Allow -fmodule-file=<module-name>=<BMI-Path> for implementation unit and document the behavior
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 8 00:45:36 PST 2023
Author: Chuanqi Xu
Date: 2023-02-08T16:45:00+08:00
New Revision: 1782e8f9e882e8f4fb59968ff555c8c93827ea02
URL: https://github.com/llvm/llvm-project/commit/1782e8f9e882e8f4fb59968ff555c8c93827ea02
DIFF: https://github.com/llvm/llvm-project/commit/1782e8f9e882e8f4fb59968ff555c8c93827ea02.diff
LOG: [C++20] [Modules] Allow -fmodule-file=<module-name>=<BMI-Path> for implementation unit and document the behavior
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.
Added:
clang/test/Modules/cxx20-impl-module-conditionally-load.cppm
clang/test/Modules/cxx20-named-conditionally-load.cppm
Modified:
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
Removed:
################################################################################
diff --git a/clang/docs/StandardCPlusPlusModules.rst b/clang/docs/StandardCPlusPlusModules.rst
index 44762fa6b3153..c8dbaa89bbfe1 100644
--- a/clang/docs/StandardCPlusPlusModules.rst
+++ b/clang/docs/StandardCPlusPlusModules.rst
@@ -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
diff erence
-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
diff erence 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.
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index 9c79e856dad4f..4a8a6b607dd75 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -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(
diff --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index f52c0247f01cd..194239ab0e100 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/clang/lib/Sema/SemaModule.cpp
@@ -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:
diff --git a/clang/test/CXX/modules-ts/basic/basic.link/module-declaration.cpp b/clang/test/CXX/modules-ts/basic/basic.link/module-declaration.cpp
index 09e10978323c3..6cdf04af47ad3 100644
--- a/clang/test/CXX/modules-ts/basic/basic.link/module-declaration.cpp
+++ b/clang/test/CXX/modules-ts/basic/basic.link/module-declaration.cpp
@@ -42,7 +42,7 @@ EXPORT module MODULE_NAME;
#elif TEST == 10
// expected-error-re at -9 {{'maybe_unused' attribute cannot be applied to a module{{$}}}}
#elif TEST == 1
-// expected-error at -11 {{definition of module 'z' is not available}}
+// expected-error at -11 {{module 'z' not found}}
#else
// expected-no-diagnostics
#endif
diff --git a/clang/test/CXX/modules-ts/dcl.dcl/dcl.module/p2.cpp b/clang/test/CXX/modules-ts/dcl.dcl/dcl.module/p2.cpp
index 992715e6b1021..53ab19e246a0e 100644
--- a/clang/test/CXX/modules-ts/dcl.dcl/dcl.module/p2.cpp
+++ b/clang/test/CXX/modules-ts/dcl.dcl/dcl.module/p2.cpp
@@ -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
index 0000000000000..d7dc896456b2d
--- /dev/null
+++ b/clang/test/Modules/cxx20-impl-module-conditionally-load.cppm
@@ -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
index 0000000000000..85254d0fdfd7f
--- /dev/null
+++ b/clang/test/Modules/cxx20-named-conditionally-load.cppm
@@ -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;
More information about the cfe-commits
mailing list