From 81328ac3a5b1bfc24010ab5ada34a5ee8f4a32ae Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Fri, 21 Apr 2017 22:39:18 +0000 Subject: [PATCH] P0629R0: Switch to latest proposal for distinguishing module interface from implementation. This switches from the prototype syntax in P0273R0 ('module' and 'module implementation') to the consensus syntax 'export module' and 'module'. In passing, drop the "module declaration must be first" enforcement, since EWG seems to have changed its mind on that. llvm-svn: 301056 --- clang/include/clang/Basic/DiagnosticParseKinds.td | 10 ++-- clang/include/clang/Basic/DiagnosticSemaKinds.td | 6 +- clang/include/clang/Sema/Sema.h | 3 +- clang/lib/Parse/Parser.cpp | 67 ++++++++++------------- clang/lib/Sema/SemaDecl.cpp | 43 +++++++++------ clang/test/CodeGenCXX/modules-ts.cppm | 2 +- clang/test/Driver/modules-ts.cpp | 12 ++-- clang/test/Parser/cxx-modules-import.cpp | 42 +++++++------- clang/test/Parser/cxx-modules-interface.cppm | 6 +- clang/test/SemaCXX/modules-ts.cppm | 4 +- 10 files changed, 99 insertions(+), 96 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td index d95e43c..f04ed8e 100644 --- a/clang/include/clang/Basic/DiagnosticParseKinds.td +++ b/clang/include/clang/Basic/DiagnosticParseKinds.td @@ -1114,14 +1114,12 @@ def err_pragma_cannot_end_force_cuda_host_device : Error< } // end of Parse Issue category. let CategoryName = "Modules Issue" in { -def err_expected_module_interface_decl : Error< - "expected module declaration at start of module interface">; def err_unexpected_module_decl : Error< - "module declaration must be the first declaration in the translation unit">; + "module declaration can only appear at the top level">; def err_module_expected_ident : Error< - "expected a module name after module%select{| import}0">; -def err_unexpected_module_kind : Error< - "unexpected module kind %0; expected 'implementation' or 'partition'">; + "expected a module name after '%select{module|import}0'">; +def err_module_implementation_partition : Error< + "module partition must be declared 'export'">; def err_attribute_not_module_attr : Error< "%0 attribute cannot be applied to a module">; def err_attribute_not_import_attr : Error< diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 6cb872c..914e304d 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -8801,9 +8801,11 @@ def err_invalid_type_for_program_scope_var : Error< } let CategoryName = "Modules Issue" in { +def err_module_decl_in_module_map_module : Error< + "'module' declaration found while building module from module map">; def err_module_interface_implementation_mismatch : Error< - "%select{'module'|'module partition'|'module implementation'}0 declaration " - "found while %select{not |not |}0building module interface">; + "missing 'export' specifier in 'module' declaration while " + "building module interface">; def err_current_module_name_mismatch : Error< "module name '%0' specified on command line does not match name of module">; def err_module_redefinition : Error< diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index bd68842..d72193e 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -1934,7 +1934,8 @@ public: /// The parser has processed a module-declaration that begins the definition /// of a module interface or implementation. - DeclGroupPtrTy ActOnModuleDecl(SourceLocation ModuleLoc, ModuleDeclKind MDK, + DeclGroupPtrTy ActOnModuleDecl(SourceLocation StartLoc, + SourceLocation ModuleLoc, ModuleDeclKind MDK, ModuleIdPath Path); /// \brief The parser has processed a module import declaration. diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp index edbfc63..38476ce 100644 --- a/clang/lib/Parse/Parser.cpp +++ b/clang/lib/Parse/Parser.cpp @@ -534,23 +534,6 @@ void Parser::LateTemplateParserCleanupCallback(void *P) { } bool Parser::ParseFirstTopLevelDecl(DeclGroupPtrTy &Result) { - // C++ Modules TS: module-declaration must be the first declaration in the - // file. (There can be no preceding preprocessor directives, but we expect - // the lexer to check that.) - if (Tok.is(tok::kw_module)) { - Result = ParseModuleDecl(); - return false; - } else if (getLangOpts().getCompilingModule() == - LangOptions::CMK_ModuleInterface) { - // FIXME: We avoid providing this diagnostic when generating an object file - // from an existing PCM file. This is not a good way to detect this - // condition; we should provide a mechanism to indicate whether we've - // already parsed a declaration in this translation unit and avoid calling - // ParseFirstTopLevelDecl in that case. - if (Actions.TUKind == TU_Module) - Diag(Tok, diag::err_expected_module_interface_decl); - } - // C11 6.9p1 says translation units must have at least one top-level // declaration. C++ doesn't have this restriction. We also don't want to // complain if we have a precompiled header, although technically if the PCH @@ -583,6 +566,14 @@ bool Parser::ParseTopLevelDecl(DeclGroupPtrTy &Result) { Result = ParseModuleImport(SourceLocation()); return false; + case tok::kw_export: + if (NextToken().isNot(tok::kw_module)) + break; + LLVM_FALLTHROUGH; + case tok::kw_module: + Result = ParseModuleDecl(); + return false; + case tok::annot_module_include: Actions.ActOnModuleInclude(Tok.getLocation(), reinterpret_cast( @@ -2049,30 +2040,28 @@ void Parser::ParseMicrosoftIfExistsExternalDeclaration() { /// Parse a C++ Modules TS module declaration, which appears at the beginning /// of a module interface, module partition, or module implementation file. /// -/// module-declaration: [Modules TS + P0273R0] -/// 'module' module-kind[opt] module-name attribute-specifier-seq[opt] ';' -/// module-kind: -/// 'implementation' -/// 'partition' +/// module-declaration: [Modules TS + P0273R0 + P0629R0] +/// 'export'[opt] 'module' 'partition'[opt] +/// module-name attribute-specifier-seq[opt] ';' /// -/// Note that the module-kind values are context-sensitive keywords. +/// Note that 'partition' is a context-sensitive keyword. Parser::DeclGroupPtrTy Parser::ParseModuleDecl() { - assert(Tok.is(tok::kw_module) && getLangOpts().ModulesTS && - "should not be parsing a module declaration"); + SourceLocation StartLoc = Tok.getLocation(); + + Sema::ModuleDeclKind MDK = TryConsumeToken(tok::kw_export) + ? Sema::ModuleDeclKind::Module + : Sema::ModuleDeclKind::Implementation; + + assert(Tok.is(tok::kw_module) && "not a module declaration"); SourceLocation ModuleLoc = ConsumeToken(); - // Check for a module-kind. - Sema::ModuleDeclKind MDK = Sema::ModuleDeclKind::Module; - if (Tok.is(tok::identifier) && NextToken().is(tok::identifier)) { - if (Tok.getIdentifierInfo()->isStr("implementation")) - MDK = Sema::ModuleDeclKind::Implementation; - else if (Tok.getIdentifierInfo()->isStr("partition")) - MDK = Sema::ModuleDeclKind::Partition; - else { - Diag(Tok, diag::err_unexpected_module_kind) << Tok.getIdentifierInfo(); - SkipUntil(tok::semi); - return nullptr; - } + if (Tok.is(tok::identifier) && NextToken().is(tok::identifier) && + Tok.getIdentifierInfo()->isStr("partition")) { + // If 'partition' is present, this must be a module interface unit. + if (MDK != Sema::ModuleDeclKind::Module) + Diag(Tok.getLocation(), diag::err_module_implementation_partition) + << FixItHint::CreateInsertion(ModuleLoc, "export "); + MDK = Sema::ModuleDeclKind::Partition; ConsumeToken(); } @@ -2080,14 +2069,14 @@ Parser::DeclGroupPtrTy Parser::ParseModuleDecl() { if (ParseModuleName(ModuleLoc, Path, /*IsImport*/false)) return nullptr; + // We don't support any module attributes yet; just parse them and diagnose. ParsedAttributesWithRange Attrs(AttrFactory); MaybeParseCXX11Attributes(Attrs); - // We don't support any module attributes yet. ProhibitCXX11Attributes(Attrs, diag::err_attribute_not_module_attr); ExpectAndConsumeSemi(diag::err_module_expected_semi); - return Actions.ActOnModuleDecl(ModuleLoc, MDK, Path); + return Actions.ActOnModuleDecl(StartLoc, ModuleLoc, MDK, Path); } /// Parse a module import declaration. This is essentially the same for diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 6269c9f..6138cbb 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -15688,30 +15688,41 @@ static void checkModuleImportContext(Sema &S, Module *M, } } -Sema::DeclGroupPtrTy Sema::ActOnModuleDecl(SourceLocation ModuleLoc, +Sema::DeclGroupPtrTy Sema::ActOnModuleDecl(SourceLocation StartLoc, + SourceLocation ModuleLoc, ModuleDeclKind MDK, ModuleIdPath Path) { - // 'module implementation' requires that we are not compiling a module of any - // kind. 'module' and 'module partition' require that we are compiling a - // module inteface (not a module map). - auto CMK = getLangOpts().getCompilingModule(); - if (MDK == ModuleDeclKind::Implementation - ? CMK != LangOptions::CMK_None - : CMK != LangOptions::CMK_ModuleInterface) { + // 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. + switch (getLangOpts().getCompilingModule()) { + case LangOptions::CMK_None: + // It's OK to compile a module interface as a normal translation unit. + break; + + case LangOptions::CMK_ModuleInterface: + if (MDK != ModuleDeclKind::Implementation) + break; + + // We were asked to compile a module interface unit but this is a module + // implementation unit. That indicates the 'export' is missing. Diag(ModuleLoc, diag::err_module_interface_implementation_mismatch) - << (unsigned)MDK; + << FixItHint::CreateInsertion(ModuleLoc, "export "); + break; + + case LangOptions::CMK_ModuleMap: + Diag(ModuleLoc, diag::err_module_decl_in_module_map_module); return nullptr; } // FIXME: Create a ModuleDecl and return it. // FIXME: Most of this work should be done by the preprocessor rather than - // here, in case we look ahead across something where the current - // module matters (eg a #include). + // here, in order to support macro import. - // The dots in a module name in the Modules TS are a lie. Unlike Clang's - // hierarchical module map modules, the dots here are just another character - // that can appear in a module name. Flatten down to the actual module name. + // Flatten the dots in a module name. Unlike Clang's hierarchical module map + // modules, the dots here are just another character that can appear in a + // module name. std::string ModuleName; for (auto &Piece : Path) { if (!ModuleName.empty()) @@ -15736,8 +15747,8 @@ Sema::DeclGroupPtrTy Sema::ActOnModuleDecl(SourceLocation ModuleLoc, case ModuleDeclKind::Module: { // FIXME: Check we're not in a submodule. - // We can't have imported a definition of this module or parsed a module - // map defining it already. + // 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)) { Diag(Path[0].second, diag::err_module_redefinition) << ModuleName; if (M->DefinitionLoc.isValid()) diff --git a/clang/test/CodeGenCXX/modules-ts.cppm b/clang/test/CodeGenCXX/modules-ts.cppm index 90f6b53..d1552d9 100644 --- a/clang/test/CodeGenCXX/modules-ts.cppm +++ b/clang/test/CodeGenCXX/modules-ts.cppm @@ -1,7 +1,7 @@ // RUN: %clang_cc1 -fmodules-ts -std=c++1z -triple=x86_64-linux-gnu -fmodules-codegen -emit-module-interface %s -o %t.pcm // RUN: %clang_cc1 -fmodules-ts -std=c++1z -triple=x86_64-linux-gnu %t.pcm -emit-llvm -o - | FileCheck %s -module FooBar; +export module FooBar; export { // CHECK-LABEL: define i32 @_Z1fv( diff --git a/clang/test/Driver/modules-ts.cpp b/clang/test/Driver/modules-ts.cpp index fd33914..3847b71 100644 --- a/clang/test/Driver/modules-ts.cpp +++ b/clang/test/Driver/modules-ts.cpp @@ -1,6 +1,6 @@ // Check compiling a module interface to a .pcm file. // -// RUN: %clang -fmodules-ts -x c++-module --precompile %s -Dimplementation= -o %t.pcm -v 2>&1 | FileCheck %s --check-prefix=CHECK-PRECOMPILE +// RUN: %clang -fmodules-ts -x c++-module --precompile %s -o %t.pcm -v 2>&1 | FileCheck %s --check-prefix=CHECK-PRECOMPILE // // CHECK-PRECOMPILE: -cc1 {{.*}} -emit-module-interface // CHECK-PRECOMPILE-SAME: -o {{.*}}.pcm @@ -18,7 +18,7 @@ // Check use of a .pcm file in another compilation. // -// RUN: %clang -fmodules-ts -fmodule-file=%t.pcm %s -c -o %t.o -v 2>&1 | FileCheck %s --check-prefix=CHECK-USE +// RUN: %clang -fmodules-ts -fmodule-file=%t.pcm -Dexport= %s -c -o %t.o -v 2>&1 | FileCheck %s --check-prefix=CHECK-USE // // CHECK-USE: -cc1 // CHECK-USE-SAME: -emit-obj @@ -28,11 +28,11 @@ // Check combining precompile and compile steps works. // -// RUN: %clang -fmodules-ts -x c++-module %s -Dimplementation= -c -o %t.pcm.o -v 2>&1 | FileCheck %s --check-prefix=CHECK-PRECOMPILE --check-prefix=CHECK-COMPILE +// RUN: %clang -fmodules-ts -x c++-module %s -c -o %t.pcm.o -v 2>&1 | FileCheck %s --check-prefix=CHECK-PRECOMPILE --check-prefix=CHECK-COMPILE // Check that .cppm is treated as a module implicitly. // RUN: cp %s %t.cppm -// RUN: %clang -fmodules-ts --precompile %t.cppm -Dimplementation= -o %t.pcm -v 2>&1 | FileCheck %s --check-prefix=CHECK-PRECOMPILE +// RUN: %clang -fmodules-ts --precompile %t.cppm -o %t.pcm -v 2>&1 | FileCheck %s --check-prefix=CHECK-PRECOMPILE -// Note, we use -Dimplementation= to make this a valid module interface unit when building the interface. -module implementation foo; +// Note, we use -Dexport= to make this a module implementation unit when building the implementation. +export module foo; diff --git a/clang/test/Parser/cxx-modules-import.cpp b/clang/test/Parser/cxx-modules-import.cpp index 0600edd..98c4838 100644 --- a/clang/test/Parser/cxx-modules-import.cpp +++ b/clang/test/Parser/cxx-modules-import.cpp @@ -1,39 +1,41 @@ // RUN: rm -rf %t // RUN: mkdir -p %t -// RUN: echo 'module x; int a, b;' > %t/x.cppm -// RUN: echo 'module x.y; int c;' > %t/x.y.cppm +// RUN: echo 'export module x; int a, b;' > %t/x.cppm +// RUN: echo 'export module x.y; int c;' > %t/x.y.cppm // // RUN: %clang_cc1 -std=c++1z -fmodules-ts -emit-module-interface %t/x.cppm -o %t/x.pcm // RUN: %clang_cc1 -std=c++1z -fmodules-ts -emit-module-interface -fmodule-file=%t/x.pcm %t/x.y.cppm -o %t/x.y.pcm // // RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=%t/x.y.pcm -verify %s \ -// RUN: -DTEST=1 -DMODULE_KIND=implementation -DMODULE_NAME=z +// RUN: -DTEST=1 -DEXPORT= -DPARTITION= -DMODULE_NAME=z // RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=%t/x.y.pcm -verify %s \ -// RUN: -DTEST=2 -DMODULE_KIND=implementation -DMODULE_NAME=x +// RUN: -DTEST=2 -DEXPORT= -DPARTITION= -DMODULE_NAME=x // RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=%t/x.y.pcm -verify %s \ -// RUN: -DTEST=3 -DMODULE_KIND= -DMODULE_NAME=z +// RUN: -DTEST=3 -DEXPORT=export -DPARTITION= -DMODULE_NAME=z // RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=%t/x.y.pcm -verify %s \ -// RUN: -DTEST=4 -DMODULE_KIND=partition -DMODULE_NAME=z +// RUN: -DTEST=4 -DEXPORT=export -DPARTITION=partition -DMODULE_NAME=z // RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=%t/x.y.pcm -verify %s \ -// RUN: -DTEST=5 -DMODULE_KIND=elderberry -DMODULE_NAME=z +// RUN: -DTEST=5 -DEXPORT= -DPARTITION=elderberry -DMODULE_NAME=z // RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=%t/x.y.pcm -verify %s \ -// RUN: -DTEST=1 -DMODULE_KIND=implementation -DMODULE_NAME='z [[]]' +// RUN: -DTEST=1 -DEXPORT= -DPARTITION= -DMODULE_NAME='z [[]]' // RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=%t/x.y.pcm -verify %s \ -// RUN: -DTEST=6 -DMODULE_KIND=implementation -DMODULE_NAME='z [[fancy]]' +// RUN: -DTEST=6 -DEXPORT= -DPARTITION= -DMODULE_NAME='z [[fancy]]' // RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=%t/x.y.pcm -verify %s \ -// RUN: -DTEST=7 -DMODULE_KIND=implementation -DMODULE_NAME='z [[maybe_unused]]' +// RUN: -DTEST=7 -DEXPORT= -DPARTITION= -DMODULE_NAME='z [[maybe_unused]]' +// RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=%t/x.y.pcm -verify %s \ +// RUN: -DTEST=8 -DEXPORT= -DPARTITION=partition -DMODULE_NAME=z -module MODULE_KIND MODULE_NAME; -#if TEST == 3 -// expected-error@-2 {{'module' declaration found while not building module interface}} -#elif TEST == 4 -// expected-error@-4 {{'module partition' declaration found while not building module interface}} +EXPORT module PARTITION MODULE_NAME; +#if TEST == 3 || TEST == 4 +// ok, building object code for module interface #elif TEST == 5 -// expected-error@-6 {{unexpected module kind 'elderberry'}} +// expected-error@-4 {{expected ';'}} expected-error@-4 {{requires a type specifier}} #elif TEST == 6 -// expected-warning@-8 {{unknown attribute 'fancy' ignored}} +// expected-warning@-6 {{unknown attribute 'fancy' ignored}} #elif TEST == 7 -// expected-error-re@-10 {{'maybe_unused' attribute cannot be applied to a module{{$}}}} +// expected-error-re@-8 {{'maybe_unused' attribute cannot be applied to a module{{$}}}} +#elif TEST == 8 +// expected-error@-10 {{module partition must be declared 'export'}} #endif int use_1 = a; @@ -52,7 +54,7 @@ import x [[noreturn]]; // expected-error {{'noreturn' attribute cannot be applie import x [[blarg::noreturn]]; // expected-warning {{unknown attribute 'noreturn' ignored}} import x.y; -import x.; // expected-error {{expected a module name after module import}} -import .x; // expected-error {{expected a module name after module import}} +import x.; // expected-error {{expected a module name after 'import'}} +import .x; // expected-error {{expected a module name after 'import'}} import blarg; // expected-error {{module 'blarg' not found}} diff --git a/clang/test/Parser/cxx-modules-interface.cppm b/clang/test/Parser/cxx-modules-interface.cppm index f7835bd..628b854 100644 --- a/clang/test/Parser/cxx-modules-interface.cppm +++ b/clang/test/Parser/cxx-modules-interface.cppm @@ -1,13 +1,13 @@ // RUN: %clang_cc1 -std=c++1z -fmodules-ts -emit-module-interface %s -o %t.pcm -verify // RUN: %clang_cc1 -std=c++1z -fmodules-ts -emit-module-interface %s -o %t.pcm -verify -Dmodule=int -DERRORS -module foo; +export module foo; #ifndef ERRORS // expected-no-diagnostics #else -// expected-error@-4 {{expected module declaration at start of module interface}} +// FIXME: diagnose 'export' declaration in non-module +// FIXME: diagnose missing module-declaration when building module interface -// FIXME: support 'export module X;' and 'export { int n; module X; }' // FIXME: proclaimed-ownership-declarations? export { diff --git a/clang/test/SemaCXX/modules-ts.cppm b/clang/test/SemaCXX/modules-ts.cppm index 71c09d3..109a1f4 100644 --- a/clang/test/SemaCXX/modules-ts.cppm +++ b/clang/test/SemaCXX/modules-ts.cppm @@ -7,9 +7,9 @@ // expected-no-diagnostics #endif -module foo; +export module foo; #if TEST == 1 -// expected-error@-2 {{expected module declaration at start of module interface}} +// FIXME: diagnose export outside of module interface unit #elif TEST == 2 // CHECK-2: error: redefinition of module 'foo' #endif -- 2.7.4