[clang] c5cb111 - [Modules] Handle the visibility of GMF during the template instantiation

Chuanqi Xu via cfe-commits cfe-commits at lists.llvm.org
Sun Feb 19 22:20:58 PST 2023


Author: Chuanqi Xu
Date: 2023-02-20T14:19:50+08:00
New Revision: c5cb1117e19273a26499a1e18770b74bdf3b9ade

URL: https://github.com/llvm/llvm-project/commit/c5cb1117e19273a26499a1e18770b74bdf3b9ade
DIFF: https://github.com/llvm/llvm-project/commit/c5cb1117e19273a26499a1e18770b74bdf3b9ade.diff

LOG: [Modules] Handle the visibility of GMF during the template instantiation

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.

Added: 
    clang/test/Modules/pr60775.cppm

Modified: 
    clang/lib/Sema/SemaLookup.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index b910974d2ce61..5a5428caf170e 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -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
index 0000000000000..e581a876ab17a
--- /dev/null
+++ b/clang/test/Modules/pr60775.cppm
@@ -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>();
+}


        


More information about the cfe-commits mailing list