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
+@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@Inputs/diagnose-missing-import/a.h:5 {{previous declaration 
is here}}
+// expected-note@Inputs/diagnose-missing-import/a.h:5 {{previous declaration 
is here}}
+// expected-note@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
+
+@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.


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
+@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@Inputs/diagnose-missing-import/a.h:5 {{previous declaration is here}}
+// expected-note@Inputs/diagnose-missing-import/a.h:5 {{previous declaration is here}}
+// expected-note@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
+
+@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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D32828: [Modul... Bruno Cardoso Lopes via Phabricator via cfe-commits

Reply via email to