hamzasood updated this revision to Diff 150649.
hamzasood added a comment.
Addressed Richard's comments.
https://reviews.llvm.org/D40443
Files:
include/clang/Basic/Module.h
lib/Basic/Module.cpp
lib/Sema/SemaDecl.cpp
test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp
test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1.cpp
test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1/interface-imports.cpp
Index: test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1.cpp
===================================================================
--- test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1.cpp
+++ test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1.cpp
@@ -1,28 +0,0 @@
-// RUN: %clang_cc1 -fmodules-ts %s -verify -o /dev/null
-// RUN: %clang_cc1 -fmodules-ts %s -DINTERFACE -verify -emit-module-interface -o %t
-// RUN: %clang_cc1 -fmodules-ts %s -DIMPLEMENTATION -verify -fmodule-file=%t -o /dev/null
-//
-// RUN: %clang_cc1 -fmodules-ts %s -DBUILT_AS_INTERFACE -emit-module-interface -verify -o /dev/null
-// RUN: %clang_cc1 -fmodules-ts %s -DINTERFACE -DBUILT_AS_INTERFACE -emit-module-interface -verify -o /dev/null
-// RUN: %clang_cc1 -fmodules-ts %s -DIMPLEMENTATION -DBUILT_AS_INTERFACE -emit-module-interface -verify -o /dev/null
-
-#if INTERFACE
-// expected-no-diagnostics
-export module A;
-#elif IMPLEMENTATION
-module A;
- #ifdef BUILT_AS_INTERFACE
- // expected-error@-2 {{missing 'export' specifier in module declaration while building module interface}}
- #define INTERFACE
- #endif
-#else
- #ifdef BUILT_AS_INTERFACE
- // expected-error@1 {{missing 'export module' declaration in module interface unit}}
- #endif
-#endif
-
-#ifndef INTERFACE
-export int b; // expected-error {{export declaration can only be used within a module interface unit}}
-#else
-export int a;
-#endif
Index: test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1/interface-imports.cpp
===================================================================
--- test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1/interface-imports.cpp
+++ test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1/interface-imports.cpp
@@ -0,0 +1,40 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: echo 'export module a; export class A { };' > %t/a.cppm
+// RUN: echo 'export module b; export class B { };' > %t/b.cppm
+//
+// RUN: %clang_cc1 -fmodules-ts -emit-module-interface %t/a.cppm -o %t/a.pcm
+// RUN: %clang_cc1 -fmodules-ts -emit-module-interface %t/b.cppm -o %t/b.pcm
+// RUN: %clang_cc1 -fmodules-ts -fprebuilt-module-path=%t -emit-module-interface %s -o %t/test.pcm -DTEST_INTERFACE
+//
+// RUN: %clang_cc1 -fmodules-ts -I%t -fmodule-file=%t/test.pcm %s -verify -DTEST_IMPLEMENTATION
+// RUN: %clang_cc1 -fmodules-ts -I%t -fmodule-file=%t/test.pcm %s -verify -DOTHER_TU
+
+
+#ifdef TEST_INTERFACE
+import a;
+export module test;
+import b;
+#endif
+
+#ifdef TEST_IMPLEMENTATION
+module test;
+
+A a; // expected-error {{must be imported from module 'a'}}
+ // expected-n...@a.cppm:1 {{here}}
+
+// Module b is imported within the purview of this module's interface unit.
+// So its exported definitions should be visible here.
+B b;
+#endif
+
+
+#ifdef OTHER_TU
+import test;
+
+A a; // expected-error {{must be imported from module 'a'}}
+ // expected-n...@a.cppm:1 {{here}}
+
+B b; // expected-error {{must be imported from module 'b'}}
+ // expected-n...@b.cppm:1 {{here}}
+#endif
Index: test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp
===================================================================
--- test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp
+++ test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp
@@ -10,9 +10,7 @@
//
// RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/M.pcm -emit-module-interface -o %t/N.pcm -DMODULE_INTERFACE -DNO_ERRORS
// RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/N.pcm -verify
-// FIXME: Once we start importing "import" declarations properly, this should
-// be rejected (-verify should be added to the following line).
-// RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/N.pcm -DNO_IMPORT
+// RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/N.pcm -DNO_IMPORT -verify
//
// RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/M.pcm -emit-module-interface -o %t/N-no-M.pcm -DMODULE_INTERFACE -DNO_ERRORS -DNO_IMPORT
// RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/N-no-M.pcm -verify
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -16570,6 +16570,17 @@
TU->setModuleOwnershipKind(Decl::ModuleOwnershipKind::ModulePrivate);
TU->setLocalOwningModule(Mod);
+ // Modules TS + p0731r0 [dcl.module.interface]p1:
+ // Every name of an entity with linkage other than internal linkage made
+ // visible in the purview of the module interface unit of a module M is
+ // visible in the purview of all module implementation units of M.
+ if (MDK == ModuleDeclKind::Implementation) {
+ for (Module *ImportedModule : Mod->Imports) {
+ VisibleModules.setVisible(ImportedModule, ModuleLoc);
+ getModuleLoader().makeModuleVisible(Mod, Module::AllVisible, StartLoc);
+ }
+ }
+
// FIXME: Create a ModuleDecl.
return nullptr;
}
@@ -16617,10 +16628,21 @@
Context.addModuleInitializer(ModuleScopes.back().Module, Import);
CurContext->addDecl(Import);
- // Re-export the module if needed.
- if (Import->isExported() &&
- !ModuleScopes.empty() && ModuleScopes.back().ModuleInterface)
- getCurrentModule()->Exports.emplace_back(Mod, false);
+ if (getLangOpts().ModulesTS) {
+ Module *CurrentModule = getCurrentModule();
+ assert(CurrentModule && "Expected to be in a module scope");
+
+ if (getLangOpts().getCompilingModule() == LangOptions::CMK_ModuleInterface) {
+ // Record imports that occur in a module interface unit; other units may
+ // need to know what's visible in the interface unit (e.g. for code
+ // synthesis purposes).
+ CurrentModule->Imports.insert(Mod);
+
+ // Re-export the module if needed.
+ if (Import->isExported())
+ CurrentModule->Exports.emplace_back(Mod, false);
+ }
+ }
return Import;
}
Index: lib/Basic/Module.cpp
===================================================================
--- lib/Basic/Module.cpp
+++ lib/Basic/Module.cpp
@@ -359,6 +359,18 @@
// This module is visible to itself.
VisibleModulesCache.insert(this);
+ // Everything visible to the interface unit's global module fragment is
+ // visible to the interface unit.
+ if (Kind == ModuleKind::ModuleInterfaceUnit) {
+ assert(!SubModules.empty() &&
+ SubModules.front()->Kind == ModuleKind::GlobalModuleFragment);
+ const Module *Global = SubModules.front();
+ if (Global->VisibleModulesCache.empty())
+ Global->buildVisibleModulesCache();
+ VisibleModulesCache.insert(Global->VisibleModulesCache.begin(),
+ Global->VisibleModulesCache.end());
+ }
+
// Every imported module is visible.
SmallVector<Module *, 16> Stack(Imports.begin(), Imports.end());
while (!Stack.empty()) {
Index: include/clang/Basic/Module.h
===================================================================
--- include/clang/Basic/Module.h
+++ include/clang/Basic/Module.h
@@ -539,6 +539,8 @@
/// Determine whether the specified module would be visible to
/// a lookup at the end of this module.
///
+ /// Note that if this is a C++ module interface unit, then modules visible
+ /// here aren't necessarily visible from the implementation units.
/// FIXME: This may return incorrect results for (submodules of) the
/// module currently being built, if it's queried before we see all
/// of its imports.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
