Skip to content

Commit 2e6db25

Browse files
ChuanqiXu9tru
authored andcommitted
[C++20] [Modules] Allow -fmodule-file=<module-name>=<BMI-Path> for implementation unit and document the behavior
Close #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. (cherry picked from commit 1782e8f)
1 parent 6cdd684 commit 2e6db25

File tree

7 files changed

+90
-23
lines changed

7 files changed

+90
-23
lines changed

clang/docs/StandardCPlusPlusModules.rst

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,12 @@ https://gcc.gnu.org/onlinedocs/gcc-3.0.2/cpp_9.html.
329329
How to specify the dependent BMIs
330330
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
331331

332+
There are 3 methods to specify the dependent BMIs:
333+
334+
* (1) ``-fprebuilt-module-path=<path/to/direcotry>``.
335+
* (2) ``-fmodule-file=<path/to/BMI>``.
336+
* (3) ``-fmodule-file=<module-name>=<path/to/BMI>``.
337+
332338
The option ``-fprebuilt-module-path`` tells the compiler the path where to search for dependent BMIs.
333339
It may be used multiple times just like ``-I`` for specifying paths for header files. The look up rule here is:
334340

@@ -337,30 +343,37 @@ It may be used multiple times just like ``-I`` for specifying paths for header f
337343
* (2) When we import partition module unit M:P. The compiler would look up M-P.pcm in the
338344
directories specified by ``-fprebuilt-module-path``.
339345

340-
Another way to specify the dependent BMIs is to use ``-fmodule-file``. The main difference
341-
is that ``-fprebuilt-module-path`` takes a directory, whereas ``-fmodule-file`` requires a
342-
specific file. In case both the ``-fprebuilt-module-path`` and ``-fmodule-file`` exist, the
343-
``-fmodule-file`` option takes higher precedence. In another word, if the compiler finds the wanted
344-
BMI specified by ``-fmodule-file``, the compiler wouldn't look up again in the directories specified
345-
by ``-fprebuilt-module-path``.
346-
347-
When we compile a ``module implementation unit``, we must pass the BMI of the corresponding
348-
``primary module interface unit`` by ``-fmodule-file``
349-
since the language specification says a module implementation unit implicitly imports
346+
The option ``-fmodule-file=<path/to/BMI>`` tells the compiler to load the specified BMI directly.
347+
The option ``-fmodule-file=<module-name>=<path/to/BMI>`` tells the compiler to load the specified BMI
348+
for the module specified by ``<module-name>`` when necessary. The main difference is that
349+
``-fmodule-file=<path/to/BMI>`` will load the BMI eagerly, whereas
350+
``-fmodule-file=<module-name>=<path/to/BMI>`` will only load the BMI lazily, which is similar
351+
with ``-fprebuilt-module-path``.
352+
353+
In case all ``-fprebuilt-module-path=<path/to/direcotry>``, ``-fmodule-file=<path/to/BMI>`` and
354+
``-fmodule-file=<module-name>=<path/to/BMI>`` exist, the ``-fmodule-file=<path/to/BMI>`` option
355+
takes highest precedence and ``-fmodule-file=<module-name>=<path/to/BMI>`` will take the second
356+
highest precedence.
357+
358+
When we compile a ``module implementation unit``, we must specify the BMI of the corresponding
359+
``primary module interface unit``.
360+
Since the language specification says a module implementation unit implicitly imports
350361
the primary module interface unit.
351362

352363
[module.unit]p8
353364

354365
A module-declaration that contains neither an export-keyword nor a module-partition implicitly
355366
imports the primary module interface unit of the module as if by a module-import-declaration.
356367

357-
Again, the option ``-fmodule-file`` may occur multiple times.
368+
All of the 3 options ``-fprebuilt-module-path=<path/to/direcotry>``, ``-fmodule-file=<path/to/BMI>``
369+
and ``-fmodule-file=<module-name>=<path/to/BMI>`` may occur multiple times.
358370
For example, the command line to compile ``M.cppm`` in
359371
the above example could be rewritten into:
360372

361373
.. code-block:: console
362374
363375
$ clang++ -std=c++20 M.cppm --precompile -fmodule-file=M-interface_part.pcm -fmodule-file=M-impl_part.pcm -o M.pcm
376+
$ 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
364377
365378
``-fprebuilt-module-path`` is more convenient and ``-fmodule-file`` is faster since
366379
it saves time for file lookup.

clang/lib/Frontend/CompilerInstance.cpp

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1983,14 +1983,7 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
19831983
Module = PP->getHeaderSearchInfo().lookupModule(
19841984
ModuleName, ImportLoc, /*AllowSearch*/ true,
19851985
/*AllowExtraModuleMapSearch*/ !IsInclusionDirective);
1986-
/// FIXME: perhaps we should (a) look for a module using the module name
1987-
// to file map (PrebuiltModuleFiles) and (b) diagnose if still not found?
1988-
//if (Module == nullptr) {
1989-
// getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_found)
1990-
// << ModuleName;
1991-
// DisableGeneratingGlobalModuleIndex = true;
1992-
// return ModuleLoadResult();
1993-
//}
1986+
19941987
MM.cacheModuleLoad(*Path[0].first, Module);
19951988
} else {
19961989
ModuleLoadResult Result = findOrCompileModuleAndReadAST(

clang/lib/Sema/SemaModule.cpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -337,20 +337,29 @@ Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
337337
}
338338

339339
case ModuleDeclKind::Implementation: {
340-
std::pair<IdentifierInfo *, SourceLocation> ModuleNameLoc(
341-
PP.getIdentifierInfo(ModuleName), Path[0].second);
342340
// C++20 A module-declaration that contains neither an export-
343341
// keyword nor a module-partition implicitly imports the primary
344342
// module interface unit of the module as if by a module-import-
345343
// declaration.
344+
std::pair<IdentifierInfo *, SourceLocation> ModuleNameLoc(
345+
PP.getIdentifierInfo(ModuleName), Path[0].second);
346+
347+
// The module loader will assume we're trying to import the module that
348+
// we're building if `LangOpts.CurrentModule` equals to 'ModuleName'.
349+
// Change the value for `LangOpts.CurrentModule` temporarily to make the
350+
// module loader work properly.
351+
const_cast<LangOptions&>(getLangOpts()).CurrentModule = "";
346352
Mod = getModuleLoader().loadModule(ModuleLoc, {ModuleNameLoc},
347353
Module::AllVisible,
348354
/*IsInclusionDirective=*/false);
355+
const_cast<LangOptions&>(getLangOpts()).CurrentModule = ModuleName;
356+
349357
if (!Mod) {
350358
Diag(ModuleLoc, diag::err_module_not_defined) << ModuleName;
351359
// Create an empty module interface unit for error recovery.
352360
Mod = Map.createModuleForInterfaceUnit(ModuleLoc, ModuleName);
353361
}
362+
354363
} break;
355364

356365
case ModuleDeclKind::PartitionImplementation:

clang/test/CXX/modules-ts/basic/basic.link/module-declaration.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ EXPORT module MODULE_NAME;
4242
#elif TEST == 10
4343
// expected-error-re@-9 {{'maybe_unused' attribute cannot be applied to a module{{$}}}}
4444
#elif TEST == 1
45-
// expected-error@-11 {{definition of module 'z' is not available}}
45+
// expected-error@-11 {{module 'z' not found}}
4646
#else
4747
// expected-no-diagnostics
4848
#endif
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// RUN: %clang_cc1 -fmodules-ts -verify %s
22

33
// A named module shall contain exactly one module interface unit.
4-
module M; // expected-error {{definition of module 'M' is not available; use -fmodule-file= to specify path to precompiled module interface}}
4+
module M; // expected-error {{module 'M' not found}}
55

66
// FIXME: How do we ensure there is not more than one?
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// From https://github.com/llvm/llvm-project/issues/57293
2+
// RUN: rm -rf %t
3+
// RUN: mkdir -p %t
4+
// RUN: split-file %s %t
5+
//
6+
// RUN: %clang_cc1 -std=c++20 %t/m.cppm -emit-module-interface -o %t/m.pcm
7+
// RUN: %clang_cc1 -std=c++20 %t/m.cpp -fmodule-file=m=%t/m.pcm -verify -fsyntax-only
8+
// RUN: %clang_cc1 -std=c++20 %t/m.cpp -fprebuilt-module-path=%t -verify -fsyntax-only
9+
10+
//--- m.cppm
11+
export module m;
12+
13+
//--- m.cpp
14+
// expected-no-diagnostics
15+
module m;
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// RUN: rm -rf %t
2+
// RUN: mkdir -p %t
3+
// RUN: split-file %s %t
4+
//
5+
// RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-module-interface -o %t/a.pcm
6+
// RUN: %clang_cc1 -std=c++20 %t/b.cppm -fmodule-file=a=%t/a.pcm -fsyntax-only -verify
7+
// RUN: %clang_cc1 -std=c++20 %t/use.cpp -fmodule-file=a=%t/a.pcm -fsyntax-only -verify
8+
//
9+
// RUN: %clang_cc1 -std=c++20 %t/M-Part.cppm -emit-module-interface -o %t/M-Part.pcm
10+
// RUN: %clang_cc1 -std=c++20 %t/M.cppm -fmodule-file=M:Part=%t/M-Part.pcm -fsyntax-only -verify
11+
12+
//--- a.cppm
13+
export module a;
14+
export int foo() { return 43; }
15+
16+
//--- b.cppm
17+
// expected-no-diagnostics
18+
export module b;
19+
import a;
20+
export int b() {
21+
return foo();
22+
}
23+
24+
//--- use.cpp
25+
// expected-no-diagnostics
26+
import a;
27+
int Use() {
28+
return foo();
29+
}
30+
31+
//--- M-Part.cppm
32+
export module M:Part;
33+
34+
//--- M.cppm
35+
// expected-no-diagnostics
36+
export module M;
37+
import :Part;

0 commit comments

Comments
 (0)