release/19.x: [C++20] [Modules] Don't insert class not in named modules to PendingEmittingVTables (#106501)

[llvmbot]

Backport 47615ff

Requested by: @ChuanqiXu9

[llvmbot]

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: None (llvmbot)

Changes

Backport 47615ff

Requested by: @ChuanqiXu9

---

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

3 Files Affected:

• (modified) clang/include/clang/Serialization/ASTWriter.h (+2-2)
• (modified) clang/lib/Serialization/ASTWriter.cpp (+3)
• (added) clang/test/Modules/pr106483.cppm (+41)

diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h
index 71a7c28047e318..700f0ad001116b 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -500,8 +500,8 @@ class ASTWriter : public ASTDeserializationListener,
   std::vector<SourceRange> NonAffectingRanges;
   std::vector<SourceLocation::UIntTy> NonAffectingOffsetAdjustments;
 
-  /// A list of classes which need to emit the VTable in the corresponding
-  /// object file.
+  /// A list of classes in named modules which need to emit the VTable in
+  /// the corresponding object file.
   llvm::SmallVector<CXXRecordDecl *> PendingEmittingVTables;
 
   /// Computes input files that didn't affect compilation of the current module,
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 7c0636962459e9..cb63dec92e331d 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -3963,6 +3963,9 @@ void ASTWriter::WriteIdentifierTable(Preprocessor &PP,
 }
 
 void ASTWriter::handleVTable(CXXRecordDecl *RD) {
+  if (!RD->isInNamedModule())
+    return;
+
   PendingEmittingVTables.push_back(RD);
 }
 
diff --git a/clang/test/Modules/pr106483.cppm b/clang/test/Modules/pr106483.cppm
new file mode 100644
index 00000000000000..a19316b9dd50cc
--- /dev/null
+++ b/clang/test/Modules/pr106483.cppm
@@ -0,0 +1,41 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++23 %t/a.cppm -emit-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++23 %t/b.cppm -emit-module-interface -o %t/b.pcm \
+// RUN:   -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++23 -fprebuilt-module-path=%t %t/b.pcm -emit-llvm \
+// RUN:     -disable-llvm-passes -o - | FileCheck %t/b.cppm
+
+//--- a.cppm
+module;
+
+struct base {
+    virtual void f() const;
+};
+
+inline void base::f() const {
+}
+
+export module a;
+export using ::base;
+
+//--- b.cppm
+module;
+
+struct base {
+    virtual void f() const;
+};
+
+inline void base::f() const {
+}
+
+export module b;
+import a;
+export using ::base;
+
+export extern "C" void func() {}
+
+// We only need to check that the IR are successfully emitted instead of crash.
+// CHECK: func

[ChuanqiXu9]

Very sorry that I failed to count #102933 as a regression support initially. I thought it was other problems. My bad. And back to the issue itself, it shows that the fix is pretty simple and it is majorly an oversight in the develop and review process. I think we need to backport this since it introduces a relatively serious regression.

[github-actions]

@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.