Skip to content

Conversation

@llvmbot
Copy link
Member

@llvmbot llvmbot commented Feb 5, 2025

Backport c5a9a72

Requested by: @ChuanqiXu9

@llvmbot llvmbot added this to the LLVM 20.X Release milestone Feb 5, 2025
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules labels Feb 5, 2025
@llvmbot
Copy link
Member Author

llvmbot commented Feb 5, 2025

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: None (llvmbot)

Changes

Backport c5a9a72

Requested by: @ChuanqiXu9


Full diff: https://github.com/llvm/llvm-project/pull/125809.diff

3 Files Affected:

  • (modified) clang/lib/AST/ASTContext.cpp (+2-1)
  • (modified) clang/lib/Serialization/ASTReaderDecl.cpp (+5)
  • (added) clang/test/Modules/pr125521.cppm (+57)
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index e58091ce95f6258..dbeb3d105ad17c4 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1055,7 +1055,8 @@ void ASTContext::PrintStats() const {
 void ASTContext::mergeDefinitionIntoModule(NamedDecl *ND, Module *M,
                                            bool NotifyListeners) {
   if (NotifyListeners)
-    if (auto *Listener = getASTMutationListener())
+    if (auto *Listener = getASTMutationListener();
+        Listener && !ND->isUnconditionallyVisible())
       Listener->RedefinedHiddenDefinition(ND, M);
 
   MergedDefModules[cast<NamedDecl>(ND->getCanonicalDecl())].push_back(M);
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 8210eb2143acf56..1aa94d5a22abe28 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -3746,6 +3746,11 @@ void ASTDeclReader::checkMultipleDefinitionInNamedModules(ASTReader &Reader,
       Func && Func->getTemplateSpecializationInfo())
     return;
 
+  // The module ownership of in-class friend declaration is not straightforward.
+  // Avoid diagnosing such cases.
+  if (D->getFriendObjectKind() || Previous->getFriendObjectKind())
+    return;
+
   Module *M = Previous->getOwningModule();
   if (!M)
     return;
diff --git a/clang/test/Modules/pr125521.cppm b/clang/test/Modules/pr125521.cppm
new file mode 100644
index 000000000000000..d064cdfe3eb73bb
--- /dev/null
+++ b/clang/test/Modules/pr125521.cppm
@@ -0,0 +1,57 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/mod2.cppm -emit-module-interface -o %t/mod2.pcm
+// RUN: %clang_cc1 -std=c++20 %t/mod1.cppm -emit-module-interface -o %t/mod1.pcm \
+// RUN:     -fmodule-file=Mod2=%t/mod2.pcm
+// RUN: %clang_cc1 -std=c++20 %t/test.cc -fmodule-file=Mod2=%t/mod2.pcm -fmodule-file=Mod=%t/mod1.pcm \
+// RUN:     -fsyntax-only -verify
+
+// RUN: %clang_cc1 -std=c++20 %t/mod2.cppm -emit-module-interface -o %t/mod2.pcm
+// RUN: %clang_cc1 -std=c++20 %t/mod1.cppm -emit-module-interface -o %t/mod1.pcm \
+// RUN:     -fmodule-file=Mod2=%t/mod2.pcm
+// RUN: %clang_cc1 -std=c++20 %t/mod1.pcm  -fmodule-file=Mod2=%t/mod2.pcm -emit-llvm -o - \
+// RUN:     | FileCheck %t/mod1.cppm
+
+//--- hello.h
+template <typename V> int get() noexcept {return 0;};
+
+template <typename T>
+class List
+{
+    template <typename V> friend int get() noexcept;
+};
+
+//--- mod2.cppm
+module;
+#include "hello.h"
+export module Mod2;
+export const char *modFn2() {
+    List<int> a;
+    return "hello";
+}
+
+//--- mod1.cppm
+module;
+#include "hello.h"
+export module Mod;
+import Mod2;
+export extern "C" const char *modFn() {
+    List<int> a;
+    List<double> b;
+    return modFn2();
+}
+
+// Fine enough to check it won't crash.
+// CHECK: define {{.*}}@modFn
+
+//--- test.cc
+// expected-no-diagnostics
+import Mod;
+import Mod2;
+
+void test() {
+    modFn();
+    modFn2();
+}

…en modules incorrectly

Close llvm#125521

We shouldn't use the ownership information for friend declarations to do
anything.

(cherry picked from commit c5a9a72)
@tstellar tstellar merged commit 0d363c3 into llvm:release/20.x Feb 10, 2025
7 of 10 checks passed
@github-actions
Copy link

@ChuanqiXu9 (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category

Projects

Development

Successfully merging this pull request may close these issues.

3 participants