[clang] ccf7191 - [Modules] Do not rewrite existing decls when deserializing class fields

[Ilya Biryukov via cfe-commits]

Author: Ilya Biryukov
Date: 2023-04-18T12:40:39+02:00
New Revision: ccf7191719375cac15da5a3eb406e1efa602b096

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

LOG: [Modules] Do not rewrite existing decls when deserializing class fields

Classes can have implicit members that were added before fields were
deserialized. These members were previously silently removed from
`decls()` when fields were deserialized after them.

This was the root cause of a compilation error exposed in
bc95f27337c7ed77c28e713c855272848f01802a, added a test for it.

Reviewed By: ChuanqiXu

Differential Revision: https://reviews.llvm.org/D148515

Added: 
    clang/test/Modules/pr61065_2.cppm

Modified: 
    clang/lib/AST/Decl.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index bf41262f0641b..b728917fdda6d 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4886,8 +4886,13 @@ void RecordDecl::LoadFieldsFromExternalStorage() const {
   if (Decls.empty())
     return;
 
-  std::tie(FirstDecl, LastDecl) = BuildDeclChain(Decls,
-                                                 /*FieldsAlreadyLoaded=*/false);
+  auto [ExternalFirst, ExternalLast] =
+      BuildDeclChain(Decls,
+                     /*FieldsAlreadyLoaded=*/false);
+  ExternalLast->NextInContextAndBits.setPointer(FirstDecl);
+  FirstDecl = ExternalFirst;
+  if (!LastDecl)
+    LastDecl = ExternalLast;
 }
 
 bool RecordDecl::mayInsertExtraPadding(bool EmitRemark) const {

diff  --git a/clang/test/Modules/pr61065_2.cppm b/clang/test/Modules/pr61065_2.cppm
new file mode 100644
index 0000000000000..10cc1a06b7e45
--- /dev/null
+++ b/clang/test/Modules/pr61065_2.cppm
@@ -0,0 +1,71 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %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 -emit-module-interface -o %t/b.pcm \
+// RUN:     -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/c.cppm -emit-module-interface -o %t/c.pcm \
+// RUN:     -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/d.cppm -emit-module-interface -o %t/d.pcm \
+// RUN:     -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/e.cpp -fsyntax-only -verify -fprebuilt-module-path=%t
+
+//--- a.cppm
+export module a;
+
+struct WithCtor {
+  WithCtor();
+};
+
+export template <typename T>
+struct Getter {
+  union {
+    WithCtor container;
+  };
+};
+
+//--- b.cppm
+export module b;
+
+import a;
+
+export template <typename T>
+class AnySpan {
+ public:
+  AnySpan();
+  AnySpan(Getter<T> getter)
+      : getter_(getter) {}
+
+ private:
+  Getter<T> getter_;
+};
+
+//--- c.cppm
+export module c;
+import b;
+
+export inline void RegisterInt322(
+   AnySpan<const int> sibling_field_nums) {
+  sibling_field_nums = sibling_field_nums;
+}
+
+//--- d.cppm
+// expected-no-diagnostics
+export module d;
+import c;
+import b;
+
+export inline void RegisterInt32(
+   AnySpan<const int> sibling_field_nums = {}) {
+  sibling_field_nums = sibling_field_nums;
+}
+
+//--- e.cpp
+import d;
+import b;
+
+// expected-no-diagnostics
+void foo(AnySpan<const int> s) {
+  s = AnySpan<const int>(s);
+}