r248085 - [Modules] More descriptive diagnostics for misplaced import directive

Serge Pavlov via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 18 22:32:57 PDT 2015


Author: sepavloff
Date: Sat Sep 19 00:32:57 2015
New Revision: 248085

URL: http://llvm.org/viewvc/llvm-project?rev=248085&view=rev
Log:
[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

Added:
    cfe/trunk/test/Modules/Inputs/misplaced/
    cfe/trunk/test/Modules/Inputs/misplaced/misplaced-a.h
    cfe/trunk/test/Modules/Inputs/misplaced/misplaced-b.h
    cfe/trunk/test/Modules/Inputs/misplaced/misplaced.modulemap
    cfe/trunk/test/Modules/misplaced-1.cpp
    cfe/trunk/test/Modules/misplaced-2.cpp
    cfe/trunk/test/Modules/misplaced-3.cpp
    cfe/trunk/test/Modules/misplaced-4.cpp
    cfe/trunk/test/Modules/misplaced-5.c
Modified:
    cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/include/clang/Parse/Parser.h
    cfe/trunk/include/clang/Sema/Sema.h
    cfe/trunk/lib/Parse/ParseDecl.cpp
    cfe/trunk/lib/Parse/ParseDeclCXX.cpp
    cfe/trunk/lib/Parse/ParseStmt.cpp
    cfe/trunk/lib/Parse/Parser.cpp
    cfe/trunk/lib/Sema/SemaDecl.cpp
    cfe/trunk/test/Modules/auto-module-import.m
    cfe/trunk/test/Modules/extern_c.cpp
    cfe/trunk/test/Modules/malformed.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=248085&r1=248084&r2=248085&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Sat Sep 19 00:32:57 2015
@@ -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 {

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=248085&r1=248084&r2=248085&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sat Sep 19 00:32:57 2015
@@ -7776,6 +7776,8 @@ def note_module_import_in_extern_c : Not
   "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'">;

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=248085&r1=248084&r2=248085&view=diff
==============================================================================
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Sat Sep 19 00:32:57 2015
@@ -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]

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=248085&r1=248084&r2=248085&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Sat Sep 19 00:32:57 2015
@@ -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.
   ///

Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=248085&r1=248084&r2=248085&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Sat Sep 19 00:32:57 2015
@@ -3589,7 +3589,8 @@ void Parser::ParseStructUnionBody(Source
   SmallVector<Decl *, 32> 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.

Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=248085&r1=248084&r2=248085&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Sat Sep 19 00:32:57 2015
@@ -210,7 +210,8 @@ void Parser::ParseInnerNamespace(std::ve
                                  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
 
   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<DeclSpec::TST>(TagType), TagDecl);
-
+    }
     T.consumeClose();
   } else {
     SkipUntil(tok::r_brace);

Modified: cfe/trunk/lib/Parse/ParseStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=248085&r1=248084&r2=248085&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
+++ cfe/trunk/lib/Parse/ParseStmt.cpp Sat Sep 19 00:32:57 2015
@@ -944,7 +944,8 @@ StmtResult Parser::ParseCompoundStatemen
       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;

Modified: cfe/trunk/lib/Parse/Parser.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/Parser.cpp?rev=248085&r1=248084&r2=248085&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/Parser.cpp (original)
+++ cfe/trunk/lib/Parse/Parser.cpp Sat Sep 19 00:32:57 2015
@@ -2018,6 +2018,37 @@ Parser::DeclGroupPtrTy Parser::ParseModu
   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<Module *>(
+        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<Module *>(
+                                 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::expectAnd
 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

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=248085&r1=248084&r2=248085&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Sat Sep 19 00:32:57 2015
@@ -14368,14 +14368,17 @@ static void checkModuleImportContext(Sem
   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(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;
+           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) {

Added: cfe/trunk/test/Modules/Inputs/misplaced/misplaced-a.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/misplaced/misplaced-a.h?rev=248085&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/misplaced/misplaced-a.h (added)
+++ cfe/trunk/test/Modules/Inputs/misplaced/misplaced-a.h Sat Sep 19 00:32:57 2015
@@ -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'}}
+  }
+}

Added: cfe/trunk/test/Modules/Inputs/misplaced/misplaced-b.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/misplaced/misplaced-b.h?rev=248085&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/misplaced/misplaced-b.h (added)
+++ cfe/trunk/test/Modules/Inputs/misplaced/misplaced-b.h Sat Sep 19 00:32:57 2015
@@ -0,0 +1 @@
+int a;
\ No newline at end of file

Added: cfe/trunk/test/Modules/Inputs/misplaced/misplaced.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/misplaced/misplaced.modulemap?rev=248085&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/misplaced/misplaced.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/misplaced/misplaced.modulemap Sat Sep 19 00:32:57 2015
@@ -0,0 +1,8 @@
+module Misplaced {
+  module Sub_A {
+    header "misplaced-a.h"
+  }
+  module Sub_B {
+    header "misplaced-b.h"
+  }
+}

Modified: cfe/trunk/test/Modules/auto-module-import.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/auto-module-import.m?rev=248085&r1=248084&r2=248085&view=diff
==============================================================================
--- cfe/trunk/test/Modules/auto-module-import.m (original)
+++ cfe/trunk/test/Modules/auto-module-import.m Sat Sep 19 00:32:57 2015
@@ -83,6 +83,7 @@ int getNotInModule() {
   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'}}
+}

Modified: cfe/trunk/test/Modules/extern_c.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/extern_c.cpp?rev=248085&r1=248084&r2=248085&view=diff
==============================================================================
--- cfe/trunk/test/Modules/extern_c.cpp (original)
+++ cfe/trunk/test/Modules/extern_c.cpp Sat Sep 19 00:32:57 2015
@@ -68,7 +68,7 @@ namespace N {
   extern "C" {
 #endif
     int f;
-#if !defined(CXX_HEADER)
+#if !defined(CXX_HEADER) && !defined(NAMESPACE)
     // expected-error at -2 {{redefinition of 'f' as different kind of symbol}}
     // expected-note at 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

Modified: cfe/trunk/test/Modules/malformed.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/malformed.cpp?rev=248085&r1=248084&r2=248085&view=diff
==============================================================================
--- cfe/trunk/test/Modules/malformed.cpp (original)
+++ cfe/trunk/test/Modules/malformed.cpp Sat Sep 19 00:32:57 2015
@@ -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<int>(); }
 // Test that we use relative paths to name files within an imported module.

Added: cfe/trunk/test/Modules/misplaced-1.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/misplaced-1.cpp?rev=248085&view=auto
==============================================================================
--- cfe/trunk/test/Modules/misplaced-1.cpp (added)
+++ cfe/trunk/test/Modules/misplaced-1.cpp Sat Sep 19 00:32:57 2015
@@ -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'}}
+}

Added: cfe/trunk/test/Modules/misplaced-2.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/misplaced-2.cpp?rev=248085&view=auto
==============================================================================
--- cfe/trunk/test/Modules/misplaced-2.cpp (added)
+++ cfe/trunk/test/Modules/misplaced-2.cpp Sat Sep 19 00:32:57 2015
@@ -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'}}
+}

Added: cfe/trunk/test/Modules/misplaced-3.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/misplaced-3.cpp?rev=248085&view=auto
==============================================================================
--- cfe/trunk/test/Modules/misplaced-3.cpp (added)
+++ cfe/trunk/test/Modules/misplaced-3.cpp Sat Sep 19 00:32:57 2015
@@ -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'}}
+}

Added: cfe/trunk/test/Modules/misplaced-4.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/misplaced-4.cpp?rev=248085&view=auto
==============================================================================
--- cfe/trunk/test/Modules/misplaced-4.cpp (added)
+++ cfe/trunk/test/Modules/misplaced-4.cpp Sat Sep 19 00:32:57 2015
@@ -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

Added: cfe/trunk/test/Modules/misplaced-5.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/misplaced-5.c?rev=248085&view=auto
==============================================================================
--- cfe/trunk/test/Modules/misplaced-5.c (added)
+++ cfe/trunk/test/Modules/misplaced-5.c Sat Sep 19 00:32:57 2015
@@ -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'}}
+}




More information about the cfe-commits mailing list