[modules] Simplify and generalize the existing rule for finding hidden

declarations in redeclaration lookup. A declaration is now visible to
lookup if:

 * It is visible (not in a module, or in an imported module), or
 * We're doing redeclaration lookup and it's externally-visible, or
 * We're doing typo correction and looking for unimported decls.

We now support multiple modules having different internal-linkage or no-linkage
definitions of the same name for all entities, not just for functions,
variables, and some typedefs. As previously, if multiple such entities are
visible, any attempt to use them will result in an ambiguity error.

This patch fixes the linkage calculation for a number of entities where we
previously didn't need to get it right (using-declarations, namespace aliases,
and so on).  It also classifies enumerators as always having no linkage, which
is a slight deviation from the C++ standard's definition, but not an observable
change outside modules (this change is being discussed on the -core reflector
currently).

This also removes the prior special case for tag lookup, which made some cases
of this work, but also led to bizarre, bogus "must use 'struct' to refer to type
'Foo' in this scope" diagnostics in C++.

llvm-svn: 252960

9 files changed, 67 insertions, 21 deletions

diff --git a/clang/test/Modules/Inputs/no-linkage/decls.h b/clang/test/Modules/Inputs/no-linkage/decls.h
new file mode 100644
index 00000000000..5c29044abf8
--- /dev/null
+++ b/clang/test/Modules/Inputs/no-linkage/decls.h
@@ -0,0 +1,6 @@
+namespace RealNS { int UsingDecl; }
+namespace NS = RealNS;
+typedef int Typedef;
+using AliasDecl = int;
+enum Enum { Enumerator };
+using RealNS::UsingDecl;
diff --git a/clang/test/Modules/Inputs/no-linkage/empty.h b/clang/test/Modules/Inputs/no-linkage/empty.h
new file mode 100644
index 00000000000..e69de29bb2d
--- /dev/null
+++ b/clang/test/Modules/Inputs/no-linkage/empty.h
diff --git a/clang/test/Modules/Inputs/no-linkage/module.modulemap b/clang/test/Modules/Inputs/no-linkage/module.modulemap
new file mode 100644
index 00000000000..3931b0fe8e7
--- /dev/null
+++ b/clang/test/Modules/Inputs/no-linkage/module.modulemap
@@ -0,0 +1 @@
+module M { module E { header "empty.h" } module D { header "decls.h" } }
diff --git a/clang/test/Modules/decldef.m b/clang/test/Modules/decldef.m
index 420c6774ecb..784743ff6e7 100644
--- a/clang/test/Modules/decldef.m
+++ b/clang/test/Modules/decldef.m
@@ -11,7 +11,13 @@ Def *def;
 #ifdef USE_EARLY
 A *a1; // expected-error{{declaration of 'A' must be imported from module 'decldef.Def' before it is required}}
 #endif
-B *b1; // expected-error{{must use 'struct' tag to refer to type 'B'}}
+B *b1;
+#ifdef USE_EARLY
+// expected-error@-2{{must use 'struct' tag to refer to type 'B'}}
+#else
+// expected-error@-4{{declaration of 'B' must be imported from module 'decldef.Decl' before it is required}}
+// expected-note@Inputs/decl.h:2 {{previous}}
+#endif
 @import decldef.Decl;
 
 A *a2;
diff --git a/clang/test/Modules/merge-enumerators.cpp b/clang/test/Modules/merge-enumerators.cpp
index 5267ac658b8..10e1914bd7b 100644
--- a/clang/test/Modules/merge-enumerators.cpp
+++ b/clang/test/Modules/merge-enumerators.cpp
@@ -16,11 +16,13 @@
 
 #ifdef MERGE_LATE
 namespace N {
-  // FIXME: Should we accept this and reject the usage below due to ambiguity instead?
-  enum { A } a; // expected-error {{redefinition of enumerator 'A'}}
-  // expected-note@a.h:1 {{here}} (from module B.b)
+  enum { A } a; // expected-note {{candidate}}
+  // expected-note@a.h:1 {{candidate}} (from module B.b)
 }
 #include "a.h"
 #endif
 
 N::E e = N::A;
+#ifdef MERGE_LATE
+// expected-error@-2 {{ambiguous}}
+#endif
diff --git a/clang/test/Modules/module-private.cpp b/clang/test/Modules/module-private.cpp
index 6e723c863ce..42ab185760b 100644
--- a/clang/test/Modules/module-private.cpp
+++ b/clang/test/Modules/module-private.cpp
@@ -12,11 +12,7 @@ void test() {
 }
 
 int test_broken() {
-  HiddenStruct hidden; // \
-  // expected-error{{must use 'struct' tag to refer to type 'HiddenStruct' in this scope}} \
-  // expected-error{{definition of 'HiddenStruct' must be imported}}
-  // expected-note@Inputs/module_private_left.h:3 {{previous definition is here}}
-
+  HiddenStruct hidden; // expected-error{{unknown type name 'HiddenStruct'}}
   Integer i; // expected-error{{unknown type name 'Integer'}}
 
   int *ip = 0;
diff --git a/clang/test/Modules/no-linkage.cpp b/clang/test/Modules/no-linkage.cpp
new file mode 100644
index 00000000000..508464e8dbe
--- /dev/null
+++ b/clang/test/Modules/no-linkage.cpp
@@ -0,0 +1,35 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fmodules-local-submodule-visibility -I%S/Inputs/no-linkage -fmodule-map-file=%S/Inputs/no-linkage/module.modulemap %s -verify
+
+#include "empty.h"
+
+namespace NS { int n; } // expected-note {{candidate}}
+struct Typedef { int n; }; // expected-note {{candidate}}
+int AliasDecl; // expected-note {{candidate}}
+enum AlsoAnEnum { Enumerator }; // expected-note {{candidate}}
+int UsingDecl; // expected-note {{candidate}}
+
+// expected-note@decls.h:2 {{candidate}}
+// expected-note@decls.h:3 {{candidate}}
+// expected-note@decls.h:4 {{candidate}}
+// expected-note@decls.h:5 {{candidate}}
+// expected-note@decls.h:6 {{candidate}}
+
+void use(int);
+void use_things() {
+  use(Typedef().n);
+  use(NS::n);
+  use(AliasDecl);
+  use(Enumerator);
+  use(UsingDecl);
+}
+
+#include "decls.h"
+
+void use_things_again() {
+  use(Typedef().n); // expected-error {{ambiguous}}
+  use(NS::n); // expected-error {{ambiguous}} expected-error{{'NS' is not a class, namespace, or enumeration}}
+  use(AliasDecl); // expected-error {{ambiguous}}
+  use(Enumerator); // expected-error {{ambiguous}}
+  use(UsingDecl); // expected-error {{ambiguous}}
+}
diff --git a/clang/test/Modules/submodule-visibility-cycles.cpp b/clang/test/Modules/submodule-visibility-cycles.cpp
index f26f6f21eea..a01fe562b14 100644
--- a/clang/test/Modules/submodule-visibility-cycles.cpp
+++ b/clang/test/Modules/submodule-visibility-cycles.cpp
@@ -3,7 +3,7 @@
 
 #include "cycle1.h"
 C1 c1;
-C2 c2; // expected-error {{must be imported}} expected-error {{}}
+C2 c2; // expected-error {{must be imported}}
 // expected-note@cycle2.h:6 {{here}}
 
 #include "cycle2.h"
diff --git a/clang/test/Modules/submodules-merge-defs.cpp b/clang/test/Modules/submodules-merge-defs.cpp
index bb8e69367ca..361f05b553e 100644
--- a/clang/test/Modules/submodules-merge-defs.cpp
+++ b/clang/test/Modules/submodules-merge-defs.cpp
@@ -12,7 +12,7 @@
 #include "indirect.h"
 #endif
 
-A pre_a; // expected-error {{must use 'struct'}}
+A pre_a;
 #ifdef IMPORT_USE_2
 // expected-error-re@-2 {{must be imported from one of {{.*}}stuff.use{{.*}}stuff.use-2}}
 #elif EARLY_INDIRECT_INCLUDE
@@ -21,29 +21,28 @@ A pre_a; // expected-error {{must use 'struct'}}
 // expected-error@-6 {{must be imported from module 'stuff.use'}}
 #endif
 // expected-note@defs.h:1 +{{here}}
+extern class A pre_a2;
+int pre_use_a = use_a(pre_a2); // expected-error {{'A' must be imported}} expected-error {{'use_a' must be imported}}
 // expected-note@defs.h:2 +{{here}}
-int pre_use_a = use_a(pre_a); // expected-error {{'A' must be imported}} expected-error {{'use_a' must be imported}}
 
 B::Inner2 pre_bi; // expected-error +{{must be imported}}
 // expected-note@defs.h:4 +{{here}}
 // expected-note@defs.h:17 +{{here}}
-void pre_bfi(B b) { // expected-error {{must use 'class'}} expected-error +{{must be imported}}
-  b.f<int>(); // expected-error +{{must be imported}} expected-error +{{}}
-  // expected-note@defs.h:19 +{{here}}
+void pre_bfi(B b) { // expected-error +{{must be imported}}
+  b.f<int>(); // expected-error +{{}}
 }
 
 C_Base<1> pre_cb1; // expected-error +{{must be imported}}
 // expected-note@defs.h:23 +{{here}}
-C1 pre_c1; // expected-error +{{must be imported}} expected-error {{must use 'struct'}}
+C1 pre_c1; // expected-error +{{must be imported}}
 // expected-note@defs.h:25 +{{here}}
-C2 pre_c2; // expected-error +{{must be imported}} expected-error {{must use 'struct'}}
+C2 pre_c2; // expected-error +{{must be imported}}
 // expected-note@defs.h:26 +{{here}}
 
 D::X pre_dx; // expected-error +{{must be imported}}
 // expected-note@defs.h:28 +{{here}}
 // expected-note@defs.h:29 +{{here}}
-// FIXME: We should warn that use_dx is being used without being imported.
-int pre_use_dx = use_dx(pre_dx);
+int pre_use_dx = use_dx(pre_dx); // ignored; pre_dx is invalid
 
 int pre_e = E(0); // expected-error {{must be imported}}
 // expected-note@defs.h:32 +{{here}}
@@ -69,8 +68,9 @@ J<> pre_j; // expected-error {{declaration of 'J' must be imported}}
 #endif
 // expected-note@defs.h:58 +{{here}}
 
-ScopedEnum pre_scopedenum; // expected-error {{must be imported}} expected-error {{must use 'enum'}}
-// expected-note@defs.h:106 {{here}}
+ScopedEnum pre_scopedenum; // expected-error {{must be imported}}
+// expected-note@defs.h:105 0-1{{here}}
+// expected-note@defs.h:106 0-1{{here}}
 enum ScopedEnum : int;
 ScopedEnum pre_scopedenum_declared; // ok