[clang] ab5a5a9 - [C++20] [Modules] Fix incorrect diagnostic for using befriend target

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2025-08-14T14:23:14+08:00
New Revision: ab5a5a90c03d25392fcc486a8c587d0dd9b7a0c6

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

LOG: [C++20] [Modules] Fix incorrect diagnostic for using befriend target

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

The compiler failed to understand the redeclaration-relationship when
performing checks when MergeFunctionDecl. This seemed to be a complex
circular problem (how can we know the redeclaration relationship before
performing merging?). But the fix seems to be easy and safe. It is fine
to only perform the check only if the using decl is a local decl.

Added: 
    clang/test/Modules/befriend-2.cppm
    clang/test/Modules/befriend-3.cppm
    clang/test/Modules/pr138558.cppm

Modified: 
    clang/lib/Sema/SemaDecl.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index cb59782b83304..6581d4c604eb2 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -3653,7 +3653,9 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD, Scope *S,
   FunctionDecl *Old = OldD->getAsFunction();
   if (!Old) {
     if (UsingShadowDecl *Shadow = dyn_cast<UsingShadowDecl>(OldD)) {
-      if (New->getFriendObjectKind()) {
+      // We don't need to check the using friend pattern from other module unit
+      // since we should have diagnosed such cases in its unit already.
+      if (New->getFriendObjectKind() && !OldD->isInAnotherModuleUnit()) {
         Diag(New->getLocation(), diag::err_using_decl_friend);
         Diag(Shadow->getTargetDecl()->getLocation(),
              diag::note_using_decl_target);

diff  --git a/clang/test/Modules/befriend-2.cppm b/clang/test/Modules/befriend-2.cppm
new file mode 100644
index 0000000000000..9d0baf849cad4
--- /dev/null
+++ b/clang/test/Modules/befriend-2.cppm
@@ -0,0 +1,65 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-reduced-module-interface -o %t/test-A.pcm
+// RUN: %clang_cc1 -std=c++20 %t/N.cppm -emit-reduced-module-interface -o %t/test-N.pcm
+// RUN: %clang_cc1 -std=c++20 %t/B.cppm -verify -fsyntax-only -fprebuilt-module-path=%t
+
+//--- a.h
+namespace N {
+
+    template <typename>
+    class C {
+    template <typename> friend void foo();
+    };
+
+    template <typename> void foo() {}
+} // namespace N
+
+//--- a.cppm
+// This is some unrelated file. It also #includes system headers, but
+// here does not even export anything.
+module;
+#include "a.h"
+export module test:A;
+export {
+    using N::C;
+    using N::foo;
+}
+
+//--- std.h
+// Declarations typically #included from C++ header files:
+namespace N {               // In practice, this would be namespace std
+    inline namespace impl {   // In practice, this would be namespace __1
+        template <typename>
+        class C {
+        template <typename> friend void foo();
+        };
+    
+        template <typename> void foo() {}
+    } // namespace impl
+    } // namespace N
+
+//--- N.cppm
+module;
+#include "std.h"
+export module test:N;
+
+// Now wrap these names into a module and export them:
+export {
+    namespace N   {
+        using N::C;
+        using N::foo;
+    }
+}
+
+//--- B.cppm
+// expected-no-diagnostics
+// A file that consumes the partitions from the other two files,
+// including the exported N::C name.
+module test:B;
+import :N;
+import :A;
+
+N::C<int> x;

diff  --git a/clang/test/Modules/befriend-3.cppm b/clang/test/Modules/befriend-3.cppm
new file mode 100644
index 0000000000000..f8dbc423be2ca
--- /dev/null
+++ b/clang/test/Modules/befriend-3.cppm
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -std=c++20 %s -fsyntax-only -verify
+export module m;
+
+namespace test {
+namespace ns1 {
+    namespace ns2 {
+    template<class T> void f(T t); // expected-note {{target of using declaration}}
+    }
+    using ns2::f; // expected-note {{using declaration}}
+}
+struct A { void f(); }; // expected-note 2{{target of using declaration}}
+struct B : public A { using A::f; }; // expected-note {{using declaration}}
+template<typename T> struct C : A { using A::f; }; // expected-note {{using declaration}}
+struct X {
+    template<class T> friend void ns1::f(T t); // expected-error {{cannot befriend target of using declaration}}
+    friend void B::f(); // expected-error {{cannot befriend target of using declaration}}
+    friend void C<int>::f(); // expected-error {{cannot befriend target of using declaration}}
+};
+}

diff  --git a/clang/test/Modules/pr138558.cppm b/clang/test/Modules/pr138558.cppm
new file mode 100644
index 0000000000000..c637ce2f266eb
--- /dev/null
+++ b/clang/test/Modules/pr138558.cppm
@@ -0,0 +1,54 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-reduced-module-interface -o %t/test-A.pcm
+// RUN: %clang_cc1 -std=c++20 %t/N.cppm -emit-reduced-module-interface -o %t/test-N.pcm
+// RUN: %clang_cc1 -std=c++20 %t/B.cppm -verify -fsyntax-only -fprebuilt-module-path=%t
+
+//--- a.h
+namespace N {
+inline namespace impl   {
+    template <typename>
+    class C {
+    template <typename> friend void foo();
+    };
+
+    template <typename> void foo() {}
+} // namespace impl
+} // namespace N
+
+//--- a.cppm
+// This is some unrelated file. It also #includes system headers, but
+// here does not even export anything.
+module;
+#include "a.h"
+export module test:A;
+// To make sure they won't elided.
+using N::C;
+using N::foo;
+
+//--- N.cppm
+module;
+#include "a.h"
+export module test:N;
+
+// Now wrap these names into a module and export them:
+export {
+  namespace N   {
+    inline namespace impl    {
+      using N::impl::C;
+      using N::impl::foo;
+    }
+  }
+}
+
+//--- B.cppm
+// expected-no-diagnostics
+// A file that consumes the partitions from the other two files,
+// including the exported N::C name.
+module test:B;
+import :N;
+import :A;
+
+N::C<int> x;