Author: bruno
Date: Tue May 23 18:53:17 2017
New Revision: 303705

URL: http://llvm.org/viewvc/llvm-project?rev=303705&view=rev
Log:
[Modules] Fix overly conservative assertion for import diagnostic

We currenltly assert when want to diagnose a missing import and the decl
in question is already visible. It turns out that the decl in question
might be visible because another decl from the same module actually made
the module visible in a previous error diagnostic.

Remove the assertion and avoid re-exporting the module if it's already
visible.

rdar://problem/27975402

Differential Revision: https://reviews.llvm.org/D32828

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

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=303705&r1=303704&r2=303705&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue May 23 18:53:17 2017
@@ -16097,7 +16097,8 @@ void Sema::ActOnModuleEnd(SourceLocation
 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.

Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=303705&r1=303704&r2=303705&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Tue May 23 18:53:17 2017
@@ -4933,8 +4933,6 @@ static NamedDecl *getDefinitionToImport(
 
 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);

Added: cfe/trunk/test/Modules/Inputs/diagnose-missing-import/a.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/diagnose-missing-import/a.h?rev=303705&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/diagnose-missing-import/a.h (added)
+++ cfe/trunk/test/Modules/Inputs/diagnose-missing-import/a.h Tue May 23 
18:53:17 2017
@@ -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

Added: cfe/trunk/test/Modules/Inputs/diagnose-missing-import/module.modulemap
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/diagnose-missing-import/module.modulemap?rev=303705&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/diagnose-missing-import/module.modulemap 
(added)
+++ cfe/trunk/test/Modules/Inputs/diagnose-missing-import/module.modulemap Tue 
May 23 18:53:17 2017
@@ -0,0 +1,3 @@
+module NCI {
+  explicit module A { header "a.h" }
+}

Added: cfe/trunk/test/Modules/diagnose-missing-import.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/diagnose-missing-import.m?rev=303705&view=auto
==============================================================================
--- cfe/trunk/test/Modules/diagnose-missing-import.m (added)
+++ cfe/trunk/test/Modules/diagnose-missing-import.m Tue May 23 18:53:17 2017
@@ -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}}
+


_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to