sepavloff updated this revision to Diff 34673.
sepavloff added a comment.
Updated diagnostics according to Sean's notes ('textual module' -> 'textual
header')
http://reviews.llvm.org/D11844
Files:
include/clang/Basic/DiagnosticParseKinds.td
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Parse/Parser.h
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/extern_c.cpp
test/Modules/malformed.cpp
test/Modules/misplaced.cpp
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}}
+}
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<int>(); }
// Test that we use relative paths to name files within an imported module.
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/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 <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'}} \
+ expected-note {{consider marking the header as textual}}
+}
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<LinkageSpecDecl>(DC))
DC = DC->getParent();
if (!isa<TranslationUnitDecl>(DC)) {
- S.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)
- << DC;
+ if (DC->isNamespace()) {
+ S.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) << 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<Decl>(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: 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<Module *>(
+ 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<Module *>(
+ 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/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/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<DeclSpec::TST>(TagType), TagDecl);
-
+ }
T.consumeClose();
} else {
SkipUntil(tok::r_brace);
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: 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/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/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 {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
