bruno created this revision.
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.
Make the assertion more flexible and avoid re-exporting the module if it's
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
@@ -4960,16 +4960,16 @@
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);
if (!Def)
Def = Decl;
Module *Owner = getOwningModule(Decl);
assert(Owner && "definition of hidden declaration is not in a module");
+ assert((!isVisible(Decl) || VisibleModules.isVisible(Owner)) &&
+ "missing import for non-hidden decl?");
llvm::SmallVector<Module*, 8> OwningModules;
OwningModules.push_back(Owner);
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -15925,7 +15925,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
@@ -4960,16 +4960,16 @@
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);
if (!Def)
Def = Decl;
Module *Owner = getOwningModule(Decl);
assert(Owner && "definition of hidden declaration is not in a module");
+ assert((!isVisible(Decl) || VisibleModules.isVisible(Owner)) &&
+ "missing import for non-hidden decl?");
llvm::SmallVector<Module*, 8> OwningModules;
OwningModules.push_back(Owner);
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -15925,7 +15925,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
