[PATCH] D32828: [Modules] Fix conservative assertion for import diagnostics

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno updated this revision to Diff 100016.
bruno added a comment.

Updated the patch after Richard's comments. Removed the assertion but bail out early if the module is already visible.


https://reviews.llvm.org/D32828

Files:
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaLookup.cpp
  test/Modules/Inputs/diagnose-missing-import/a.h
  test/Modules/Inputs/diagnose-missing-import/module.modulemap
  test/Modules/diagnose-missing-import.m


Index: test/Modules/diagnose-missing-import.m
===================================================================
--- /dev/null
+++ test/Modules/diagnose-missing-import.m
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I%S/Inputs/diagnose-missing-import \
+// RUN:   -Werror=implicit-function-declaration -fsyntax-only \
+// RUN:   -fimplicit-module-maps -verify %s
+ at import NCI;
+
+void foo() {
+  XYZLogEvent(xyzRiskyCloseOpenParam, xyzRiskyCloseOpenParam); // expected-error {{implicit declaration of function 'XYZLogEvent'}} expected-error {{declaration of 'XYZLogEvent' must be imported}} expected-error {{declaration of 'xyzRiskyCloseOpenParam' must be imported from module 'NCI.A'}} expected-error {{declaration of 'xyzRiskyCloseOpenParam' must be imported from module 'NCI.A'}}
+}
+
+// expected-note at Inputs/diagnose-missing-import/a.h:5 {{previous declaration is here}}
+// expected-note at Inputs/diagnose-missing-import/a.h:5 {{previous declaration is here}}
+// expected-note at Inputs/diagnose-missing-import/a.h:6 {{previous declaration is here}}
+
Index: test/Modules/Inputs/diagnose-missing-import/module.modulemap
===================================================================
--- /dev/null
+++ test/Modules/Inputs/diagnose-missing-import/module.modulemap
@@ -0,0 +1,3 @@
+module NCI {
+  explicit module A { header "a.h" }
+}
Index: test/Modules/Inputs/diagnose-missing-import/a.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/diagnose-missing-import/a.h
@@ -0,0 +1,8 @@
+#ifndef A_h
+#define A_h
+
+ at class NSString;
+static NSString * const xyzRiskyCloseOpenParam = @"riskyCloseParam";
+static inline void XYZLogEvent(NSString* eventName, NSString* params);
+
+#endif
Index: lib/Sema/SemaLookup.cpp
===================================================================
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -4929,8 +4929,6 @@
 
 void Sema::diagnoseMissingImport(SourceLocation Loc, NamedDecl *Decl,
                                  MissingImportKind MIK, bool Recover) {
-  assert(!isVisible(Decl) && "missing import for non-hidden decl?");
-
   // Suggest importing a module providing the definition of this entity, if
   // possible.
   NamedDecl *Def = getDefinitionToImport(Decl);
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -16097,7 +16097,8 @@
 void Sema::createImplicitModuleImportForErrorRecovery(SourceLocation Loc,
                                                       Module *Mod) {
   // Bail if we're not allowed to implicitly import a module here.
-  if (isSFINAEContext() || !getLangOpts().ModulesErrorRecovery)
+  if (isSFINAEContext() || !getLangOpts().ModulesErrorRecovery ||
+      VisibleModules.isVisible(Mod))
     return;
 
   // Create the implicit import declaration.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D32828.100016.patch
Type: text/x-patch
Size: 2943 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170523/4aafff8d/attachment-0001.bin>