[PATCH] D40270: [Modules TS] Added re-export support

[Boris Kolpackov via Phabricator via cfe-commits]

boris added a comment.

All our tests pass as well. Thanks for your work!


Repository:
  rL LLVM

https://reviews.llvm.org/D40270



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40270: [Modules TS] Added re-export support

[Hamza Sood via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL318744: [Modules TS] Added module re-export support. 
(authored by hamzasood).

Changed prior to commit:
  https://reviews.llvm.org/D40270?vs=123666=123736#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D40270

Files:
  cfe/trunk/include/clang/Parse/Parser.h
  cfe/trunk/lib/Parse/ParseObjc.cpp
  cfe/trunk/lib/Parse/Parser.cpp
  cfe/trunk/lib/Sema/SemaDecl.cpp
  cfe/trunk/test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.export/p1.cpp
  cfe/trunk/test/SemaCXX/modules-ts.cppm

Index: cfe/trunk/test/SemaCXX/modules-ts.cppm
===
--- cfe/trunk/test/SemaCXX/modules-ts.cppm
+++ cfe/trunk/test/SemaCXX/modules-ts.cppm
@@ -52,10 +52,6 @@
 export { ; }
 export { static_assert(true); }
 
-// FIXME: These diagnostics are not very good.
-export import foo; // expected-error {{expected unqualified-id}}
-export { import foo; } // expected-error {{expected unqualified-id}}
-
 int use_b = b;
 int use_n = n; // FIXME: this should not be visible, because it is not exported
 
Index: cfe/trunk/test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.export/p1.cpp
===
--- cfe/trunk/test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.export/p1.cpp
+++ cfe/trunk/test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.export/p1.cpp
@@ -0,0 +1,40 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+//
+// RUN: echo 'export module a; export class A{};' | %clang_cc1 -x c++ -fmodules-ts -emit-module-interface - -o %t/a.pcm
+// RUN: echo 'export module b; export class B{};' | %clang_cc1 -x c++ -fmodules-ts -emit-module-interface - -o %t/b.pcm
+// RUN: echo 'export module c; export class C{};' | %clang_cc1 -x c++ -fmodules-ts -emit-module-interface - -o %t/c.pcm
+//
+// RUN: %clang_cc1 -fmodules-ts -fprebuilt-module-path=%t -emit-module-interface %s -o %t/aggregate.internal.pcm -DAGGREGATE_INTERNAL
+// RUN: %clang_cc1 -fmodules-ts -fprebuilt-module-path=%t -emit-module-interface %s -o %t/aggregate.pcm -DAGGREGATE
+//
+// RUN: %clang_cc1 -fmodules-ts -fprebuilt-module-path=%t %s -verify -DTEST
+// expected-no-diagnostics
+
+
+#ifdef AGGREGATE_INTERNAL
+export module aggregate.internal;
+export import a;
+export {
+  import b;
+  import c;
+}
+#endif
+
+
+// Export the above aggregate module.
+// This is done to ensure that re-exports are transitive.
+#ifdef AGGREGATE
+export module aggregate;
+export import aggregate.internal;
+#endif
+
+
+// For the actual test, just try using the classes from the exported modules
+// and hope that they're accessible.
+#ifdef TEST
+import aggregate;
+A a;
+B b;
+C c;
+#endif
Index: cfe/trunk/lib/Sema/SemaDecl.cpp
===
--- cfe/trunk/lib/Sema/SemaDecl.cpp
+++ cfe/trunk/lib/Sema/SemaDecl.cpp
@@ -16171,7 +16171,7 @@
 DC = LSD->getParent();
   }
 
-  while (isa(DC))
+  while (isa(DC) || isa(DC))
 DC = DC->getParent();
 
   if (!isa(DC)) {
@@ -16349,12 +16349,17 @@
 IdentifierLocs.push_back(Path[I].second);
   }
 
-  TranslationUnitDecl *TU = getASTContext().getTranslationUnitDecl();
-  ImportDecl *Import = ImportDecl::Create(Context, TU, StartLoc,
+  ImportDecl *Import = ImportDecl::Create(Context, CurContext, StartLoc,
   Mod, IdentifierLocs);
   if (!ModuleScopes.empty())
 Context.addModuleInitializer(ModuleScopes.back().Module, Import);
-  TU->addDecl(Import);
+  CurContext->addDecl(Import);
+
+  // Re-export the module if needed.
+  if (Import->isExported() &&
+  !ModuleScopes.empty() && ModuleScopes.back().ModuleInterface)
+getCurrentModule()->Exports.emplace_back(Mod, false);
+
   return Import;
 }
 
Index: cfe/trunk/lib/Parse/ParseObjc.cpp
===
--- cfe/trunk/lib/Parse/ParseObjc.cpp
+++ cfe/trunk/lib/Parse/ParseObjc.cpp
@@ -81,8 +81,10 @@
 SingleDecl = ParseObjCPropertyDynamic(AtLoc);
 break;
   case tok::objc_import:
-if (getLangOpts().Modules || getLangOpts().DebuggerSupport)
-  return ParseModuleImport(AtLoc);
+if (getLangOpts().Modules || getLangOpts().DebuggerSupport) {
+  SingleDecl = ParseModuleImport(AtLoc);
+  break;
+}
 Diag(AtLoc, diag::err_atimport);
 SkipUntil(tok::semi);
 return Actions.ConvertDeclToDeclGroup(nullptr);
Index: cfe/trunk/lib/Parse/Parser.cpp
===
--- cfe/trunk/lib/Parse/Parser.cpp
+++ cfe/trunk/lib/Parse/Parser.cpp
@@ -556,10 +556,6 @@
 HandlePragmaUnused();
 return false;
 
-  case tok::kw_import:
-Result = ParseModuleImport(SourceLocation());
-return false;
-
   case tok::kw_export:
 if (NextToken().isNot(tok::kw_module))
   break;
@@ -637,6 +633,9 @@
 ///   ';'
 ///
 /// [C++0x/GNU] 'extern' 'template' declaration
+///
+/// [Modules-TS] 

[PATCH] D40270: [Modules TS] Added re-export support

[Boris Kolpackov via Phabricator via cfe-commits]

boris added a comment.

LGTM. Will be happy to also run our re-export tests in build2 once this is 
merged.


https://reviews.llvm.org/D40270



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40270: [Modules TS] Added re-export support

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

Thanks, looks great.




Comment at: test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.export/p1.cpp:22
+}
+#endif
+

Maybe consider adding one more module to this test: a module that is imported 
into this module but *not* re-exported.


https://reviews.llvm.org/D40270



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40270: [Modules TS] Added re-export support

[Hamza Sood via Phabricator via cfe-commits]

hamzasood created this revision.

This patch adds support for re-exporting modules as described in 
`[dcl.modules.export]`


https://reviews.llvm.org/D40270

Files:
  include/clang/Parse/Parser.h
  lib/Parse/ParseObjc.cpp
  lib/Parse/Parser.cpp
  lib/Sema/SemaDecl.cpp
  test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.export/p1.cpp
  test/SemaCXX/modules-ts.cppm

Index: test/SemaCXX/modules-ts.cppm
===
--- test/SemaCXX/modules-ts.cppm
+++ test/SemaCXX/modules-ts.cppm
@@ -52,10 +52,6 @@
 export { ; }
 export { static_assert(true); }
 
-// FIXME: These diagnostics are not very good.
-export import foo; // expected-error {{expected unqualified-id}}
-export { import foo; } // expected-error {{expected unqualified-id}}
-
 int use_b = b;
 int use_n = n; // FIXME: this should not be visible, because it is not exported
 
Index: test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.export/p1.cpp
===
--- test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.export/p1.cpp
+++ test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.export/p1.cpp
@@ -0,0 +1,40 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+//
+// RUN: echo 'export module a; export class A{};' | %clang_cc1 -x c++ -fmodules-ts -emit-module-interface - -o %t/a.pcm
+// RUN: echo 'export module b; export class B{};' | %clang_cc1 -x c++ -fmodules-ts -emit-module-interface - -o %t/b.pcm
+// RUN: echo 'export module c; export class C{};' | %clang_cc1 -x c++ -fmodules-ts -emit-module-interface - -o %t/c.pcm
+//
+// RUN: %clang_cc1 -fmodules-ts -fprebuilt-module-path=%t -emit-module-interface %s -o %t/aggregate.internal.pcm -DAGGREGATE_INTERNAL
+// RUN: %clang_cc1 -fmodules-ts -fprebuilt-module-path=%t -emit-module-interface %s -o %t/aggregate.pcm -DAGGREGATE
+//
+// RUN: %clang_cc1 -fmodules-ts -fprebuilt-module-path=%t %s -verify -DTEST
+// expected-no-diagnostics
+
+
+#ifdef AGGREGATE_INTERNAL
+export module aggregate.internal;
+export import a;
+export {
+  import b;
+  import c;
+}
+#endif
+
+
+// Export the above aggregate module.
+// This is done to ensure that re-exports are transitive.
+#ifdef AGGREGATE
+export module aggregate;
+export import aggregate.internal;
+#endif
+
+
+// For the actual test, just try using the classes from the exported modules
+// and hope that they're accessible.
+#ifdef TEST
+import aggregate;
+A a;
+B b;
+C c;
+#endif
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -16171,7 +16171,7 @@
 DC = LSD->getParent();
   }
 
-  while (isa(DC))
+  while (isa(DC) || isa(DC))
 DC = DC->getParent();
 
   if (!isa(DC)) {
@@ -16349,12 +16349,17 @@
 IdentifierLocs.push_back(Path[I].second);
   }
 
-  TranslationUnitDecl *TU = getASTContext().getTranslationUnitDecl();
-  ImportDecl *Import = ImportDecl::Create(Context, TU, StartLoc,
+  ImportDecl *Import = ImportDecl::Create(Context, CurContext, StartLoc,
   Mod, IdentifierLocs);
   if (!ModuleScopes.empty())
 Context.addModuleInitializer(ModuleScopes.back().Module, Import);
-  TU->addDecl(Import);
+  CurContext->addDecl(Import);
+
+  // Re-export the module if needed.
+  if (Import->isExported() &&
+  !ModuleScopes.empty() && ModuleScopes.back().ModuleInterface)
+getCurrentModule()->Exports.emplace_back(Mod, false);
+
   return Import;
 }
 
Index: lib/Parse/Parser.cpp
===
--- lib/Parse/Parser.cpp
+++ lib/Parse/Parser.cpp
@@ -556,10 +556,6 @@
 HandlePragmaUnused();
 return false;
 
-  case tok::kw_import:
-Result = ParseModuleImport(SourceLocation());
-return false;
-
   case tok::kw_export:
 if (NextToken().isNot(tok::kw_module))
   break;
@@ -637,6 +633,9 @@
 ///   ';'
 ///
 /// [C++0x/GNU] 'extern' 'template' declaration
+///
+/// [Modules-TS] module-import-declaration
+///
 Parser::DeclGroupPtrTy
 Parser::ParseExternalDeclaration(ParsedAttributesWithRange ,
  ParsingDeclSpec *DS) {
@@ -764,6 +763,9 @@
 CurParsedObjCImpl ? Sema::PCC_ObjCImplementation : Sema::PCC_Namespace);
 cutOffParsing();
 return nullptr;
+  case tok::kw_import:
+SingleDecl = ParseModuleImport(SourceLocation());
+break;
   case tok::kw_export:
 if (getLangOpts().ModulesTS) {
   SingleDecl = ParseExportDeclaration();
@@ -2092,7 +2094,7 @@
 ///   '@' 'import' module-name ';'
 /// [ModTS] module-import-declaration:
 ///   'import' module-name attribute-specifier-seq[opt] ';'
-Parser::DeclGroupPtrTy Parser::ParseModuleImport(SourceLocation AtLoc) {
+Decl *Parser::ParseModuleImport(SourceLocation AtLoc) {
   assert((AtLoc.isInvalid() ? Tok.is(tok::kw_import)
 : Tok.isObjCAtKeyword(tok::objc_import)) &&
  "Improper start to module import");