ChuanqiXu created this revision.
ChuanqiXu added reviewers: rsmith, aaron.ballman, erichkeane, urnathan, 
hubert.reinterpretcast.
ChuanqiXu added a project: clang.
Herald added a subscriber: dexonsmith.
ChuanqiXu requested review of this revision.
Herald added a subscriber: cfe-commits.

This fixes: https://bugs.llvm.org/show_bug.cgi?id=52281 and 
https://godbolt.org/z/81f3ocjfW.

This patch introduces a new kind of `ModuleOwnershipKind` as 
`ReachableWhenImported`. This intended the status for reachable described at: 
https://eel.is/c++draft/module.reach#3.

Note that this patch is not intended to support all semantics about reachable 
semantics.

An important feature not included in this patch is discarding declaration in 
global module fragment. See https://eel.is/c++draft/module.global.frag#3 and 
https://eel.is/c++draft/module.global.frag#4. This feature is important since 
it would cut off many unused declarations so that user could import a large 
module without worrying it would be too heavy. And I think this is related to 
the bug https://bugs.llvm.org/show_bug.cgi?id=52342 mentioned @rsmith .

But after all, I think it is not easy to implement in one shot. So this patch 
didn't contain that feature. In other words, now all the declarations in the 
global module fragment are reachable even they could be reduced. But this is 
not worse than the current situation. So I think it may not be unacceptable.

Test Plan: check-all and https://godbolt.org/z/81f3ocjfW.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113545

Files:
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/Basic/Module.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaCXXScopeSpec.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp
  clang/test/CXX/module/module.interface/p2.cpp
  clang/test/CXX/module/module.reach/Inputs/p5-A.cppm
  clang/test/CXX/module/module.reach/p5.cpp
  clang/test/Modules/Inputs/Reachability-Private/Private.cppm
  clang/test/Modules/Inputs/Reachability-func-default-arg/func_default_arg.cppm
  clang/test/Modules/Inputs/Reachability-func-ret/func_ret.cppm
  
clang/test/Modules/Inputs/Reachability-template-default-arg/template_default_arg.cppm
  clang/test/Modules/Inputs/Reachability-using-templates/mod-templates.cppm
  clang/test/Modules/Inputs/Reachability-using/mod.cppm
  clang/test/Modules/Reachability-Private.cpp
  clang/test/Modules/Reachability-func-default-arg.cpp
  clang/test/Modules/Reachability-func-ret.cpp
  clang/test/Modules/Reachability-template-default-arg.cpp
  clang/test/Modules/Reachability-using-templates.cpp
  clang/test/Modules/Reachability-using.cpp
  clang/test/SemaCXX/compare-modules-cxx2a.cpp

Index: clang/test/SemaCXX/compare-modules-cxx2a.cpp
===================================================================
--- clang/test/SemaCXX/compare-modules-cxx2a.cpp
+++ clang/test/SemaCXX/compare-modules-cxx2a.cpp
@@ -1,3 +1,4 @@
+// This tests the unused declaration in global module fragment should be discarded.
 // RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -I%S/Inputs %s -fno-modules-error-recovery
 
 #pragma clang module build compare
Index: clang/test/Modules/Reachability-using.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/Reachability-using.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/Reachability-using/mod.cppm --precompile -o %t/mod.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c
+// expected-no-diagnostics
+import mod;
+void foo() {
+  u v{};
+}
Index: clang/test/Modules/Reachability-using-templates.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/Reachability-using-templates.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/Reachability-using-templates/mod-templates.cppm --precompile -o %t/mod.templates.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c
+// expected-no-diagnostics
+import mod.templates;
+void foo() {
+  u<int> v{};
+}
Index: clang/test/Modules/Reachability-template-default-arg.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/Reachability-template-default-arg.cpp
@@ -0,0 +1,11 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/Reachability-template-default-arg/template_default_arg.cppm --precompile -o %t/template_default_arg.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c -Xclang -verify
+
+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@Inputs/Reachability-template-default-arg/template_default_arg.cppm:2 {{declaration here is not visible}}
+}
Index: clang/test/Modules/Reachability-func-ret.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/Reachability-func-ret.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/Reachability-func-ret/func_ret.cppm --precompile -o %t/func_ret.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c
+// expected-no-diagnostics
+import func_ret;
+void bar() {
+  auto ret = foo();
+}
Index: clang/test/Modules/Reachability-func-default-arg.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/Reachability-func-default-arg.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/Reachability-func-default-arg/func_default_arg.cppm --precompile -o %t/func_default_arg.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c
+// expected-no-diagnostics
+import func_default_arg;
+void bar() {
+  auto ret = foo();
+}
Index: clang/test/Modules/Reachability-Private.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/Reachability-Private.cpp
@@ -0,0 +1,19 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/Reachability-Private/Private.cppm --precompile -o %t/Private.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c -Xclang -verify
+
+import Private;
+void foo() {
+    X x; // expected-error {{definition of 'X' must be imported from module 'Private.<private>' before it is required}}
+         // expected-error@-1 {{definition of 'X' must be imported from module 'Private.<private>' before it is required}}
+         // expected-note@Inputs/Reachability-Private/Private.cppm:13 {{definition here is not reachable}}
+         // expected-note@Inputs/Reachability-Private/Private.cppm:13 {{definition here is not reachable}}
+    auto _ = factory();
+    auto *__ = factory();
+    X* ___ = factory();
+
+    g(__);
+    g(___);
+    g(factory());
+}
Index: clang/test/Modules/Inputs/Reachability-using/mod.cppm
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/Reachability-using/mod.cppm
@@ -0,0 +1,3 @@
+export module mod;
+struct t { };
+export using u = t;
Index: clang/test/Modules/Inputs/Reachability-using-templates/mod-templates.cppm
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/Reachability-using-templates/mod-templates.cppm
@@ -0,0 +1,3 @@
+export module mod.templates;
+template<class> struct t { };
+export template<class T> using u = t<T>;
Index: clang/test/Modules/Inputs/Reachability-template-default-arg/template_default_arg.cppm
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/Reachability-template-default-arg/template_default_arg.cppm
@@ -0,0 +1,8 @@
+export module template_default_arg;
+struct t { };
+
+export
+template <typename T = t>
+struct A {
+    T a;
+};
Index: clang/test/Modules/Inputs/Reachability-func-ret/func_ret.cppm
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/Reachability-func-ret/func_ret.cppm
@@ -0,0 +1,5 @@
+export module func_ret;
+struct t { };
+export t foo() {
+    return t{};
+}
Index: clang/test/Modules/Inputs/Reachability-func-default-arg/func_default_arg.cppm
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/Reachability-func-default-arg/func_default_arg.cppm
@@ -0,0 +1,5 @@
+export module func_default_arg;
+struct t { };
+export t foo(t t1= t()) {
+    return t1;
+}
Index: clang/test/Modules/Inputs/Reachability-Private/Private.cppm
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/Reachability-Private/Private.cppm
@@ -0,0 +1,18 @@
+export module Private;
+inline void fn_m();             // OK, module-linkage inline function
+static void fn_s();
+export struct X;
+
+export void g(X *x) {
+  fn_s();                       // OK, call to static function in same translation unit
+  fn_m();                       // OK, call to module-linkage inline function
+}
+export X *factory();            // OK
+
+module :private;
+struct X {};                    // definition not reachable from importers of A
+X *factory() {
+  return new X ();
+}
+void fn_m() {}
+void fn_s() {}
Index: clang/test/CXX/module/module.reach/p5.cpp
===================================================================
--- /dev/null
+++ clang/test/CXX/module/module.reach/p5.cpp
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/p5-A.cppm --precompile -o %t/A.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t %s -c -Xclang -verify
+
+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@Inputs/p5-A.cppm:2 {{declaration here is not visible}}
Index: clang/test/CXX/module/module.reach/Inputs/p5-A.cppm
===================================================================
--- /dev/null
+++ clang/test/CXX/module/module.reach/Inputs/p5-A.cppm
@@ -0,0 +1,3 @@
+export module A;
+struct X {};
+export using Y = X;
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,22 +69,29 @@
 
 void use() {
   // namespace A is implicitly exported by the export of A::g.
-  A::f(); // expected-error {{no member named 'f' in namespace 'A'}}
+  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::g();
-  A::h(); // expected-error {{no member named 'h' in namespace 'A'}}
-  using namespace A::inner; // expected-error {{expected namespace name}}
+  A::h();                   // expected-error {{declaration of 'h' must be imported from module 'p2' before it is required}}
+                            // expected-note@* {{declaration here is not visible}}
+  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 {{no member named 'f' in namespace 'B'}}
-  f(); // expected-error {{undeclared identifier 'f'}}
+  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}}
 
   // namespace C is not exported
-  using namespace C; // expected-error {{expected namespace name}}
+  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 {{no member named 'f' in namespace 'D'}}
+  D::f(); // expected-error {{declaration of 'f' must be imported from module 'p2' before it is required}}
+          // expected-note@* {{declaration here is not visible}}
 }
 
 int use_header() { return foo + bar::baz(); }
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
@@ -29,18 +29,18 @@
 #endif
 
 void test_early() {
-  in_header = 1; // expected-error {{missing '#include "foo.h"'; 'in_header' must be declared before it is used}}
+  in_header = 1; // expected-error {{use of undeclared identifier 'in_header'}}
   // expected-note@*{{not visible}}
 
-  global_module_fragment = 1; // expected-error {{missing '#include'; 'global_module_fragment' must be declared before it is used}}
-  // expected-n...@p2.cpp:16 {{not visible}}
+  global_module_fragment = 1; // expected-error {{use of undeclared identifier 'global_module_fragment'}}
 
   exported = 1; // expected-error {{must be imported from module 'A'}}
-  // expected-n...@p2.cpp:18 {{not visible}}
 
-  not_exported = 1; // expected-error {{undeclared identifier}}
+  not_exported = 1; // expected-error {{declaration of 'not_exported' must be imported from module 'A' before it is required}}
 
-  internal = 1; // expected-error {{undeclared identifier}}
+  // FIXME: We need better diagnostic message for static variable.
+  internal = 1; // expected-error {{declaration of 'internal' must be imported from module 'A' before it is required}}
+                // expected-n...@p2.cpp:20 {{declaration here is not visible}}
 
   not_exported_private = 1; // expected-error {{undeclared identifier}}
 
@@ -54,24 +54,23 @@
 #endif
 
 void test_late() {
-  in_header = 1; // expected-error {{missing '#include "foo.h"'; 'in_header' must be declared before it is used}}
+  in_header = 1; // expected-error {{use of undeclared identifier 'in_header'}}
   // expected-note@*{{not visible}}
 
-  global_module_fragment = 1; // expected-error {{missing '#include'; 'global_module_fragment' must be declared before it is used}}
-  // expected-n...@p2.cpp:16 {{not visible}}
+  global_module_fragment = 1; // expected-error {{use of undeclared identifier 'global_module_fragment'}}
 
   exported = 1;
 
   not_exported = 1;
 #ifndef IMPLEMENTATION
-  // expected-error@-2 {{undeclared identifier 'not_exported'; did you mean 'exported'}}
-  // expected-n...@p2.cpp:18 {{declared here}}
+  // 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}}
 #endif
 
   internal = 1;
 #ifndef IMPLEMENTATION
-  // FIXME: should not be visible here
-  // expected-error@-3 {{undeclared identifier}}
+  // 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
 
   not_exported_private = 1;
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===================================================================
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -310,7 +310,7 @@
   Record.push_back(D->isReferenced());
   Record.push_back(D->isTopLevelDeclInObjCContainer());
   Record.push_back(D->getAccess());
-  Record.push_back(D->isModulePrivate());
+  Record.push_back((uint64_t)D->getModuleOwnershipKind());
   Record.push_back(Writer.getSubmoduleID(D->getOwningModule()));
 
   // If this declaration injected a name into a context different from its
@@ -1921,7 +1921,7 @@
   Abv->Add(BitCodeAbbrevOp(0));                       // isReferenced
   Abv->Add(BitCodeAbbrevOp(0));                   // TopLevelDeclInObjCContainer
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 2));  // AccessSpecifier
-  Abv->Add(BitCodeAbbrevOp(0));                       // ModulePrivate
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3));  // ModulePrivate
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // SubmoduleID
   // NamedDecl
   Abv->Add(BitCodeAbbrevOp(0));                       // NameKind = Identifier
@@ -1954,7 +1954,7 @@
   Abv->Add(BitCodeAbbrevOp(0));                       // isReferenced
   Abv->Add(BitCodeAbbrevOp(0));                   // TopLevelDeclInObjCContainer
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 2));  // AccessSpecifier
-  Abv->Add(BitCodeAbbrevOp(0));                       // ModulePrivate
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3));  // ModulePrivate
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // SubmoduleID
   // NamedDecl
   Abv->Add(BitCodeAbbrevOp(0));                       // NameKind = Identifier
@@ -1992,7 +1992,7 @@
   Abv->Add(BitCodeAbbrevOp(0));                       // isReferenced
   Abv->Add(BitCodeAbbrevOp(0));                   // TopLevelDeclInObjCContainer
   Abv->Add(BitCodeAbbrevOp(AS_none));                 // C++ AccessSpecifier
-  Abv->Add(BitCodeAbbrevOp(0));                       // ModulePrivate
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); // ModulePrivate
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // SubmoduleID
   // NamedDecl
   Abv->Add(BitCodeAbbrevOp(0));                       // NameKind = Identifier
@@ -2042,7 +2042,7 @@
   Abv->Add(BitCodeAbbrevOp(0));                       // isReferenced
   Abv->Add(BitCodeAbbrevOp(0));                   // TopLevelDeclInObjCContainer
   Abv->Add(BitCodeAbbrevOp(AS_none));                 // C++ AccessSpecifier
-  Abv->Add(BitCodeAbbrevOp(0));                       // ModulePrivate
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); // ModulePrivate
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // SubmoduleID
   // NamedDecl
   Abv->Add(BitCodeAbbrevOp(0));                       // NameKind = Identifier
@@ -2104,7 +2104,7 @@
   Abv->Add(BitCodeAbbrevOp(0));                       // isReferenced
   Abv->Add(BitCodeAbbrevOp(0));                   // TopLevelDeclInObjCContainer
   Abv->Add(BitCodeAbbrevOp(AS_none));                 // C++ AccessSpecifier
-  Abv->Add(BitCodeAbbrevOp(0));                       // ModulePrivate
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); // ModulePrivate
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // SubmoduleID
   // NamedDecl
   Abv->Add(BitCodeAbbrevOp(0));                       // NameKind = Identifier
@@ -2152,7 +2152,7 @@
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isReferenced
   Abv->Add(BitCodeAbbrevOp(0));                   // TopLevelDeclInObjCContainer
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 2)); // C++ AccessSpecifier
-  Abv->Add(BitCodeAbbrevOp(0));                       // ModulePrivate
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); // ModulePrivate
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // SubmoduleID
   // NamedDecl
   Abv->Add(BitCodeAbbrevOp(0));                       // NameKind = Identifier
@@ -2181,7 +2181,7 @@
   Abv->Add(BitCodeAbbrevOp(0));                       // isReferenced
   Abv->Add(BitCodeAbbrevOp(0));                   // TopLevelDeclInObjCContainer
   Abv->Add(BitCodeAbbrevOp(AS_none));                 // C++ AccessSpecifier
-  Abv->Add(BitCodeAbbrevOp(0));                       // ModulePrivate
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); // ModulePrivate
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // SubmoduleID
   // NamedDecl
   Abv->Add(BitCodeAbbrevOp(0));                       // NameKind = Identifier
@@ -2233,7 +2233,7 @@
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Referenced
   Abv->Add(BitCodeAbbrevOp(0));                         // InObjCContainer
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 2)); // Access
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // ModulePrivate
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); // ModulePrivate
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));   // SubmoduleID
   // NamedDecl
   Abv->Add(BitCodeAbbrevOp(DeclarationName::Identifier)); // NameKind
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -603,15 +603,22 @@
   D->setTopLevelDeclInObjCContainer(Record.readInt());
   D->setAccess((AccessSpecifier)Record.readInt());
   D->FromASTFile = true;
-  bool ModulePrivate = Record.readInt();
+  auto ModuleOwnership = (Decl::ModuleOwnershipKind)Record.readInt();
+  bool ModulePrivate =
+      (ModuleOwnership == Decl::ModuleOwnershipKind::ModulePrivate);
 
   // Determine whether this declaration is part of a (sub)module. If so, it
   // may not yet be visible.
   if (unsigned SubmoduleID = readSubmoduleID()) {
+    if (ModuleOwnership == Decl::ModuleOwnershipKind::Visible)
+      ModuleOwnership = Decl::ModuleOwnershipKind::VisibleWhenImported;
+
+    if ((int)ModuleOwnership > 4)
+      llvm::report_fatal_error(
+          "The size of sizeModuleOwnership is larger than 4.\n");
+
+    D->setModuleOwnershipKind(ModuleOwnership);
     // Store the owning submodule ID in the declaration.
-    D->setModuleOwnershipKind(
-        ModulePrivate ? Decl::ModuleOwnershipKind::ModulePrivate
-                      : Decl::ModuleOwnershipKind::VisibleWhenImported);
     D->setOwningModuleID(SubmoduleID);
 
     if (ModulePrivate) {
Index: clang/lib/Sema/SemaType.cpp
===================================================================
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -8571,6 +8571,56 @@
   return false;
 }
 
+// [module.reach]p3.1:
+// A declaration D is reachable from a point P if
+// - D appears prior to P in the same translation unit, or
+// - D is not discarded ([module.global.frag]), appears in a translation unit
+// that is reachable from P, and does not appear within a
+// private-module-fragment. A declaration is reachable if it is reachable from
+// any point in the instantiation context ([module.context]).
+//
+// [module.reach]p2:
+// All translation units that are necessarily reachable are reachable.
+// Additional translation units on which the point within the program has an
+// interface dependency may be considered reachable, but it is unspecified which
+// are and under what circumstances.
+bool Sema::hasReachableDefinition(NamedDecl *D, NamedDecl **Suggested,
+                                  bool OnlyNeedComplete) {
+  bool IsVisible = hasVisibleDefinition(D, Suggested, OnlyNeedComplete);
+
+  // If it didn't introduce C++ Modules, it is meaningless to talk about
+  // reachable definition.
+  if (!getLangOpts().CPlusPlusModules)
+    return IsVisible;
+
+  // Any visible declaration is reachable.
+  if (IsVisible)
+    return true;
+
+  // If D owns a module and the module is not visible. It implies that the
+  // module is not imported. So we should return false directly. This is
+  // possible in case of `#pragma clang module`. See
+  // clang/test/SemaCXX/compare-modules-cxx2a.cpp for example.
+  if (D->getOwningModule() && !VisibleModules.isVisible(D->getOwningModule()))
+    return false;
+
+  // If D isn't from AST file, it implies that D appears in the same TU.
+  // So it should be reachable.
+  if (!D->isFromASTFile())
+    return true;
+
+  if (D->isModulePrivate())
+    return false;
+
+  if (!D->isInGlobalModuleFragment())
+    return true;
+
+  if (D->isDiscardedInGlobalModuleFragment())
+    return false;
+
+  return true;
+}
+
 /// Locks in the inheritance model for the given class and all of its bases.
 static void assignInheritanceModel(Sema &S, CXXRecordDecl *RD) {
   RD = RD->getMostRecentNonInjectedDecl();
@@ -8644,16 +8694,15 @@
 
   // If we have a complete type, we're done.
   if (!Incomplete) {
-    // If we know about the definition but it is not visible, complain.
-    NamedDecl *SuggestedDef = nullptr;
+    NamedDecl *Suggested = nullptr;
     if (Def &&
-        !hasVisibleDefinition(Def, &SuggestedDef, /*OnlyNeedComplete*/true)) {
+        !hasReachableDefinition(Def, &Suggested, /*OnlyNeedComplete=*/true)) {
       // If the user is going to see an error here, recover by making the
       // definition visible.
       bool TreatAsComplete = Diagnoser && !isSFINAEContext();
-      if (Diagnoser && SuggestedDef)
-        diagnoseMissingImport(Loc, SuggestedDef, MissingImportKind::Definition,
-                              /*Recover*/TreatAsComplete);
+      if (Diagnoser && Suggested)
+        diagnoseMissingImport(Loc, Suggested, MissingImportKind::Definition,
+                              /*Recover*/ TreatAsComplete);
       return !TreatAsComplete;
     } else if (Def && !TemplateInstCallbacks.empty()) {
       CodeSynthesisContext TempInst;
Index: clang/lib/Sema/SemaTemplate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -795,8 +795,9 @@
 
   if (PatternDef && !IsEntityBeingDefined) {
     NamedDecl *SuggestedDef = nullptr;
-    if (!hasVisibleDefinition(const_cast<NamedDecl*>(PatternDef), &SuggestedDef,
-                              /*OnlyNeedComplete*/false)) {
+    if (!hasReachableDefinition(const_cast<NamedDecl *>(PatternDef),
+                                &SuggestedDef,
+                                /*OnlyNeedComplete*/ false)) {
       // If we're allowed to diagnose this and recover, do so.
       bool Recover = Complain && !isSFINAEContext();
       if (Complain)
Index: clang/lib/Sema/SemaModule.cpp
===================================================================
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -80,7 +80,15 @@
 
   // All declarations created from now on are owned by the global module.
   auto *TU = Context.getTranslationUnitDecl();
-  TU->setModuleOwnershipKind(Decl::ModuleOwnershipKind::Visible);
+  // [module.global.frag]p2
+  // A global-module-fragment specifies the contents of the global module
+  // fragment for a module unit. The global module fragment can be used to
+  // provide declarations that are attached to the global module and usable
+  // within the module unit.
+  //
+  // So that the global module fragment could only be used in current module
+  // unit.
+  TU->setModuleOwnershipKind(Decl::ModuleOwnershipKind::ModulePrivate);
   TU->setLocalOwningModule(GlobalModule);
 
   // FIXME: Consider creating an explicit representation of this declaration.
@@ -232,10 +240,16 @@
   VisibleModules.setVisible(Mod, ModuleLoc);
 
   // From now on, we have an owning module for all declarations we see.
-  // However, those declarations are module-private unless explicitly
+  // In C++20 modules, those declaration would be reachable when imported
+  // unless explicitily exported.
+  // Otherwise, those declarations are module-private unless explicitly
   // exported.
   auto *TU = Context.getTranslationUnitDecl();
-  TU->setModuleOwnershipKind(Decl::ModuleOwnershipKind::ModulePrivate);
+  if (getLangOpts().CPlusPlusModules)
+    TU->setModuleOwnershipKind(
+        Decl::ModuleOwnershipKind::ReachableWhenImported);
+  else
+    TU->setModuleOwnershipKind(Decl::ModuleOwnershipKind::ModulePrivate);
   TU->setLocalOwningModule(Mod);
 
   // FIXME: Create a ModuleDecl.
Index: clang/lib/Sema/SemaLookup.cpp
===================================================================
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -1684,7 +1684,8 @@
   assert(DeclModule && "hidden decl has no owning module");
 
   // If the owning module is visible, the decl is visible.
-  if (SemaRef.isModuleVisible(DeclModule, D->isModulePrivate()))
+  if (SemaRef.isModuleVisible(DeclModule,
+                              D->isInvisibleOutsideTheOwningModule()))
     return true;
 
   // Determine whether a decl context is a file context for the purpose of
@@ -1752,6 +1753,18 @@
 }
 
 bool Sema::isModuleVisible(const Module *M, bool ModulePrivate) {
+  // [module.global.frag]p2:
+  // A global-module-fragment specifies the contents of the global module
+  // fragment for a module unit. The global module fragment can be used to
+  // provide declarations that are attached to the global module and usable
+  // within the module unit.
+  //
+  // Global module fragment is special. Global Module fragment is only usable
+  // within the module unit it got defined [module.global.frag]p2. In this
+  // case, the global module fragment shouldn't own an AST File.
+  if (M->isGlobalModule() && M->getASTFile())
+    return false;
+
   // The module might be ordinarily visible. For a module-private query, that
   // means it is part of the current module. For any other query, that means it
   // is in our visible module set.
Index: clang/lib/Sema/SemaCXXScopeSpec.cpp
===================================================================
--- clang/lib/Sema/SemaCXXScopeSpec.cpp
+++ clang/lib/Sema/SemaCXXScopeSpec.cpp
@@ -121,7 +121,7 @@
             // entering the context, and that can't happen in a SFINAE context.
             assert(!isSFINAEContext() &&
                    "partial specialization scope specifier in SFINAE context?");
-            if (!hasVisibleDeclaration(PartialSpec))
+            if (!hasReachableDefinition(PartialSpec))
               diagnoseMissingImport(SS.getLastQualifierNameLoc(), PartialSpec,
                                     MissingImportKind::PartialSpecialization,
                                     /*Recover*/true);
@@ -243,8 +243,8 @@
   if (EnumD->isCompleteDefinition()) {
     // If we know about the definition but it is not visible, complain.
     NamedDecl *SuggestedDef = nullptr;
-    if (!hasVisibleDefinition(EnumD, &SuggestedDef,
-                              /*OnlyNeedComplete*/false)) {
+    if (!hasReachableDefinition(EnumD, &SuggestedDef,
+                                /*OnlyNeedComplete*/ false)) {
       // If the user is going to see an error here, recover by making the
       // definition visible.
       bool TreatAsComplete = !isSFINAEContext();
Index: clang/lib/AST/Decl.cpp
===================================================================
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -579,6 +579,7 @@
   // FIXME: Handle isModulePrivate.
   switch (D->getModuleOwnershipKind()) {
   case Decl::ModuleOwnershipKind::Unowned:
+  case Decl::ModuleOwnershipKind::ReachableWhenImported:
   case Decl::ModuleOwnershipKind::ModulePrivate:
     return false;
   case Decl::ModuleOwnershipKind::Visible:
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -2258,6 +2258,13 @@
     return hasVisibleDefinition(const_cast<NamedDecl*>(D), &Hidden);
   }
 
+  bool hasReachableDefinition(NamedDecl *D, NamedDecl **Suggested,
+                              bool OnlyNeedComplete = false);
+  bool hasReachableDefinition(NamedDecl *D) {
+    NamedDecl *Hidden;
+    return hasVisibleDefinition(D, &Hidden);
+  }
+
   /// Determine if the template parameter \p D has a visible default argument.
   bool
   hasVisibleDefaultArgument(const NamedDecl *D,
Index: clang/include/clang/Basic/Module.h
===================================================================
--- clang/include/clang/Basic/Module.h
+++ clang/include/clang/Basic/Module.h
@@ -153,6 +153,10 @@
     return Kind == ModuleInterfaceUnit || Kind == PrivateModuleFragment;
   }
 
+  /// Does this Module scope describe a fragment of the global module within
+  /// some C++ module.
+  bool isGlobalModule() const { return Kind == GlobalModuleFragment; }
+
 private:
   /// The submodules of this module, indexed by name.
   std::vector<Module *> SubModules;
Index: clang/include/clang/AST/DeclBase.h
===================================================================
--- clang/include/clang/AST/DeclBase.h
+++ clang/include/clang/AST/DeclBase.h
@@ -18,6 +18,7 @@
 #include "clang/AST/DeclarationName.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/Module.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/ArrayRef.h"
@@ -231,8 +232,15 @@
     /// module is imported.
     VisibleWhenImported,
 
+    /// This declaration has an owninig module, and is visible to lookups
+    /// that occurs within that module. And it is reachable to other module
+    /// when the owninig module is imported.
+    ReachableWhenImported,
+
     /// This declaration has an owning module, but is only visible to
     /// lookups that occur within that module.
+    /// The discarded declarations in global module fragment belongs
+    /// to this group too.
     ModulePrivate
   };
 
@@ -241,8 +249,8 @@
   /// DeclContext. These pointers form the linked list that is
   /// traversed via DeclContext's decls_begin()/decls_end().
   ///
-  /// The extra two bits are used for the ModuleOwnershipKind.
-  llvm::PointerIntPair<Decl *, 2, ModuleOwnershipKind> NextInContextAndBits;
+  /// The extra three bits are used for the ModuleOwnershipKind.
+  llvm::PointerIntPair<Decl *, 3, ModuleOwnershipKind> NextInContextAndBits;
 
 private:
   friend class DeclContext;
@@ -613,6 +621,19 @@
     return getModuleOwnershipKind() == ModuleOwnershipKind::ModulePrivate;
   }
 
+  bool isInvisibleOutsideTheOwningModule() const {
+    return getModuleOwnershipKind() > ModuleOwnershipKind::VisibleWhenImported;
+  }
+
+  bool isInGlobalModuleFragment() const {
+    Module *M = getOwningModule();
+    return M ? M->isGlobalModule() : false;
+  }
+
+  /// FIXME: Implement discarding declarations actually in global module
+  /// fragment. See [module.global.frag]p3,4 for details.
+  bool isDiscardedInGlobalModuleFragment() const { return false; }
+
   /// Return true if this declaration has an attribute which acts as
   /// definition of the entity, such as 'alias' or 'ifunc'.
   bool hasDefiningAttr() const;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to