Merge pull request #81228 from j-hui/fix-ast-layout-dependent-types

Author: j-hui

benchmark/cxx-source/CxxSetToCollection.swift

@@ -13,8 +13,6 @@
 // This is a benchmark that tracks how quickly Swift can convert a C++ set
 // to a Swift collection.
 
-/* FIXME: rdar://150067288
-
 import TestsUtils
 import CxxStdlibPerformance
 import Cxx
@@ -66,5 +64,3 @@ public func run_CxxSetOfU32_forEach(_ n: Int) {
     }
   }
 }
-
-*/

benchmark/cxx-source/CxxStringConversion.swift

@@ -10,8 +10,6 @@
 //
 //===----------------------------------------------------------------------===//
 
-/* FIXME: rdar://150067288
-
 import TestsUtils
 import CxxStdlibPerformance
 import CxxStdlib
@@ -60,5 +58,3 @@ public func run_cxxToSwift(_ n: Int) {
     blackHole(x)
   }
 }
-
-*/

benchmark/cxx-source/CxxVectorSum.swift

@@ -13,8 +13,6 @@
 // This is a benchmark that tracks how quickly Swift can sum up a C++ vector
 // as compared to the C++ implementation of such sum.
 
-/* FIXME: rdar://150067288
-
 import TestsUtils
 import CxxStdlibPerformance
 import Cxx
@@ -121,5 +119,3 @@ public func run_CxxVectorOfU32_Sum_Swift_Reduce(_ n: Int) {
     }
     blackHole(sum)
 }
-
-*/

lib/ClangImporter/ImportDecl.cpp

@@ -4480,11 +4480,17 @@ namespace {
     }
 
     Decl *VisitFieldDecl(const clang::FieldDecl *decl) {
-      if (decl->hasAttr<clang::NoUniqueAddressAttr>()) {
+      if (!Impl.importSymbolicCXXDecls &&
+          decl->hasAttr<clang::NoUniqueAddressAttr>()) {
         if (const auto *rd = decl->getType()->getAsRecordDecl()) {
           // Clang can store the next field in the padding of this one. Swift
           // does not support this yet so let's not import the field and
           // represent it with an opaque blob in codegen.
+          //
+          // This check is not relevant when importing the decl symbolically
+          // (since that isn't used for codegen). In fact, we need to avoid this
+          // check because symbolic imports can expose us to dependent types
+          // whose ASTRecordLayout cannot be queried.
           const auto &fieldLayout =
               decl->getASTContext().getASTRecordLayout(rd);
           auto &clangCtx = decl->getASTContext();

test/Interop/Cxx/class/Inputs/member-variables.h

@@ -2,6 +2,7 @@
 #define TEST_INTEROP_CXX_CLASS_INPUTS_MEMBER_VARIABLES_H
 
 #include <cstddef>
+#include <type_traits>
 #include <optional>
 
 class MyClass {
@@ -27,25 +28,56 @@ struct HasZeroSizedField {
   void set_c(short c) { this->c = c; }
 };
 
-struct ReuseFieldPadding {
+struct ReuseOptionalFieldPadding {
   [[no_unique_address]] std::optional<int> a = {2};
   char c;
   char get_c() const { return c; }
   void set_c(char c) { this->c = c; }
-  int offset() const { return offsetof(ReuseFieldPadding, c); }
+  int offset() const { return offsetof(ReuseOptionalFieldPadding, c); }
   std::optional<int> getOptional() { return a; }
 };
 
 using OptInt = std::optional<int>;
 
-struct ReuseFieldPaddingWithTypedef {
+struct ReuseOptionalFieldPaddingWithTypedef {
   [[no_unique_address]] OptInt a;
   char c;
   char get_c() const { return c; }
   void set_c(char c) { this->c = c; }
-  int offset() const { return offsetof(ReuseFieldPadding, c); }
+  int offset() const { return offsetof(ReuseOptionalFieldPadding, c); }
 };
 
+// Using a mix of private and public fields prevents this class from being
+// standard-layout, which is necessary to allow clang to reuse its padding.
+template<typename T>
+struct NonStandardLayoutClass {
+private:
+  T x;
+public:
+  char pad_me;
+};
+static_assert(std::is_standard_layout_v<NonStandardLayoutClass<int>> == false);
+
+struct ReuseNonStandardLayoutFieldPadding {
+  [[no_unique_address]] NonStandardLayoutClass<int> a;
+  char c;
+  char get_c() const { return c; }
+  void set_c(char c) { this->c = c; }
+  // C-style implementation of offsetof() to avoid non-standard-layout warning
+  int offset() const { return (char *) &this->c - (char *) this; }
+};
+
+template<typename T>
+struct ReuseDependentFieldPadding {
+  [[no_unique_address]] struct { private: T x; public: char pad_me; } a;
+  char c;
+  char get_c() const { return c; }
+  void set_c(char c) { this->c = c; }
+  // C-style implementation of offsetof() to avoid non-standard-layout warning
+  int offset() const { return (char *) &this->c - (char *) this; }
+};
+
+typedef ReuseDependentFieldPadding<int> ReuseDependentFieldPaddingInt;
 
 inline int takesZeroSizedInCpp(HasZeroSizedField x) {
   return x.a;

test/Interop/Cxx/class/zero-sized-field.swift

@@ -1,4 +1,5 @@
-// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -enable-experimental-cxx-interop -Xcc -std=c++20)
+// RUN: %empty-directory(%t/index)
+// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -enable-experimental-cxx-interop -Xcc -std=c++20 -Xfrontend -index-store-path -Xfrontend %t/index)
 //
 // REQUIRES: executable_test
 
@@ -24,12 +25,12 @@ FieldsTestSuite.test("Zero sized field") {
   expectEqual(s.b.getNum(), 42)
 }
 
-FieldsTestSuite.test("Field padding reused") {
-  var s = ReuseFieldPadding()
+FieldsTestSuite.test("Optional field padding reused") {
+  var s = ReuseOptionalFieldPadding()
   let opt = s.getOptional()
   expectEqual(Int(opt.pointee), 2)
   s.c = 5
-  expectEqual(Int(s.offset()),  MemoryLayout<ReuseFieldPadding>.offset(of: \.c)!)
+  expectEqual(Int(s.offset()),  MemoryLayout<ReuseOptionalFieldPadding>.offset(of: \.c)!)
   expectEqual(s.c, 5)
   expectEqual(s.get_c(), 5)
   s.set_c(6)
@@ -40,10 +41,38 @@ FieldsTestSuite.test("Field padding reused") {
   expectEqual(s2.get_c(), 6)
 }
 
-FieldsTestSuite.test("Typedef'd field padding reused") {
-  var s = ReuseFieldPaddingWithTypedef()
+FieldsTestSuite.test("Typedef'd optional field padding reused") {
+  var s = ReuseOptionalFieldPaddingWithTypedef()
   s.c = 5
-  expectEqual(Int(s.offset()),  MemoryLayout<ReuseFieldPadding>.offset(of: \.c)!)
+  expectEqual(Int(s.offset()),  MemoryLayout<ReuseOptionalFieldPadding>.offset(of: \.c)!)
+  expectEqual(s.c, 5)
+  expectEqual(s.get_c(), 5)
+  s.set_c(6)
+  expectEqual(s.c, 6)
+  expectEqual(s.get_c(), 6)
+  let s2 = s
+  expectEqual(s2.c, 6)
+  expectEqual(s2.get_c(), 6)
+}
+
+FieldsTestSuite.test("Non-standard-layout field padding reused") {
+  var s = ReuseNonStandardLayoutFieldPadding()
+  s.c = 5
+  expectEqual(Int(s.offset()),  MemoryLayout<ReuseNonStandardLayoutFieldPadding>.offset(of: \.c)!)
+  expectEqual(s.c, 5)
+  expectEqual(s.get_c(), 5)
+  s.set_c(6)
+  expectEqual(s.c, 6)
+  expectEqual(s.get_c(), 6)
+  let s2 = s
+  expectEqual(s2.c, 6)
+  expectEqual(s2.get_c(), 6)
+}
+
+FieldsTestSuite.test("Non-standard-layout field padding in templated class reused") {
+  var s = ReuseDependentFieldPaddingInt()
+  s.c = 5
+  expectEqual(Int(s.offset()),  MemoryLayout<ReuseDependentFieldPaddingInt>.offset(of: \.c)!)
   expectEqual(s.c, 5)
   expectEqual(s.get_c(), 5)
   s.set_c(6)