sepavloff created this revision. sepavloff added a subscriber: cfe-commits.
If an import directive was put into wrong context, the error message was obscure, complaining on misbalanced braces. To get more descriptive messages, annotation tokens related to modules are processed where they must not be seen. http://reviews.llvm.org/D11844 Files: include/clang/Sema/Sema.h lib/Parse/ParseDeclCXX.cpp lib/Parse/ParseStmt.cpp lib/Parse/Parser.cpp lib/Sema/SemaDecl.cpp test/Modules/auto-module-import.m test/Modules/malformed.cpp test/Modules/misplaced.cpp
Index: test/Modules/misplaced.cpp =================================================================== --- /dev/null +++ test/Modules/misplaced.cpp @@ -0,0 +1,14 @@ +// 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'}} +} + +void func1() { // expected-note{{function 'func1' begins here}} +#include "dummy.h" // expected-error{{import of module 'dummy' appears within function 'func1'}} +} + +struct S1 { // expected-note{{'S1' begins here}} +#include "dummy.h" // expected-error{{import of module 'dummy' appears within 'S1'}} +}; Index: test/Modules/malformed.cpp =================================================================== --- test/Modules/malformed.cpp +++ test/Modules/malformed.cpp @@ -12,20 +12,18 @@ #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:{{.*}} note: to match this '{' +// CHECK-A: {{^}}Inputs/malformed/a1.h:1:{{.*}} error: import of module 'malformed_a.a1' appears within function +// CHECK-A: {{^}}Inputs/malformed/a1.h:1:{{.*}} note: function 'f' begins here // // 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: {{^}}Inputs/malformed/b1.h:2:{{.*}} error: import of module 'malformed_b.b2' appears within 'S' +// CHECK-B: {{^}}Inputs/malformed/b1.h:1:{{.*}} note: 'S' begins here // // 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/b2.h:1:{{.*}} error: import of module 'malformed_b.b2' appears within 'S' +// CHECK-B: {{^}}Inputs/malformed/b1.h:1:{{.*}} note: 'S' begins here void test() { f<int>(); } // Test that we use relative paths to name files within an imported module. Index: test/Modules/auto-module-import.m =================================================================== --- test/Modules/auto-module-import.m +++ test/Modules/auto-module-import.m @@ -83,6 +83,6 @@ return not_in_module; } -void includeNotAtTopLevel() { // expected-note {{to match this '{'}} - #include <NoUmbrella/A.h> // expected-warning {{treating #include as an import}} expected-error {{expected '}'}} -} // expected-error {{extraneous closing brace}} +void includeNotAtTopLevel() { // expected-note {{function 'includeNotAtTopLevel' begins here}} + #include <NoUmbrella/A.h> // expected-warning {{treating #include as an import}} expected-error {{import of module 'NoUmbrella.A' appears within function 'includeNotAtTopLevel'}} +} Index: lib/Sema/SemaDecl.cpp =================================================================== --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -14244,16 +14244,15 @@ return New; } -static void checkModuleImportContext(Sema &S, Module *M, - SourceLocation ImportLoc, - DeclContext *DC) { +void Sema::checkModuleImportContext(Module *M, SourceLocation ImportLoc, + DeclContext *DC) { if (auto *LSD = dyn_cast<LinkageSpecDecl>(DC)) { switch (LSD->getLanguage()) { case LinkageSpecDecl::lang_c: if (!M->IsExternC) { - S.Diag(ImportLoc, diag::err_module_import_in_extern_c) + Diag(ImportLoc, diag::err_module_import_in_extern_c) << M->getFullModuleName(); - S.Diag(LSD->getLocStart(), diag::note_module_import_in_extern_c); + Diag(LSD->getLocStart(), diag::note_module_import_in_extern_c); return; } break; @@ -14266,10 +14265,10 @@ while (isa<LinkageSpecDecl>(DC)) DC = DC->getParent(); if (!isa<TranslationUnitDecl>(DC)) { - S.Diag(ImportLoc, diag::err_module_import_not_at_top_level) + Diag(ImportLoc, diag::err_module_import_not_at_top_level) << M->getFullModuleName() << DC; - S.Diag(cast<Decl>(DC)->getLocStart(), - diag::note_module_import_not_at_top_level) + Diag(cast<Decl>(DC)->getLocStart(), + diag::note_module_import_not_at_top_level) << DC; } } @@ -14285,7 +14284,7 @@ VisibleModules.setVisible(Mod, ImportLoc); - checkModuleImportContext(*this, Mod, ImportLoc, CurContext); + checkModuleImportContext(Mod, ImportLoc, CurContext); // FIXME: we should support importing a submodule within a different submodule // of the same top-level module. Until we do, make it an error rather than @@ -14318,7 +14317,7 @@ } void Sema::ActOnModuleInclude(SourceLocation DirectiveLoc, Module *Mod) { - checkModuleImportContext(*this, Mod, DirectiveLoc, CurContext); + checkModuleImportContext(Mod, DirectiveLoc, CurContext); // Determine whether we're in the #include buffer for a module. The #includes // in that buffer do not qualify as module imports; they're just an @@ -14345,15 +14344,15 @@ } void Sema::ActOnModuleBegin(SourceLocation DirectiveLoc, Module *Mod) { - checkModuleImportContext(*this, Mod, DirectiveLoc, CurContext); + checkModuleImportContext(Mod, DirectiveLoc, CurContext); if (getLangOpts().ModulesLocalVisibility) VisibleModulesStack.push_back(std::move(VisibleModules)); VisibleModules.setVisible(Mod, DirectiveLoc); } void Sema::ActOnModuleEnd(SourceLocation DirectiveLoc, Module *Mod) { - checkModuleImportContext(*this, Mod, DirectiveLoc, CurContext); + checkModuleImportContext(Mod, DirectiveLoc, CurContext); if (getLangOpts().ModulesLocalVisibility) { VisibleModules = std::move(VisibleModulesStack.back()); Index: lib/Parse/Parser.cpp =================================================================== --- lib/Parse/Parser.cpp +++ lib/Parse/Parser.cpp @@ -793,7 +793,15 @@ case tok::kw___if_not_exists: ParseMicrosoftIfExistsExternalDeclaration(); return DeclGroupPtrTy(); - + + case tok::annot_module_include: + case tok::annot_module_begin: + case tok::annot_module_end: + Actions.checkModuleImportContext(reinterpret_cast<Module *>( + Tok.getAnnotationValue()), Tok.getLocation(), Actions.CurContext); + ConsumeToken(); + return DeclGroupPtrTy(); + default: dont_know: // We can't tell whether this is a function-definition or declaration yet. Index: lib/Parse/ParseStmt.cpp =================================================================== --- lib/Parse/ParseStmt.cpp +++ lib/Parse/ParseStmt.cpp @@ -944,11 +944,19 @@ Stmts.push_back(R.get()); } - while (Tok.isNot(tok::r_brace) && !isEofOrEom()) { + while (Tok.isNot(tok::r_brace) && Tok.isNot(tok::eof)) { if (Tok.is(tok::annot_pragma_unused)) { HandlePragmaUnused(); continue; } + if (Tok.isOneOf(tok::annot_module_include, + tok::annot_module_begin, + tok::annot_module_end)) { + Actions.checkModuleImportContext(reinterpret_cast<Module *>( + Tok.getAnnotationValue()), Tok.getLocation(), Actions.CurContext); + ConsumeToken(); + continue; + } StmtResult R; if (Tok.isNot(tok::kw___extension__)) { Index: lib/Parse/ParseDeclCXX.cpp =================================================================== --- lib/Parse/ParseDeclCXX.cpp +++ lib/Parse/ParseDeclCXX.cpp @@ -210,7 +210,7 @@ ParsedAttributes &attrs, BalancedDelimiterTracker &Tracker) { if (index == Ident.size()) { - while (Tok.isNot(tok::r_brace) && !isEofOrEom()) { + while (Tok.isNot(tok::r_brace) && Tok.isNot(tok::eof)) { ParsedAttributesWithRange attrs(AttrFactory); MaybeParseCXX11Attributes(attrs); MaybeParseMicrosoftAttributes(attrs); @@ -2853,6 +2853,15 @@ return DeclGroupPtrTy(); } + if (Tok.isOneOf(tok::annot_module_include, + tok::annot_module_begin, + tok::annot_module_end)) { + Actions.checkModuleImportContext(reinterpret_cast<Module *>( + Tok.getAnnotationValue()), Tok.getLocation(), Actions.CurContext); + ConsumeToken(); + return DeclGroupPtrTy(); + } + // If we see a namespace here, a close brace was missing somewhere. if (Tok.is(tok::kw_namespace)) { DiagnoseUnexpectedNamespace(cast<NamedDecl>(TagDecl)); @@ -3063,7 +3072,7 @@ if (TagDecl) { // While we still have something to read, read the member-declarations. - while (Tok.isNot(tok::r_brace) && !isEofOrEom()) + while (Tok.isNot(tok::r_brace) && Tok.isNot(tok::eof)) // Each iteration of this loop reads one member-declaration. ParseCXXClassMemberDeclarationWithPragmas( CurAS, AccessAttrs, static_cast<DeclSpec::TST>(TagType), TagDecl); Index: include/clang/Sema/Sema.h =================================================================== --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -1778,6 +1778,11 @@ /// \brief The parser has left a submodule. void ActOnModuleEnd(SourceLocation DirectiveLoc, Module *Mod); + /// \brief Check if module import may be found in the specified context, + /// emit error if not. + void checkModuleImportContext(Module *M, SourceLocation ImportLoc, + DeclContext *DC); + /// \brief Create an implicit import of the given module at the given /// source location, for error recovery, if possible. ///
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits