[Modules] Handle the visibility of GMF during the template instantiation
authorChuanqi Xu <yedeng.yd@linux.alibaba.com>
Mon, 20 Feb 2023 05:58:28 +0000 (13:58 +0800)
committerChuanqi Xu <yedeng.yd@linux.alibaba.com>
Mon, 20 Feb 2023 06:19:50 +0000 (14:19 +0800)
Close https://github.com/llvm/llvm-project/issues/60775

Previously, we will mark all the declarations in the GMF as not visible
to other module units. But this is too strict and the users may meet
problems during the template instantiation like the above exampel shows.
The patch addresseds the problem.

clang/lib/Sema/SemaLookup.cpp
clang/test/Modules/pr60775.cppm [new file with mode: 0644]

index b910974..5a5428c 100644 (file)
@@ -1858,19 +1858,6 @@ bool LookupResult::isAcceptableSlow(Sema &SemaRef, NamedDecl *D,
 }
 
 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. So here we
-  // check if the Module is the global module fragment in current translation
-  // unit.
-  if (M->isGlobalModule() && M != this->GlobalModuleFragment)
-    return false;
-
   // The module might be ordinarily visible. For a module-private query, that
   // means it is part of the current module.
   if (ModulePrivate && isUsableModule(M))
@@ -1889,6 +1876,12 @@ bool Sema::isModuleVisible(const Module *M, bool ModulePrivate) {
   if (LookupModules.empty())
     return false;
 
+  // The global module fragments are visible to its corresponding module unit.
+  // So the global module fragment should be visible if the its corresponding
+  // module unit is visible.
+  if (M->isGlobalModule())
+    M = M->getTopLevelModule();
+
   // If our lookup set contains the module, it's visible.
   if (LookupModules.count(M))
     return true;
@@ -3888,40 +3881,8 @@ void Sema::ArgumentDependentLookup(DeclarationName Name, SourceLocation Loc,
           if (!getLangOpts().CPlusPlusModules)
             continue;
 
-          // A partial mock implementation for instantiation context for
-          // modules. [module.context]p1:
-          //    The instantiation context is a set of points within the program
-          //    that determines which declarations are found by
-          //    argument-dependent name lookup...
-          //
-          // FIXME: This didn't implement [module.context] well. For example,
-          // [module.context]p3 says:
-          //    During the implicit instantiation of a template whose point of
-          //    instantiation is specified as that of an enclosing
-          //    specialization if the template is defined in a module interface
-          //    unit of a module M and the point of instantiation is not in a
-          //    module interface unit of M, the point at the end of the
-          //    declaration-seq of the primary module interface unit of M.
-          //
-          // But we didn't check if the template is defined in a module
-          // interface unit nor we check the context for the primary module
-          // interface.
-          Module *FM = D->getOwningModule();
-          if (FM && !FM->isHeaderLikeModule()) {
-            // LookupModules will contain all the modules we visited during the
-            // template instantiation (if any). And if LookupModules contains
-            // FM, the declarations in FM should be visible for the
-            // instantiation.
-            const auto &LookupModules = getLookupModules();
-            // Note: We can't use 'Module::isModuleVisible()' since that is for
-            // clang modules.
-            if (LookupModules.count(FM->getTopLevelModule())) {
-              Visible = true;
-              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
diff --git a/clang/test/Modules/pr60775.cppm b/clang/test/Modules/pr60775.cppm
new file mode 100644 (file)
index 0000000..e581a87
--- /dev/null
@@ -0,0 +1,97 @@
+// https://github.com/llvm/llvm-project/issues/60775
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/a.cppm -I%t -emit-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 %t/b.cpp -fmodule-file=a=%t/a.pcm -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 %t/c.cppm -I%t -emit-module-interface -o %t/c.pcm
+// RUN: %clang_cc1 -std=c++20 %t/d.cppm -emit-module-interface -fmodule-file=c=%t/c.pcm -o %t/d.pcm
+// RUN: %clang_cc1 -std=c++20 %t/e.cpp -fmodule-file=d=%t/d.pcm -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 %t/f.cppm -emit-module-interface -fmodule-file=c=%t/c.pcm -o %t/f.pcm
+// RUN: %clang_cc1 -std=c++20 %t/g.cpp -fmodule-file=f=%t/f.pcm -verify -fsyntax-only
+
+//--- initializer_list.h
+namespace std {
+  typedef decltype(sizeof(int)) size_t;
+  template<typename T> struct initializer_list {
+    initializer_list(const T *, size_t);
+    T* begin();
+    T* end();
+  };
+}
+
+//--- a.cppm
+module;
+#include "initializer_list.h"
+export module a;
+export template<typename>
+void a() {
+       for (int x : {0}) {
+       }
+}
+
+//--- b.cpp
+// expected-no-diagnostics
+import a;
+void b() {
+       a<int>();
+}
+
+//--- c.cppm
+module;
+#include "initializer_list.h"
+export module c;
+namespace std {
+    export using std::initializer_list;
+}
+
+//--- d.cppm
+export module d;
+import c;
+export template<typename>
+void d() {
+       for (int x : {0}) {
+       }
+}
+
+//--- e.cpp
+import d;
+void e() {
+    for (int x : {0}) { // expected-error {{cannot deduce type of initializer list because std::initializer_list was not found; include <initializer_list>}}
+       }
+}
+
+template <typename>
+void ee() {
+    for (int x : {0}) { // expected-error {{cannot deduce type of initializer list because std::initializer_list was not found; include <initializer_list>}}
+    }
+}
+
+void eee() {
+    ee<int>();
+    d<int>();
+}
+
+//--- f.cppm
+export module f;
+export import c;
+
+//--- g.cpp
+// expected-no-diagnostics
+import f;
+void g() {
+    for (int x : {0}) {
+       }
+}
+
+template <typename>
+void gg() {
+    for (int x : {0}) {
+    }
+}
+
+void ggg() {
+    gg<int>();
+}