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

[ilya-biryukov]

Summary

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.

[ilya-biryukov]

I plan to land this together with a reland of bc95f27337c7ed77c28e713c855272848f01802a and finally close GH61065.
I have checked this works on previously broken targets. @ChuanqiXu sorry for taking so long to track it down, reproducing module-related errors is quite hard in our setup.

[ChuanqiXu]

Thank you very much!

[ilya-biryukov]

• clang-format the code

clang/lib/AST/Decl.cpp

#ifndef NDEBUG
   // Check that all decls we got were FieldDecls.
   for (unsigned i=0, e=Decls.size(); i != e; ++i)
     assert(isa<FieldDecl>(Decls[i]) || isa<IndirectFieldDecl>(Decls[i]));
 #endif
 
   if (Decls.empty())
     return;
 
-  std::tie(FirstDecl, LastDecl) = BuildDeclChain(Decls,
-                                                 /*FieldsAlreadyLoaded=*/false);
+  auto [ExternalFirst, ExternalLast] =
+      BuildDeclChain(Decls,
+  ExternalLast->NextInContextAndBits.setPointer(FirstDecl);
+  FirstDecl = ExternalFirst;
+  if (!LastDecl)
+    LastDecl = ExternalLast;
 }
 
 bool RecordDecl::mayInsertExtraPadding(bool EmitRemark) const {
   ASTContext &Context = getASTContext();
   const SanitizerMask EnabledAsanMask = Context.getLangOpts().Sanitize.Mask &
       (SanitizerKind::Address | SanitizerKind::KernelAddress);
   if (!EnabledAsanMask || !Context.getLangOpts().SanitizeAddressFieldPadding)
     return false;

clang/test/Modules/pr61065_2.cppm

+// 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);
+}