Index: include/clang/Basic/DiagnosticParseKinds.td =================================================================== --- include/clang/Basic/DiagnosticParseKinds.td +++ include/clang/Basic/DiagnosticParseKinds.td @@ -1013,6 +1013,7 @@ "expected a module name after module import">; def err_module_expected_semi : Error< "expected ';' after module name">; +def err_missing_before_module_end : Error<"expected %0 at the end of module">; } let CategoryName = "Generics Issue" in { Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -7764,11 +7764,15 @@ "extern \"C\" language linkage specification begins here">; def err_module_import_not_at_top_level : Error< "import of module '%0' appears within %1">; +def err_module_import_not_at_top_level_fatal : Error< + "import of module '%0' appears within %1">, DefaultFatal; def note_module_import_not_at_top_level : Note<"%0 begins here">; def err_module_self_import : Error< "import of module '%0' appears within same top-level module '%1'">; def err_module_import_in_implementation : Error< "@import of module '%0' in implementation of '%1'; use #import">; +def note_module_need_top_level : Note<"consider moving the import to top level">; +def note_module_need_textual : Note<"consider marking the header as textual">; } let CategoryName = "Documentation Issue" in { Index: include/clang/Parse/Parser.h =================================================================== --- include/clang/Parse/Parser.h +++ include/clang/Parse/Parser.h @@ -2555,6 +2555,7 @@ //===--------------------------------------------------------------------===// // Modules DeclGroupPtrTy ParseModuleImport(SourceLocation AtLoc); + bool tryParseMisplacedModuleImport(); //===--------------------------------------------------------------------===// // C++11/G++: Type Traits [Type-Traits.html in the GCC manual] Index: include/clang/Sema/Sema.h =================================================================== --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -1799,6 +1799,10 @@ /// \brief The parser has left a submodule. void ActOnModuleEnd(SourceLocation DirectiveLoc, Module *Mod); + /// \brief Check if module import may be found in the current context, + /// emit error if not. + void diagnoseMisplacedModuleImport(Module *M, SourceLocation ImportLoc); + /// \brief Create an implicit import of the given module at the given /// source location, for error recovery, if possible. /// Index: lib/Parse/ParseDeclCXX.cpp =================================================================== --- lib/Parse/ParseDeclCXX.cpp +++ lib/Parse/ParseDeclCXX.cpp @@ -210,7 +210,8 @@ ParsedAttributes &attrs, BalancedDelimiterTracker &Tracker) { if (index == Ident.size()) { - while (Tok.isNot(tok::r_brace) && !isEofOrEom()) { + while (tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) && + Tok.isNot(tok::eof)) { ParsedAttributesWithRange attrs(AttrFactory); MaybeParseCXX11Attributes(attrs); MaybeParseMicrosoftAttributes(attrs); @@ -3063,11 +3064,12 @@ if (TagDecl) { // While we still have something to read, read the member-declarations. - while (Tok.isNot(tok::r_brace) && !isEofOrEom()) + while (tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) && + Tok.isNot(tok::eof)) { // Each iteration of this loop reads one member-declaration. ParseCXXClassMemberDeclarationWithPragmas( CurAS, AccessAttrs, static_cast(TagType), TagDecl); - + } T.consumeClose(); } else { SkipUntil(tok::r_brace); Index: lib/Parse/ParseStmt.cpp =================================================================== --- lib/Parse/ParseStmt.cpp +++ lib/Parse/ParseStmt.cpp @@ -944,7 +944,8 @@ Stmts.push_back(R.get()); } - while (Tok.isNot(tok::r_brace) && !isEofOrEom()) { + while (tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) && + Tok.isNot(tok::eof)) { if (Tok.is(tok::annot_pragma_unused)) { HandlePragmaUnused(); continue; Index: lib/Parse/Parser.cpp =================================================================== --- lib/Parse/Parser.cpp +++ lib/Parse/Parser.cpp @@ -2018,6 +2018,37 @@ return Actions.ConvertDeclToDeclGroup(Import.get()); } +/// \brief Try recover parser when module annotation appears where it must not +/// be found. +/// \returns true if the recover was successful and parsing may be continued, or +/// false if parser must bail out to top level and handle the token there. +bool Parser::tryParseMisplacedModuleImport() { + while (true) { + switch (Tok.getKind()) { + case tok::annot_module_end: + // Inform caller that recovery failed, the error must be handled at upper + // level. + return false; + case tok::annot_module_begin: + Actions.diagnoseMisplacedModuleImport(reinterpret_cast( + Tok.getAnnotationValue()), Tok.getLocation()); + return false; + case tok::annot_module_include: + // Module import found where it should not be, for instance, inside a + // namespace. Recover by importing the module. + Actions.ActOnModuleInclude(Tok.getLocation(), + reinterpret_cast( + Tok.getAnnotationValue())); + ConsumeToken(); + // If there is another module import, process it. + continue; + default: + return true; + } + } + return true; +} + bool BalancedDelimiterTracker::diagnoseOverflow() { P.Diag(P.Tok, diag::err_bracket_depth_exceeded) << P.getLangOpts().BracketDepth; @@ -2045,7 +2076,10 @@ bool BalancedDelimiterTracker::diagnoseMissingClose() { assert(!P.Tok.is(Close) && "Should have consumed closing delimiter"); - P.Diag(P.Tok, diag::err_expected) << Close; + if (P.Tok.is(tok::annot_module_end)) + P.Diag(P.Tok, diag::err_missing_before_module_end) << Close; + else + P.Diag(P.Tok, diag::err_expected) << Close; P.Diag(LOpen, diag::note_matching) << Kind; // If we're not already at some kind of closing bracket, skip to our closing Index: lib/Sema/SemaDecl.cpp =================================================================== --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -14345,6 +14345,7 @@ S.Diag(ImportLoc, diag::err_module_import_in_extern_c) << M->getFullModuleName(); S.Diag(LSD->getLocStart(), diag::note_module_import_in_extern_c); + S.Diag(ImportLoc, diag::note_module_need_top_level); return; } break; @@ -14357,14 +14358,26 @@ while (isa(DC)) DC = DC->getParent(); if (!isa(DC)) { - S.Diag(ImportLoc, diag::err_module_import_not_at_top_level) - << M->getFullModuleName() << DC; - S.Diag(cast(DC)->getLocStart(), - diag::note_module_import_not_at_top_level) - << DC; + if (DC->isNamespace()) { + S.Diag(ImportLoc, diag::err_module_import_not_at_top_level) + << M->getFullModuleName() << DC; + S.Diag(cast(DC)->getLocStart(), + diag::note_module_import_not_at_top_level) << DC; + S.Diag(ImportLoc, diag::note_module_need_top_level); + } else { + S.Diag(ImportLoc, diag::err_module_import_not_at_top_level_fatal) + << M->getFullModuleName() << DC; + S.Diag(cast(DC)->getLocStart(), + diag::note_module_import_not_at_top_level) << DC; + S.Diag(ImportLoc, diag::note_module_need_textual); + } } } +void Sema::diagnoseMisplacedModuleImport(Module *M, SourceLocation ImportLoc) { + return checkModuleImportContext(*this, M, ImportLoc, CurContext); +} + DeclResult Sema::ActOnModuleImport(SourceLocation AtLoc, SourceLocation ImportLoc, ModuleIdPath Path) { Index: test/Modules/auto-module-import.m =================================================================== --- test/Modules/auto-module-import.m +++ test/Modules/auto-module-import.m @@ -83,6 +83,8 @@ return not_in_module; } -void includeNotAtTopLevel() { // expected-note {{to match this '{'}} - #include // expected-warning {{treating #include as an import}} expected-error {{expected '}'}} -} // expected-error {{extraneous closing brace}} +void includeNotAtTopLevel() { // expected-note {{function 'includeNotAtTopLevel' begins here}} + #include // expected-warning {{treating #include as an import}} \ + expected-error {{import of module 'NoUmbrella.A' appears within function 'includeNotAtTopLevel'}} \ + expected-note {{consider marking the header as textual}} +} Index: test/Modules/extern_c.cpp =================================================================== --- test/Modules/extern_c.cpp +++ test/Modules/extern_c.cpp @@ -39,9 +39,11 @@ #if defined(EXTERN_C) && !defined(EXTERN_CXX) && defined(CXX_HEADER) // expected-error@-3 {{import of C++ module 'cxx_library' appears within extern "C" language linkage specification}} // expected-note@-17 {{extern "C" language linkage specification begins here}} +// expected-note@-5 {{consider moving the import to top level}} #elif defined(NAMESPACE) -// expected-error-re@-6 {{import of module '{{c_library.inner|cxx_library}}' appears within namespace 'M'}} -// expected-note@-24 {{namespace 'M' begins here}} +// expected-error-re@-7 {{import of module '{{c_library.inner|cxx_library}}' appears within namespace 'M'}} +// expected-note@-25 {{namespace 'M' begins here}} +// expected-note@-9 {{consider moving the import to top level}} #endif #ifdef EXTERN_CXX Index: test/Modules/malformed.cpp =================================================================== --- test/Modules/malformed.cpp +++ test/Modules/malformed.cpp @@ -12,20 +12,14 @@ #include STR(HEADER) // CHECK-A: While building module 'malformed_a' -// CHECK-A: {{^}}Inputs/malformed/a1.h:1:{{.*}} error: expected '}' +// CHECK-A: {{^}}Inputs/malformed/a1.h:1:{{.*}} error: expected '}' at the end of module // CHECK-A: {{^}}Inputs/malformed/a1.h:1:{{.*}} note: to match this '{' // // CHECK-A: While building module 'malformed_a' // CHECK-A: {{^}}Inputs/malformed/a2.h:1:{{.*}} error: extraneous closing brace // CHECK-B: While building module 'malformed_b' -// CHECK-B: {{^}}Inputs/malformed/b1.h:2:{{.*}} error: expected '}' -// CHECK-B: {{^}}Inputs/malformed/b1.h:1:{{.*}} note: to match this '{' -// CHECK-B: {{^}}Inputs/malformed/b1.h:3:{{.*}} error: extraneous closing brace ('}') -// -// CHECK-B: While building module 'malformed_b' -// CHECK-B: {{^}}Inputs/malformed/b2.h:1:{{.*}} error: redefinition of 'g' -// CHECK-B: {{^}}Inputs/malformed/b2.h:1:{{.*}} note: previous definition is here +// CHECK-B: {{^}}Inputs/malformed/b1.h:2:{{.*}} error: import of module 'malformed_b.b2' appears within 'S' void test() { f(); } // Test that we use relative paths to name files within an imported module. Index: test/Modules/misplaced.cpp =================================================================== --- /dev/null +++ test/Modules/misplaced.cpp @@ -0,0 +1,12 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs %s -verify + +namespace N1 { // expected-note{{namespace 'N1' begins here}} +#include "dummy.h" // expected-error{{import of module 'dummy' appears within namespace 'N1'}} \ + // expected-note{{consider moving the import to top level}} +} + +void func1() { // expected-note{{function 'func1' begins here}} +#include "dummy.h" // expected-error{{import of module 'dummy' appears within function 'func1'}} \ + // expected-note{{consider marking the header as textual}} +}