iains updated this revision to Diff 409627.
iains marked 2 inline comments as done.
iains added a comment.
address review comments, rebase
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118587/new/
https://reviews.llvm.org/D118587
Files:
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaModule.cpp
clang/test/Modules/cxx20-10-1-ex1.cpp
clang/test/Modules/cxx20-10-1-ex2.cpp
clang/test/Modules/cxx20-import-diagnostics-a.cpp
Index: clang/test/Modules/cxx20-import-diagnostics-a.cpp
===================================================================
--- clang/test/Modules/cxx20-import-diagnostics-a.cpp
+++ clang/test/Modules/cxx20-import-diagnostics-a.cpp
@@ -94,9 +94,9 @@
#elif TU == 6
module;
-// We can only have preprocessor commands here, which could include an include
+// We can only have preprocessor directives here, which permits an include-
// translated header unit. However those are identified specifically by the
-// preprocessor; non-preprocessed user code should not contain an import here.
+// preprocessor; non-preprocessed user code should not contain an 'import' here.
import B; // expected-error {{module imports cannot be in the global module fragment}}
export module D;
Index: clang/test/Modules/cxx20-10-1-ex2.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/cxx20-10-1-ex2.cpp
@@ -0,0 +1,60 @@
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=1 -x c++ %s \
+// RUN: -o %t/B_Y.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=2 -x c++ %s \
+// RUN: -fmodule-file=%t/B_Y.pcm -o %t/B.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=3 -x c++ %s \
+// RUN: -o %t/B_X1.pcm -verify
+
+// Not expected to work yet.
+// %clang_cc1 -std=c++20 -emit-module-interface -D TU=4 -x c++ %s \
+// -fmodule-file=%t/B.pcm -o %t/B_X2.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj -D TU=5 -x c++ %s \
+// RUN: -fmodule-file=%t/B.pcm -o %t/b_tu5.s
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=6 -x c++ %s \
+// RUN: -fmodule-file=%t/B.pcm -o %t/b_tu6.s -verify
+
+// Not expected to work yet.
+// %clang_cc1 -std=c++20 -emit-module-interface -D TU=7 -x c++ %s \
+// -fmodule-file=%t/B_X2.pcm -o %t/B_X3.pcm -verify
+
+#if TU == 1
+ module B:Y;
+ int y();
+// expected-no-diagnostics
+#elif TU == 2
+ export module B;
+ import :Y;
+ int n = y();
+// expected-no-diagnostics
+#elif TU == 3
+ module B:X1; // does not implicitly import B
+ int &a = n; // expected-error {{use of undeclared identifier }}
+#elif TU == 4
+ module B:X2; // does not implicitly import B
+ import B;
+ int &b = n; // OK
+// expected-no-diagnostics
+#elif TU == 5
+ module B; // implicitly imports B
+ int &c = n; // OK
+// expected-no-diagnostics
+#elif TU == 6
+ import B;
+ // error, n is module-local and this is not a module.
+ int &c = n; // expected-error {{use of undeclared identifier}}
+#elif TU == 7
+ module B:X3; // does not implicitly import B
+ import :X2; // X2 is an implementation so exports nothing.
+ // error: n not visible here.
+ int &c = n; // expected-error {{use of undeclared identifier }}
+#else
+#error "no TU set"
+#endif
Index: clang/test/Modules/cxx20-10-1-ex1.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/cxx20-10-1-ex1.cpp
@@ -0,0 +1,50 @@
+// The example in the standard is not in required build order.
+// revised here
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=1 -x c++ %s \
+// RUN: -o %t/A_Internals.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=2 -x c++ %s \
+// RUN: -fmodule-file=%t/A_Internals.pcm -o %t/A_Foo.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=3 -x c++ %s \
+// RUN: -fmodule-file=%t/A_Foo.pcm -o %t/A.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj -D TU=4 -x c++ %s \
+// RUN: -fmodule-file=%t/A.pcm -o %t/ex1.o
+
+// expected-no-diagnostics
+
+#if TU == 1
+
+module A:Internals;
+int bar();
+
+#elif TU == 2
+
+export module A:Foo;
+
+import :Internals;
+
+export int foo() { return 2 * (bar() + 1); }
+
+#elif TU == 3
+
+export module A;
+export import :Foo;
+export int baz();
+
+#elif TU == 4
+module A;
+
+import :Internals;
+
+int bar() { return baz() - 10; }
+int baz() { return 30; }
+
+#else
+#error "no TU set"
+#endif
Index: clang/lib/Sema/SemaModule.cpp
===================================================================
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -110,9 +110,24 @@
// module state;
ImportState = ModuleImportState::NotACXX20Module;
- // A module implementation unit requires that we are not compiling a module
- // of any kind. A module interface unit requires that we are not compiling a
- // module map.
+ bool IsPartition = !Partition.empty();
+ if (IsPartition)
+ switch (MDK) {
+ case ModuleDeclKind::Implementation:
+ MDK = ModuleDeclKind::PartitionImplementation;
+ break;
+ case ModuleDeclKind::Interface:
+ MDK = ModuleDeclKind::PartitionInterface;
+ break;
+ default:
+ llvm::unreachable("how did we get a partition type set?");
+ }
+
+ // A (non-partition) module implementation unit requires that we are not
+ // compiling a module of any kind. A partition implementation emits an
+ // interface (and the AST for the implementation), which will subsequently
+ // be consumed to emit a binary.
+ // A module interface unit requires that we are not compiling a module map.
switch (getLangOpts().getCompilingModule()) {
case LangOptions::CMK_None:
// It's OK to compile a module interface as a normal translation unit.
@@ -123,7 +138,7 @@
break;
// We were asked to compile a module interface unit but this is a module
- // implementation unit. That indicates the 'export' is missing.
+ // implementation unit.
Diag(ModuleLoc, diag::err_module_interface_implementation_mismatch)
<< FixItHint::CreateInsertion(ModuleLoc, "export ");
MDK = ModuleDeclKind::Interface;
@@ -180,7 +195,6 @@
// modules, the dots here are just another character that can appear in a
// module name.
std::string ModuleName = stringFromPath(Path);
- bool IsPartition = !Partition.empty();
if (IsPartition) {
ModuleName += ":";
ModuleName += stringFromPath(Partition);
@@ -202,7 +216,8 @@
Module *Mod;
switch (MDK) {
- case ModuleDeclKind::Interface: {
+ case ModuleDeclKind::Interface:
+ case ModuleDeclKind::PartitionInterface: {
// We can't have parsed or imported a definition of this module or parsed a
// module map defining it already.
if (auto *M = Map.findModule(ModuleName)) {
@@ -219,36 +234,36 @@
// Create a Module for the module that we're defining.
Mod = Map.createModuleForInterfaceUnit(ModuleLoc, ModuleName,
GlobalModuleFragment);
- if (IsPartition)
+ if (MDK == ModuleDeclKind::PartitionInterface)
Mod->Kind = Module::ModulePartitionInterface;
assert(Mod && "module creation should not fail");
break;
}
- case ModuleDeclKind::Implementation:
+ case ModuleDeclKind::Implementation: {
std::pair<IdentifierInfo *, SourceLocation> ModuleNameLoc(
PP.getIdentifierInfo(ModuleName), Path[0].second);
- if (IsPartition) {
- // Create an interface, but note that it is an implementation
- // unit.
+ // 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.
+ Mod = getModuleLoader().loadModule(ModuleLoc, {ModuleNameLoc},
+ Module::AllVisible,
+ /*IsInclusionDirective=*/false);
+ if (!Mod) {
+ Diag(ModuleLoc, diag::err_module_not_defined) << ModuleName;
+ // Create an empty module interface unit for error recovery.
Mod = Map.createModuleForInterfaceUnit(ModuleLoc, ModuleName,
GlobalModuleFragment);
- Mod->Kind = Module::ModulePartitionImplementation;
- } else {
- // 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.
- Mod = getModuleLoader().loadModule(ModuleLoc, {ModuleNameLoc},
- Module::AllVisible,
- /*IsInclusionDirective=*/false);
- if (!Mod) {
- Diag(ModuleLoc, diag::err_module_not_defined) << ModuleName;
- // Create an empty module interface unit for error recovery.
- Mod = Map.createModuleForInterfaceUnit(ModuleLoc, ModuleName,
- GlobalModuleFragment);
- }
}
+ } break;
+
+ case ModuleDeclKind::PartitionImplementation:
+ // Create an interface, but note that it is an implementation
+ // unit.
+ Mod = Map.createModuleForInterfaceUnit(ModuleLoc, ModuleName,
+ GlobalModuleFragment);
+ Mod->Kind = Module::ModulePartitionImplementation;
break;
}
@@ -264,8 +279,7 @@
// Switch from the global module fragment (if any) to the named module.
ModuleScopes.back().BeginLoc = StartLoc;
ModuleScopes.back().Module = Mod;
- ModuleScopes.back().ModuleInterface =
- (MDK != ModuleDeclKind::Implementation || IsPartition);
+ ModuleScopes.back().ModuleInterface = MDK != ModuleDeclKind::Implementation;
ModuleScopes.back().IsPartition = IsPartition;
VisibleModules.setVisible(Mod, ModuleLoc);
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -2946,8 +2946,10 @@
SourceLocation SemiLoc);
enum class ModuleDeclKind {
- Interface, ///< 'export module X;'
- Implementation, ///< 'module X;'
+ Interface, ///< 'export module X;'
+ Implementation, ///< 'module X;'
+ PartitionInterface, ///< 'export module X:Y;'
+ PartitionImplementation, ///< 'module X:Y;'
};
/// An enumeration to represent the transition of states in parsing module
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
