From: Serge Pavlov Date: Sat, 19 Sep 2015 05:32:57 +0000 (+0000) Subject: [Modules] More descriptive diagnostics for misplaced import directive X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=5bd7ce1215838ea98fcf8d5209266ea146f3c555;p=clang [Modules] More descriptive diagnostics for misplaced import directive 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. Differential Revision: http://reviews.llvm.org/D11844 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@248085 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticParseKinds.td b/include/clang/Basic/DiagnosticParseKinds.td index 3f86681303..291eca2d71 100644 --- a/include/clang/Basic/DiagnosticParseKinds.td +++ b/include/clang/Basic/DiagnosticParseKinds.td @@ -1013,6 +1013,7 @@ def err_module_expected_ident : Error< "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 end of module">; } let CategoryName = "Generics Issue" in { diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 7d22327961..3432b6c268 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -7776,6 +7776,8 @@ def note_module_import_in_extern_c : Note< "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'">; diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h index 272b504d1e..54bac15489 100644 --- a/include/clang/Parse/Parser.h +++ b/include/clang/Parse/Parser.h @@ -2555,6 +2555,14 @@ private: //===--------------------------------------------------------------------===// // Modules DeclGroupPtrTy ParseModuleImport(SourceLocation AtLoc); + bool parseMisplacedModuleImport(); + bool tryParseMisplacedModuleImport() { + tok::TokenKind Kind = Tok.getKind(); + if (Kind == tok::annot_module_begin || Kind == tok::annot_module_end || + Kind == tok::annot_module_include) + return parseMisplacedModuleImport(); + return false; + } //===--------------------------------------------------------------------===// // C++11/G++: Type Traits [Type-Traits.html in the GCC manual] diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index a7b74172a7..b55db2e8cb 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -1799,6 +1799,10 @@ public: /// \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. /// diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index 80d25426f6..f5983c0295 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -3589,7 +3589,8 @@ void Parser::ParseStructUnionBody(SourceLocation RecordLoc, SmallVector FieldDecls; // While we still have something to read, read the declarations in the struct. - while (Tok.isNot(tok::r_brace) && !isEofOrEom()) { + while (Tok.isNot(tok::r_brace) && Tok.isNot(tok::eof) && + !tryParseMisplacedModuleImport()) { // Each iteration of this loop reads one struct-declaration. // Check for extraneous top-level semicolon. diff --git a/lib/Parse/ParseDeclCXX.cpp b/lib/Parse/ParseDeclCXX.cpp index 1a0d262279..5e59d5dba2 100644 --- a/lib/Parse/ParseDeclCXX.cpp +++ b/lib/Parse/ParseDeclCXX.cpp @@ -210,7 +210,8 @@ void Parser::ParseInnerNamespace(std::vector &IdentLoc, 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) && + !tryParseMisplacedModuleImport()) { ParsedAttributesWithRange attrs(AttrFactory); MaybeParseCXX11Attributes(attrs); MaybeParseMicrosoftAttributes(attrs); @@ -3063,11 +3064,12 @@ void Parser::ParseCXXMemberSpecification(SourceLocation RecordLoc, 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) && + !tryParseMisplacedModuleImport()) { // Each iteration of this loop reads one member-declaration. ParseCXXClassMemberDeclarationWithPragmas( CurAS, AccessAttrs, static_cast(TagType), TagDecl); - + } T.consumeClose(); } else { SkipUntil(tok::r_brace); diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp index b658cef234..312c5e6f51 100644 --- a/lib/Parse/ParseStmt.cpp +++ b/lib/Parse/ParseStmt.cpp @@ -944,7 +944,8 @@ StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) { Stmts.push_back(R.get()); } - while (Tok.isNot(tok::r_brace) && !isEofOrEom()) { + while (Tok.isNot(tok::r_brace) && Tok.isNot(tok::eof) && + !tryParseMisplacedModuleImport()) { if (Tok.is(tok::annot_pragma_unused)) { HandlePragmaUnused(); continue; diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp index 35e853f64f..5b0704658a 100644 --- a/lib/Parse/Parser.cpp +++ b/lib/Parse/Parser.cpp @@ -2018,6 +2018,37 @@ Parser::DeclGroupPtrTy Parser::ParseModuleImport(SourceLocation AtLoc) { return Actions.ConvertDeclToDeclGroup(Import.get()); } +/// \brief Try recover parser when module annotation appears where it must not +/// be found. +/// \returns false if the recover was successful and parsing may be continued, or +/// true if parser must bail out to top level and handle the token there. +bool Parser::parseMisplacedModuleImport() { + while (true) { + switch (Tok.getKind()) { + case tok::annot_module_end: + // Inform caller that recovery failed, the error must be handled at upper + // level. + return true; + case tok::annot_module_begin: + Actions.diagnoseMisplacedModuleImport(reinterpret_cast( + Tok.getAnnotationValue()), Tok.getLocation()); + return true; + 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 false; + } + } + return false; +} + bool BalancedDelimiterTracker::diagnoseOverflow() { P.Diag(P.Tok, diag::err_bracket_depth_exceeded) << P.getLangOpts().BracketDepth; @@ -2045,7 +2076,10 @@ bool BalancedDelimiterTracker::expectAndConsume(unsigned DiagID, 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 diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index 701d8c7290..3101fda933 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -14368,14 +14368,17 @@ static void checkModuleImportContext(Sema &S, Module *M, 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(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; + diag::note_module_import_not_at_top_level) << DC; } } +void Sema::diagnoseMisplacedModuleImport(Module *M, SourceLocation ImportLoc) { + return checkModuleImportContext(*this, M, ImportLoc, CurContext); +} + DeclResult Sema::ActOnModuleImport(SourceLocation AtLoc, SourceLocation ImportLoc, ModuleIdPath Path) { diff --git a/test/Modules/Inputs/misplaced/misplaced-a.h b/test/Modules/Inputs/misplaced/misplaced-a.h new file mode 100644 index 0000000000..f50e5cefc3 --- /dev/null +++ b/test/Modules/Inputs/misplaced/misplaced-a.h @@ -0,0 +1,5 @@ +namespace A { + namespace B { // expected-note{{namespace 'A::B' begins here}} + #include "misplaced-b.h" // expected-error{{import of module 'Misplaced.Sub_B' appears within namespace 'A::B'}} + } +} diff --git a/test/Modules/Inputs/misplaced/misplaced-b.h b/test/Modules/Inputs/misplaced/misplaced-b.h new file mode 100644 index 0000000000..68dd955ef8 --- /dev/null +++ b/test/Modules/Inputs/misplaced/misplaced-b.h @@ -0,0 +1 @@ +int a; \ No newline at end of file diff --git a/test/Modules/Inputs/misplaced/misplaced.modulemap b/test/Modules/Inputs/misplaced/misplaced.modulemap new file mode 100644 index 0000000000..50aa7a47e5 --- /dev/null +++ b/test/Modules/Inputs/misplaced/misplaced.modulemap @@ -0,0 +1,8 @@ +module Misplaced { + module Sub_A { + header "misplaced-a.h" + } + module Sub_B { + header "misplaced-b.h" + } +} diff --git a/test/Modules/auto-module-import.m b/test/Modules/auto-module-import.m index de2c355a42..76441fcfed 100644 --- a/test/Modules/auto-module-import.m +++ b/test/Modules/auto-module-import.m @@ -83,6 +83,7 @@ int getNotInModule() { 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'}} +} diff --git a/test/Modules/extern_c.cpp b/test/Modules/extern_c.cpp index f505c11f7c..c9b4b8435f 100644 --- a/test/Modules/extern_c.cpp +++ b/test/Modules/extern_c.cpp @@ -68,7 +68,7 @@ namespace N { extern "C" { #endif int f; -#if !defined(CXX_HEADER) +#if !defined(CXX_HEADER) && !defined(NAMESPACE) // expected-error@-2 {{redefinition of 'f' as different kind of symbol}} // expected-note@c-header.h:1 {{previous}} #endif @@ -78,4 +78,6 @@ namespace N { } #endif +#if !defined(NAMESPACE) suppress_expected_no_diagnostics_error error_here; // expected-error {{}} +#endif diff --git a/test/Modules/malformed.cpp b/test/Modules/malformed.cpp index 7d5bcc8c9d..040361c9ee 100644 --- a/test/Modules/malformed.cpp +++ b/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 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. diff --git a/test/Modules/misplaced-1.cpp b/test/Modules/misplaced-1.cpp new file mode 100644 index 0000000000..d67ad194a9 --- /dev/null +++ b/test/Modules/misplaced-1.cpp @@ -0,0 +1,6 @@ +// 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'}} +} diff --git a/test/Modules/misplaced-2.cpp b/test/Modules/misplaced-2.cpp new file mode 100644 index 0000000000..da46099386 --- /dev/null +++ b/test/Modules/misplaced-2.cpp @@ -0,0 +1,6 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs %s -verify + +void func1() { // expected-note{{function 'func1' begins here}} +#include "dummy.h" // expected-error{{import of module 'dummy' appears within function 'func1'}} +} diff --git a/test/Modules/misplaced-3.cpp b/test/Modules/misplaced-3.cpp new file mode 100644 index 0000000000..6d18e13ecb --- /dev/null +++ b/test/Modules/misplaced-3.cpp @@ -0,0 +1,6 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs %s -verify + +class C1 { // expected-note{{'C1' begins here}} +#include "dummy.h" // expected-error{{import of module 'dummy' appears within 'C1'}} +} diff --git a/test/Modules/misplaced-4.cpp b/test/Modules/misplaced-4.cpp new file mode 100644 index 0000000000..8f795d5c50 --- /dev/null +++ b/test/Modules/misplaced-4.cpp @@ -0,0 +1,2 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fmodules -emit-module -fmodule-name=Misplaced -fmodules-cache-path=%t -x c++ -I %S/Inputs %S/Inputs/misplaced/misplaced.modulemap -verify diff --git a/test/Modules/misplaced-5.c b/test/Modules/misplaced-5.c new file mode 100644 index 0000000000..ae4b3f5d8b --- /dev/null +++ b/test/Modules/misplaced-5.c @@ -0,0 +1,6 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs %s -verify + +struct S1 { // expected-note{{'struct S1' begins here}} +#include "dummy.h" // expected-error{{import of module 'dummy' appears within 'struct S1'}} +}