iains updated this revision to Diff 511327.
iains added a comment.

rebased, addressed review comments (patch still needs refactoring)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145965/new/

https://reviews.llvm.org/D145965

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Lookup.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaLookup.cpp
  clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp
  clang/test/CXX/module/basic/basic.def.odr/p4.cppm
  clang/test/CXX/module/basic/basic.link/p2.cppm
  clang/test/CXX/module/module.import/p2.cpp
  clang/test/CXX/module/module.interface/p2.cpp
  clang/test/CXX/module/module.interface/p7.cpp
  clang/test/CXX/module/module.reach/ex1.cpp
  clang/test/CXX/module/module.reach/p2.cpp
  clang/test/CXX/module/module.reach/p5.cpp
  clang/test/Modules/Reachability-template-default-arg.cpp
  clang/test/Modules/cxx20-10-1-ex2.cpp
  clang/test/Modules/deduction-guide3.cppm
  clang/test/Modules/diagnose-missing-import.m

Index: clang/test/Modules/diagnose-missing-import.m
===================================================================
--- clang/test/Modules/diagnose-missing-import.m
+++ clang/test/Modules/diagnose-missing-import.m
@@ -6,9 +6,9 @@
 
 void foo(void) {
   XYZLogEvent(xyzRiskyCloseOpenParam, xyzRiskyCloseOpenParam); // expected-error {{call to undeclared function 'XYZLogEvent'; ISO C99 and later do not support implicit function declarations}} \
-                                                                  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-error {{declaration of 'XYZLogEvent' is internal to 'NCI.A'}} \
+                                                                  expected-error {{declaration of 'xyzRiskyCloseOpenParam' is internal to 'NCI.A'}} \
+                                                                  expected-error {{declaration of 'xyzRiskyCloseOpenParam' is internal to 'NCI.A'}}
 }
 
 // expected-note@Inputs/diagnose-missing-import/a.h:5 {{declaration here is not visible}}
Index: clang/test/Modules/deduction-guide3.cppm
===================================================================
--- clang/test/Modules/deduction-guide3.cppm
+++ clang/test/Modules/deduction-guide3.cppm
@@ -19,8 +19,8 @@
 //--- Use.cpp
 import Templ;
 void func() {
-    Templ t(5); // expected-error {{declaration of 'Templ' must be imported from module 'Templ' before it is required}}
+    Templ t(5); // expected-error {{declaration of 'Templ' is private to module 'Templ'}}
                 // expected-error@-1 {{unknown type name 'Templ'}}
-                // expected-n...@templ.cppm:3 {{declaration here is not visible}}
+                // expected-n...@templ.cppm:3 {{export the declaration to make it available}}
 }
 
Index: clang/test/Modules/cxx20-10-1-ex2.cpp
===================================================================
--- clang/test/Modules/cxx20-10-1-ex2.cpp
+++ clang/test/Modules/cxx20-10-1-ex2.cpp
@@ -53,8 +53,8 @@
 //--- std10-1-ex2-tu6.cpp
 import B;
 // error, n is module-local and this is not a module.
-int &c = n; // expected-error {{declaration of 'n' must be imported}}
-            // expected-note@* {{declaration here is not visible}}
+int &c = n; // expected-error {{declaration of 'n' is private to module 'B'}}
+            // expected-note@* {{export the declaration to make it available}}
 
 //--- std10-1-ex2-tu7.cpp
 // expected-no-diagnostics
Index: clang/test/Modules/Reachability-template-default-arg.cpp
===================================================================
--- clang/test/Modules/Reachability-template-default-arg.cpp
+++ clang/test/Modules/Reachability-template-default-arg.cpp
@@ -18,6 +18,6 @@
 import template_default_arg;
 void bar() {
   A<> a0;
-  A<t> a1; // expected-error {{declaration of 't' must be imported from module 'template_default_arg' before it is required}}
-           // expected-note@* {{declaration here is not visible}}
+  A<t> a1; // expected-error {{declaration of 't' is private to module 'template_default_arg'}}
+           // expected-note@* {{export the declaration to make it available}}
 }
Index: clang/test/CXX/module/module.reach/p5.cpp
===================================================================
--- clang/test/CXX/module/module.reach/p5.cpp
+++ clang/test/CXX/module/module.reach/p5.cpp
@@ -14,5 +14,5 @@
 export module B;
 import A;
 Y y; // OK, definition of X is reachable
-X x; // expected-error {{declaration of 'X' must be imported from module 'A' before it is required}}
-     // expected-note@* {{declaration here is not visible}}
+X x; // expected-error {{declaration of 'X' is private to module 'A'}}
+     // expected-note@* {{export the declaration to make it available}}
Index: clang/test/CXX/module/module.reach/p2.cpp
===================================================================
--- clang/test/CXX/module/module.reach/p2.cpp
+++ clang/test/CXX/module/module.reach/p2.cpp
@@ -17,6 +17,6 @@
 //--- UseStrict.cpp
 import M;
 void test() {
-  auto a = f(); // expected-error {{definition of 'A' must be imported from module 'M:impl' before it is required}} expected-error{{}}
-                // expected-note@* {{definition here is not reachable}} expected-note@* {{}}
+  auto a = f(); // expected-error 1+{{definition of 'A' is private to module 'M:impl'}}
+                // expected-note@* 1+{{export the definition to make it available}}
 }
Index: clang/test/CXX/module/module.reach/ex1.cpp
===================================================================
--- clang/test/CXX/module/module.reach/ex1.cpp
+++ clang/test/CXX/module/module.reach/ex1.cpp
@@ -37,10 +37,10 @@
 //--- X.cppm
 export module X;
 import M;
-B b3; // expected-error {{definition of 'B' must be imported from module 'M:B' before it is required}} expected-error {{}}
-      // expected-note@* {{definition here is not reachable}} expected-note@* {{}}
-// FIXME: We should emit an error for unreachable definition of B.
+B b3; // expected-error {{definition of 'B' is private to module 'M:B'}} expected-error {{}}
+      // expected-note@* {{export the definition to make it available}} expected-note@* {{}}
+
 void g() { f(); }
-void g1() { f(B()); } // expected-error 1+{{definition of 'B' must be imported from module 'M:B' before it is required}}
-                      // expected-note@* 1+{{definition here is not reachable}}
+void g1() { f(B()); } // expected-error 1+{{definition of 'B' is private to module 'M:B'}}
+                      // expected-note@* 1+{{export the definition to make it available}}
                       // expected-n...@m.cppm:5 {{passing argument to parameter 'b' here}}
Index: clang/test/CXX/module/module.interface/p7.cpp
===================================================================
--- clang/test/CXX/module/module.interface/p7.cpp
+++ clang/test/CXX/module/module.interface/p7.cpp
@@ -57,12 +57,12 @@
 void test2() {
   auto a = E1::e1;               // OK, namespace-scope name E1 is visible and e1 is reachable
   auto b = e1;                   // OK, namespace-scope name e1 is visible
-  auto c = E2::e2;               // expected-error {{declaration of 'E2' must be imported from module}}
-                                 // expected-note@* {{declaration here is not visible}}
+  auto c = E2::e2;               // expected-error {{declaration of 'E2' is private to module 'p7'}}
+                                 // expected-note@* {{export the declaration to make it available}}
   auto d = e2;                   // should be error, namespace-scope name e2 is not visible
   auto e = E2U::e2;              // OK, namespace-scope name E2U is visible and E2::e2 is reachable
-  auto f = E3::e3;               // expected-error {{declaration of 'E3' must be imported from module 'p7' before it is required}}
-                                 // expected-note@* {{declaration here is not visible}}
+  auto f = E3::e3;               // expected-error {{declaration of 'E3' is private to module 'p7'}}
+                                 // expected-note@* {{export the declaration to make it available}}
   auto g = e3;                   // should be error, namespace-scope name e3 is not visible
   auto h = decltype(func())::e3; // OK, namespace-scope name f is visible and E3::e3 is reachable
 }
Index: clang/test/CXX/module/module.interface/p2.cpp
===================================================================
--- clang/test/CXX/module/module.interface/p2.cpp
+++ clang/test/CXX/module/module.interface/p2.cpp
@@ -69,29 +69,29 @@
 
 void use() {
   // namespace A is implicitly exported by the export of A::g.
-  A::f(); // expected-error {{declaration of 'f' must be imported from module 'p2' before it is required}}
-          // expected-note@* {{declaration here is not visible}}
+  A::f(); // expected-error {{declaration of 'f' is private to module 'p2'}}
+          // expected-note@* {{export the declaration to make it available}}
   A::g();
-  A::h();                   // expected-error {{declaration of 'h' must be imported from module 'p2' before it is required}}
-                            // expected-note@* {{declaration here is not visible}}
+  A::h();                   // expected-error {{declaration of 'h' is private to module 'p2'}}
+                            // expected-note@* {{export the declaration to make it available}}
   using namespace A::inner; // expected-error {{declaration of 'inner' must be imported from module 'p2' before it is required}}
                             // expected-note@* {{declaration here is not visible}}
 
   // namespace B and B::inner are explicitly exported
   using namespace B;
   using namespace B::inner;
-  B::f(); // expected-error {{declaration of 'f' must be imported from module 'p2' before it is required}}
-          // expected-note@* {{declaration here is not visible}}
-  f();    // expected-error {{declaration of 'f' must be imported from module 'p2' before it is required}}
-          // expected-note@* {{declaration here is not visible}}
+  B::f(); // expected-error {{declaration of 'f' is private to module 'p2'}}
+          // expected-note@* {{export the declaration to make it available}}
+  f();    // expected-error {{declaration of 'f' is private to module 'p2'}}
+          // expected-note@* {{export the declaration to make it available}}
 
   // namespace C is not exported
   using namespace C; // expected-error {{declaration of 'C' must be imported from module 'p2' before it is required}}
                      // expected-note@* {{declaration here is not visible}}
 
   // namespace D is exported, but D::f is not
-  D::f(); // expected-error {{declaration of 'f' must be imported from module 'p2' before it is required}}
-          // expected-note@* {{declaration here is not visible}}
+  D::f(); // expected-error {{declaration of 'f' is private to module 'p2'}}
+          // expected-note@* {{export the declaration to make it available}}
 }
 
 int use_header() { return foo + bar::baz(); }
Index: clang/test/CXX/module/module.import/p2.cpp
===================================================================
--- clang/test/CXX/module/module.import/p2.cpp
+++ clang/test/CXX/module/module.import/p2.cpp
@@ -23,10 +23,10 @@
 //--- Use.cpp
 import M;
 void test() {
-  A a; // expected-error {{declaration of 'A' must be imported from module 'M:impl'}}
-       // expected-error@-1 {{definition of 'A' must be imported from module 'M:impl'}} expected-error@-1 {{}}
-       // expected-n...@impl.cppm:2 {{declaration here is not visible}}
-       // expected-n...@impl.cppm:2 {{definition here is not reachable}} expected-n...@impl.cppm:2 {{}}
+  A a; // expected-error {{declaration of 'A' is private to module 'M:impl'}}
+       // expected-n...@impl.cppm:2 {{export the declaration to make it available}}
+       // expected-error@-2 1+{{definition of 'A' is private to module 'M:impl'}}
+       // expected-n...@impl.cppm:2 1+{{export the definition to make it available}}
 }
 
 //--- UseInPartA.cppm
@@ -41,10 +41,10 @@
 export module B;
 import M;
 void test() {
-  A a; // expected-error {{declaration of 'A' must be imported from module 'M:impl'}}
-       // expected-error@-1 {{definition of 'A' must be imported from module 'M:impl'}} expected-error@-1 {{}}
-       // expected-n...@impl.cppm:2 {{declaration here is not visible}}
-       // expected-n...@impl.cppm:2 {{definition here is not reachable}} expected-n...@impl.cppm:2 {{}}
+  A a; // expected-error {{declaration of 'A' is private to module 'M:impl'}}
+       // expected-n...@impl.cppm:2 {{export the declaration to make it available}}
+       // expected-error@-2 1+{{definition of 'A' is private to module 'M:impl'}}
+       // expected-n...@impl.cppm:2 1+{{export the definition to make it available}}
 }
 
 //--- Private.cppm
Index: clang/test/CXX/module/basic/basic.link/p2.cppm
===================================================================
--- clang/test/CXX/module/basic/basic.link/p2.cppm
+++ clang/test/CXX/module/basic/basic.link/p2.cppm
@@ -39,22 +39,19 @@
 }
 
 //--- M.cpp
-
 module M;
 
 void use_from_module_impl() {
   external_linkage_fn();
   module_linkage_fn();
-  internal_linkage_fn(); // expected-error {{no matching function for call to 'internal_linkage_fn'}}
+  internal_linkage_fn(); // expected-error {{use of undeclared identifier 'internal_linkage_fn'; did you mean 'external_linkage_fn'}}
+                         // expected-n...@m.cppm:8 {{'external_linkage_fn' declared here}}
   (void)external_linkage_class{};
   (void)module_linkage_class{};
+  (void)internal_linkage_class;  // expected-error {{use of undeclared identifier 'internal_linkage_class'}}
   (void)external_linkage_var;
   (void)module_linkage_var;
-
-  // FIXME: Issue #61427 Internal-linkage declarations in the interface TU
-  // should not be not visible here.
-  (void)internal_linkage_class{};
-  (void)internal_linkage_var;
+  (void)internal_linkage_var; // expected-error {{use of undeclared identifier 'internal_linkage_var'}}
 }
 
 //--- user.cpp
@@ -62,13 +59,13 @@
 
 void use_from_module_impl() {
   external_linkage_fn();
-  module_linkage_fn();   // expected-error {{declaration of 'module_linkage_fn' must be imported}}
-  internal_linkage_fn(); // expected-error {{declaration of 'internal_linkage_fn' must be imported}}
+  module_linkage_fn();   // expected-error {{declaration of 'module_linkage_fn' is private to module 'M'}}
+                         // expected-n...@m.cppm:9 {{export the declaration to make it available}}
+  internal_linkage_fn(); // expected-error {{use of undeclared identifier 'internal_linkage_fn'; did you mean 'external_linkage_fn'}}
   (void)external_linkage_class{};
   (void)module_linkage_class{}; // expected-error {{undeclared identifier}} expected-error 0+{{}}
   (void)internal_linkage_class{}; // expected-error {{undeclared identifier}} expected-error 0+{{}}
-  // expected-n...@m.cppm:9 {{declaration here is not visible}}
-  // expected-n...@m.cppm:10 {{declaration here is not visible}}
+  // expected-n...@m.cppm:8 {{'external_linkage_fn' declared here}}
   (void)external_linkage_var;
   (void)module_linkage_var; // expected-error {{undeclared identifier}}
   (void)internal_linkage_var; // expected-error {{undeclared identifier}}
Index: clang/test/CXX/module/basic/basic.def.odr/p4.cppm
===================================================================
--- clang/test/CXX/module/basic/basic.def.odr/p4.cppm
+++ clang/test/CXX/module/basic/basic.def.odr/p4.cppm
@@ -128,7 +128,6 @@
 //
 // CHECK-DAG: @_ZW6Module25extern_var_module_linkage = external {{(dso_local )?}}global
 // CHECK-DAG: @_ZW6Module25inline_var_module_linkage = linkonce_odr {{(dso_local )?}}global
-// CHECK-DAG: @_ZL25static_var_module_linkage = internal {{(dso_local )?}}global i32 0,
 // CHECK-DAG: @_ZW6Module24const_var_module_linkage = available_externally {{(dso_local )?}}constant i32 3,
 
 module Module;
@@ -151,12 +150,9 @@
 
   (void)&extern_var_module_linkage;
   (void)&inline_var_module_linkage;
+  (void)&const_var_module_linkage;
 
-  // FIXME: Issue #61427 Internal-linkage declarations in the interface TU
-  // should not be not visible here.
-  (void)&static_var_module_linkage; // FIXME: Should not be visible here.
-
-  (void)&const_var_module_linkage; // FIXME: will be visible after P2788R0
+  // Internal-linkage declarations are not visible here.
 }
 
 //--- user.cpp
@@ -177,6 +173,6 @@
   (void)&inline_var_exported;
   (void)&const_var_exported;
 
-  // Internal-linkage declarations are not visible here.
   // Module-linkage declarations are not visible here.
+  // Internal-linkage declarations are not visible here.
 }
Index: clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp
===================================================================
--- clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp
+++ clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp
@@ -62,27 +62,15 @@
 
   not_exported = 1;
 #ifndef IMPLEMENTATION
-  // expected-error@-2 {{declaration of 'not_exported' must be imported from module 'A' before it is required}}
-  // expected-n...@p2.cpp:19 {{declaration here is not visible}}
+  // expected-error@-2 {{declaration of 'not_exported' is private to module 'A'}}
+  // expected-n...@p2.cpp:19 {{export the declaration to make it available}}
 #endif
 
-  internal = 1;
-#ifndef IMPLEMENTATION
-  // expected-error@-2 {{declaration of 'internal' must be imported from module 'A' before it is required}}
-  // expected-n...@p2.cpp:20 {{declaration here is not visible}}
-#endif
+  internal = 1; // expected-error {{use of undeclared identifier 'internal'}}
 
-  not_exported_private = 1;
-#ifndef IMPLEMENTATION
-  // FIXME: should not be visible here
-  // expected-error@-3 {{undeclared identifier}}
-#endif
+  not_exported_private = 1; // expected-error {{use of undeclared identifier 'not_exported_private'}}
 
-  internal_private = 1;
-#ifndef IMPLEMENTATION
-  // FIXME: should not be visible here
-  // expected-error@-3 {{undeclared identifier}}
-#endif
+  internal_private = 1; // expected-error {{undeclared identifier 'internal_private'}}
 }
 
 #endif
Index: clang/lib/Sema/SemaLookup.cpp
===================================================================
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -1780,7 +1780,7 @@
   Module *DeclModule = SemaRef.getOwningModule(D);
   assert(DeclModule && "hidden decl has no owning module");
 
-  // If the owning module is visible, the decl is acceptable.
+  // If the owning module is visible, the decl is potentially acceptable.
   if (SemaRef.isModuleVisible(DeclModule,
                               D->isInvisibleOutsideTheOwningModule()))
     return true;
@@ -2065,14 +2065,27 @@
   return findAcceptableDecl(getSema(), D, IDNS);
 }
 
-bool LookupResult::isVisible(Sema &SemaRef, NamedDecl *D) {
+bool LookupResult::isVisible(Sema &SemaRef, NamedDecl *D, bool ForTypo) {
   // If this declaration is already visible, return it directly.
   if (D->isUnconditionallyVisible())
     return true;
 
   // During template instantiation, we can refer to hidden declarations, if
   // they were visible in any module along the path of instantiation.
-  return isAcceptableSlow(SemaRef, D, Sema::AcceptableKind::Visible);
+  if (!isAcceptableSlow(SemaRef, D, Sema::AcceptableKind::Visible))
+    return false;
+
+  if (ForTypo && SemaRef.getLangOpts().CPlusPlusModules) {
+    // There is no point in offering a typo correction to an item that we will
+    // reject because it has internal linkage and is in a different module TU;
+    Module *DeclModule = SemaRef.getOwningModule(D);
+    if (DeclModule && !DeclModule->isHeaderLikeModule() &&
+        !SemaRef.isModuleUnitOfCurrentTU(DeclModule) &&
+        (D->getFormalLinkage() == Linkage::InternalLinkage ||
+         DeclModule->isPrivateModule()))
+      return false; // This is not usefully visible.
+  }
+  return true;
 }
 
 bool LookupResult::isReachable(Sema &SemaRef, NamedDecl *D) {
@@ -2084,8 +2097,22 @@
 
 bool LookupResult::isAvailableForLookup(Sema &SemaRef, NamedDecl *ND) {
   // We should check the visibility at the callsite already.
-  if (isVisible(SemaRef, ND))
+  if (isVisible(SemaRef, ND)) {
+    if (SemaRef.getLangOpts().CPlusPlusModules) {
+      // The module that owns the decl is visible; However
+      // [basic.lookup.general]/2, specifically 2.3
+      // If the owning module is part of a different TU than the current one,
+      // and declaration is internal (or from the PMF), then we should not
+      // include it.
+      Module *DeclModule = SemaRef.getOwningModule(ND);
+      if (DeclModule && !DeclModule->isHeaderLikeModule() &&
+          !SemaRef.isModuleUnitOfCurrentTU(DeclModule) &&
+          (ND->getFormalLinkage() == Linkage::InternalLinkage ||
+           DeclModule->isPrivateModule()))
+        return false;
+    }
     return true;
+  }
 
   // Deduction guide lives in namespace scope generally, but it is just a
   // hint to the compilers. What we actually lookup for is the generated member
@@ -3874,16 +3901,44 @@
       for (D = D->getMostRecentDecl(); D;
            D = cast_or_null<NamedDecl>(D->getPreviousDecl())) {
         if (D->getIdentifierNamespace() & Decl::IDNS_Ordinary) {
-          if (isVisible(D)) {
-            Visible = true;
-            break;
+          Visible = isVisible(D);
+          if (!getLangOpts().CPlusPlusModules) {
+            if (Visible)
+              break;
+            continue;
           }
 
-          if (!getLangOpts().CPlusPlusModules)
-            continue;
+          Module *FM = D->getOwningModule();
+          if (Visible) {
+            if (!FM)
+              break;
+            assert(D->hasLinkage() && "an imported func with no linkage?");
+            // Unless the module is a defining one for the
+            bool Ovr = true;
+            for (unsigned I = 0; I < CodeSynthesisContexts.size(); ++I) {
+              Module *M = CodeSynthesisContexts[I].Entity
+                              ? getDefiningModule(
+                                    *this, CodeSynthesisContexts[I].Entity)
+                              : nullptr;
+              if (FM == M) {
+                Ovr = false;
+                break;
+              }
+            }
+            if (Ovr && D->getFormalLinkage() == Linkage::InternalLinkage &&
+                !isModuleUnitOfCurrentTU(FM)) {
+              Visible = false;
+              continue;
+            }
+            if (Ovr && D->getFormalLinkage() == Linkage::ModuleLinkage &&
+                !isPartOfCurrentNamedModule(FM)) {
+              Visible = false;
+              continue;
+            }
+            break;
+          }
 
           if (D->isInExportDeclContext()) {
-            Module *FM = D->getOwningModule();
             // C++20 [basic.lookup.argdep] p4.3 .. are exported ...
             // exports are only valid in module purview and outside of any
             // PMF (although a PMF should not even be present in a module
@@ -4463,9 +4518,13 @@
 static void checkCorrectionVisibility(Sema &SemaRef, TypoCorrection &TC) {
   TypoCorrection::decl_iterator DI = TC.begin(), DE = TC.end();
 
-  for (/**/; DI != DE; ++DI)
-    if (!LookupResult::isVisible(SemaRef, *DI))
+  // During the lookups for Typo corrections, we allow 'hidden' decls to be
+  // found, this can also include (potential typo correction) decls that are
+  // not in their own right viable.  Filter these from the visible set.
+  for (/**/; DI != DE; ++DI) {
+    if (!LookupResult::isVisible(SemaRef, *DI, /*ForTypo*/ true))
       break;
+  }
   // No filtering needed if all decls are visible.
   if (DI == DE) {
     TC.setRequiresImport(false);
@@ -4476,15 +4535,25 @@
   bool AnyVisibleDecls = !NewDecls.empty();
 
   for (/**/; DI != DE; ++DI) {
-    if (LookupResult::isVisible(SemaRef, *DI)) {
+    if (LookupResult::isVisible(SemaRef, *DI, /*ForTypo*/ true)) {
       if (!AnyVisibleDecls) {
         // Found a visible decl, discard all hidden ones.
         AnyVisibleDecls = true;
         NewDecls.clear();
       }
       NewDecls.push_back(*DI);
-    } else if (!AnyVisibleDecls && !(*DI)->isModulePrivate())
-      NewDecls.push_back(*DI);
+    } else if (!AnyVisibleDecls) {
+      // We do not want to push a hidden decl if it is not usefully reported
+      // as a typo (which means the proposed replacement has to be viable).
+      NamedDecl *ND = *DI;
+      if (SemaRef.getLangOpts().CPlusPlusModules && ND->isFromASTFile() &&
+          ND->getOwningModule() &&
+          !SemaRef.isModuleUnitOfCurrentTU(ND->getOwningModule()) &&
+          ND->getFormalLinkage() == Linkage::InternalLinkage)
+        continue; // Internal symbol from a different TU, not visible.
+      if (!ND->isModulePrivate())
+        NewDecls.push_back(ND);
+    }
   }
 
   if (NewDecls.empty())
@@ -4552,7 +4621,7 @@
 
   // Only consider visible declarations and declarations from modules with
   // names that exactly match.
-  if (!LookupResult::isVisible(SemaRef, ND) && Name != Typo)
+  if (!LookupResult::isVisible(SemaRef, ND, /*ForTypo*/ true) && Name != Typo)
     return;
 
   FoundName(Name->getName());
@@ -5735,6 +5804,7 @@
 
   Modules = UniqueModules;
 
+  bool DidFixit = false;
   if (Modules.size() > 1) {
     std::string ModuleList;
     unsigned N = 0;
@@ -5749,13 +5819,26 @@
 
     Diag(UseLoc, diag::err_module_unimported_use_multiple)
       << (int)MIK << Decl << ModuleList;
+  } else if (Decl->hasLinkage() &&
+             Decl->getFormalLinkage() == Linkage::ModuleLinkage) {
+    Diag(UseLoc, diag::err_module_private_use)
+        << (int)MIK << Decl << Modules[0]->getFullModuleName();
+    Diag(Decl->getBeginLoc(), diag::note_suggest_export)
+        << (int)MIK
+        << FixItHint::CreateInsertion(Decl->getBeginLoc(), "export");
+    DidFixit = true;
+  } else if (Decl->hasLinkage() &&
+             Decl->getFormalLinkage() == Linkage::InternalLinkage) {
+    Diag(UseLoc, diag::err_module_internal_use)
+        << (int)MIK << Decl << Modules[0]->getFullModuleName();
   } else {
     // FIXME: Add a FixItHint that imports the corresponding module.
     Diag(UseLoc, diag::err_module_unimported_use)
       << (int)MIK << Decl << Modules[0]->getFullModuleName();
   }
 
-  NotePrevious();
+  if (!DidFixit)
+    NotePrevious();
 
   // Try to recover by implicitly importing this module.
   if (Recover)
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -2355,6 +2355,21 @@
   // Determine whether the module M belongs to the  current TU.
   bool isModuleUnitOfCurrentTU(const Module *M) const;
 
+  /// Determine whether the module MA is part of the same named module as MB.
+  bool arePartOfSameNamedModule(const Module *MA, const Module *MB) const {
+    if (!MA || MA->isGlobalModule())
+      return false;
+    if (!MB || MB->isGlobalModule())
+      return false;
+    return MA->getPrimaryModuleInterfaceName() ==
+           MB->getPrimaryModuleInterfaceName();
+  }
+
+  /// Determine whether the module M is part of the current named module.
+  bool isPartOfCurrentNamedModule(const Module *M) const {
+    return arePartOfSameNamedModule(getCurrentModule(), M);
+  }
+
   /// Make a merged definition of an existing hidden definition \p ND
   /// visible at the specified location.
   void makeMergedDefinitionVisible(NamedDecl *ND);
Index: clang/include/clang/Sema/Lookup.h
===================================================================
--- clang/include/clang/Sema/Lookup.h
+++ clang/include/clang/Sema/Lookup.h
@@ -346,7 +346,7 @@
 
   /// Determine whether the given declaration is visible to the
   /// program.
-  static bool isVisible(Sema &SemaRef, NamedDecl *D);
+  static bool isVisible(Sema &SemaRef, NamedDecl *D, bool ForTypo = false);
 
   static bool isReachable(Sema &SemaRef, NamedDecl *D);
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11176,6 +11176,17 @@
   "%select{declaration|definition|default argument|"
   "explicit specialization|partial specialization}0 of %1 must be imported "
   "from module '%2' before it is required">;
+def err_module_private_use : Error<
+  "%select{declaration|definition|default argument|"
+  "explicit specialization|partial specialization}0 of %1 is private to module "
+  "'%2'">;
+def err_module_internal_use : Error<
+  "%select{declaration|definition|default argument|"
+  "explicit specialization|partial specialization}0 of %1 is internal to "
+  "'%2'">;
+def note_suggest_export : Note<
+  "export the %select{declaration|definition|default argument|"
+  "explicit specialization|partial specialization}0 to make it available">;
 def err_module_unimported_use_header : Error<
   "%select{missing '#include'|missing '#include %3'}2; "
   "%select{||default argument of |explicit specialization of |"
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to