[clang] 60a3784 - [C++20] [Modules] Handle modules visible relationship properly

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2023-05-09T18:36:37+08:00
New Revision: 60a3784b3e1f0e65e2a0e6daf070bbc7cd4bbab5

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

LOG: [C++20] [Modules] Handle modules visible relationship properly

Close https://github.com/llvm/llvm-project/issues/62589.

Previously, for global module fragments, we will justify if it is
visible by the visibility of their top level parents. But this is an
overkill, it is problematic if we have a deduction guide in the global
module fragments. See the attached test for example. In this example, we
will mark the global module fragments as visible, but our old strategy
will miss the case surprisingly due to we will only search for the top
level modules.

Added: 
    clang/test/Modules/named-modules-adl-3.cppm
    clang/test/Modules/pr62589.cppm

Modified: 
    clang/lib/Sema/SemaLookup.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index e82c17cfe630f..202e5f49ac4a1 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -1877,14 +1877,14 @@ bool Sema::isModuleVisible(const Module *M, bool ModulePrivate) {
   if (LookupModules.empty())
     return false;
 
+  // If our lookup set contains the module, it's visible.
+  if (LookupModules.count(M))
+    return true;
+
   // 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))
+  if (M->isGlobalModule() && LookupModules.count(M->getTopLevelModule()))
     return true;
 
   // For a module-private query, that's everywhere we get to look.

diff  --git a/clang/test/Modules/named-modules-adl-3.cppm b/clang/test/Modules/named-modules-adl-3.cppm
new file mode 100644
index 0000000000000..2fc2962c926b1
--- /dev/null
+++ b/clang/test/Modules/named-modules-adl-3.cppm
@@ -0,0 +1,64 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 %t/b.cppm -fmodule-file=a=%t/a.pcm  -emit-module-interface \
+// RUN:     -o %t/b.pcm
+// RUN: %clang_cc1 -std=c++20 %t/c.cppm -fmodule-file=a=%t/a.pcm -fmodule-file=b=%t/b.pcm \
+// RUN:     -fsyntax-only -verify
+//
+// RUN: %clang_cc1 -std=c++20 -DEXPORT_OPERATOR %t/a.cppm -emit-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 -DEXPORT_OPERATOR %t/b.cppm -fmodule-file=a=%t/a.pcm  \
+// RUN:     -emit-module-interface -o %t/b.pcm
+// RUN: %clang_cc1 -std=c++20 -DEXPORT_OPERATOR %t/c.cppm -fmodule-file=a=%t/a.pcm \
+// RUN:     -fmodule-file=b=%t/b.pcm -fsyntax-only -verify
+
+//--- foo.h
+namespace n {
+
+struct s { };
+
+void operator+(s, int) {}
+
+} // namespace n
+
+//--- a.cppm
+module;
+#include "foo.h"
+export module a;
+export namespace n {
+    using n::s;
+#ifdef EXPORT_OPERATOR
+    using n::operator+;
+#endif
+}
+
+//--- b.cppm
+export module b;
+export import a;
+
+export template<typename T>
+void b(T x) {
+	n::s() + x;
+}
+
+//--- c.cppm
+#ifdef EXPORT_OPERATOR
+// expected-no-diagnostics
+#endif
+export module c;
+import b;
+
+void c(int x) {
+#ifndef EXPORT_OPERATOR
+	// expected-error at b.cppm:6 {{invalid operands to binary expression ('n::s' and 'int')}}
+    // expected-note at +2 {{in instantiation of function template specialization 'b<int>' requested here}}
+#endif
+    b(0);
+
+#ifndef EXPORT_OPERATOR
+    // expected-error at +2 {{invalid operands to binary expression ('n::s' and 'int')}}
+#endif
+    n::s() + x;
+}

diff  --git a/clang/test/Modules/pr62589.cppm b/clang/test/Modules/pr62589.cppm
new file mode 100644
index 0000000000000..4164c3405ac0e
--- /dev/null
+++ b/clang/test/Modules/pr62589.cppm
@@ -0,0 +1,79 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++23 -emit-module-interface %t/a.cppm -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++23 %t/b.cpp -fmodule-file=a=%t/a.pcm -fsyntax-only -verify
+
+//--- foo.h
+class TypeA {};
+
+template<class _Tp, class _Up>
+concept __comparable = requires (_Tp &&__t, _Up &&__u) {
+    __t == __u;
+};
+
+namespace ranges {
+namespace __end {
+  template <class _Tp>
+  concept __member_end =
+    requires(_Tp&& __t) {
+        { __t.end() } -> __comparable<TypeA>;
+    };
+
+  struct __fn {
+    template <class _Tp>
+      requires __member_end<_Tp>
+    constexpr auto operator()(_Tp&& __t) const
+    {
+      return true;
+    }
+
+    void operator()(auto&&) const = delete;
+  };
+}
+
+inline namespace __cpo {
+  inline constexpr auto end = __end::__fn{};
+}
+}
+
+template <class _Tp>
+concept range = requires(_Tp& __t) {
+    ranges::end(__t);
+};
+
+template <class T>
+class a {
+public:
+    a(T*) {}
+    TypeA end() { return {}; }
+};
+
+template <class T>
+class a_view {
+public:
+    template <class U>
+    a_view(a<U>) {}
+};
+template <range _Range>
+a_view(_Range) -> a_view<int>;
+
+constexpr bool operator==(TypeA, TypeA) {
+    return true;
+}
+
+//--- a.cppm
+module;
+#include "foo.h"
+export module a;
+export using ::a;
+export using ::a_view;
+
+//--- b.cpp
+// expected-no-diagnostics
+import a;
+void use() {
+    auto _ = a{"char"};
+    auto __ = a_view{_};
+}