From 83de4e174a21b8f38a3f38cf5a78d0e71883e1ff Mon Sep 17 00:00:00 2001
From: Alexander Shabalin <alexander.shabalin@jetbrains.com>
Date: Mon, 23 Nov 2020 10:03:01 +0300
Subject: [PATCH] Keep MemoryState empty. (#4544)

---
 .../runtime/src/main/cpp/CppSupport.hpp       |  2 --
 kotlin-native/runtime/src/main/cpp/Utils.hpp  | 19 ---------------
 .../runtime/src/main/cpp/UtilsTest.cpp        | 24 -------------------
 kotlin-native/runtime/src/mm/cpp/Memory.cpp   | 14 +++++++----
 4 files changed, 9 insertions(+), 50 deletions(-)

diff --git a/kotlin-native/runtime/src/main/cpp/CppSupport.hpp b/kotlin-native/runtime/src/main/cpp/CppSupport.hpp
index e423ab56d6c..9f04f544c41 100644
--- a/kotlin-native/runtime/src/main/cpp/CppSupport.hpp
+++ b/kotlin-native/runtime/src/main/cpp/CppSupport.hpp
@@ -44,8 +44,6 @@ template <typename T>
 constexpr bool is_nothrow_move_constructible_v = std::is_nothrow_move_constructible<T>::value;
 template <typename T>
 constexpr bool is_nothrow_move_assignable_v = std::is_nothrow_move_assignable<T>::value;
-template <typename T>
-constexpr bool is_standard_layout_v = std::is_standard_layout<T>::value;
 
 } // namespace std_support
 } // namespace kotlin
diff --git a/kotlin-native/runtime/src/main/cpp/Utils.hpp b/kotlin-native/runtime/src/main/cpp/Utils.hpp
index 645c2924e88..e6b94f5a2f8 100644
--- a/kotlin-native/runtime/src/main/cpp/Utils.hpp
+++ b/kotlin-native/runtime/src/main/cpp/Utils.hpp
@@ -54,25 +54,6 @@ protected:
     ~Pinned() = default;
 };
 
-// Given
-// struct SomeWrapper {
-//     SomeType value;
-//     ... // (possibly) some other fields
-// };
-// allows to cast from `SomeValue*` to `SomeWrapper*` as no-op. It only works
-// if `SomeWrapper` is standard layout and `value` is the first non-static data member.
-// See https://en.cppreference.com/w/cpp/language/data_members#Standard_layout
-//
-// Useful for exporting SomeType under a different name (e.g. exporting inner C++ class
-// as public C struct).
-#define wrapper_cast(Wrapper, inner, field) \
-    /* With -O2 the lambda is replaced with a cast in the bitcode. */ \
-    [inner]() { \
-        static_assert(std_support::is_standard_layout_v<Wrapper>, #Wrapper " must be standard layout"); \
-        static_assert(offsetof(Wrapper, field) == 0, #field " must be at 0 offset"); \
-        return reinterpret_cast<Wrapper*>(inner); \
-    }()
-
 } // namespace kotlin
 
 #endif // RUNTIME_UTILS_H
diff --git a/kotlin-native/runtime/src/main/cpp/UtilsTest.cpp b/kotlin-native/runtime/src/main/cpp/UtilsTest.cpp
index 78f39f520f7..5ba0aea917c 100644
--- a/kotlin-native/runtime/src/main/cpp/UtilsTest.cpp
+++ b/kotlin-native/runtime/src/main/cpp/UtilsTest.cpp
@@ -48,27 +48,3 @@ TEST(UtilsTest, PinnedImpl) {
     static_assert(!std_support::is_move_assignable_v<PinnedImpl>, "Must not be move assignable");
     static_assert(sizeof(PinnedImpl) == sizeof(A), "Must not increase size");
 }
-
-namespace {
-
-struct Wrapper {
-    A wrapped;
-};
-
-struct WrapperOverPinned {
-    PinnedImpl wrapped;
-};
-
-} // namespace
-
-TEST(UtilsTest, WrapperCast) {
-    A value;
-    Wrapper* wrapper = wrapper_cast(Wrapper, &value, wrapped);
-    EXPECT_EQ(&value, &wrapper->wrapped);
-}
-
-TEST(UtilsTest, WrapperOverPinnedCast) {
-    PinnedImpl value;
-    WrapperOverPinned* wrapper = wrapper_cast(WrapperOverPinned, &value, wrapped);
-    EXPECT_EQ(&value, &wrapper->wrapped);
-}
diff --git a/kotlin-native/runtime/src/mm/cpp/Memory.cpp b/kotlin-native/runtime/src/mm/cpp/Memory.cpp
index be1b12e85a5..dd192ee8404 100644
--- a/kotlin-native/runtime/src/mm/cpp/Memory.cpp
+++ b/kotlin-native/runtime/src/mm/cpp/Memory.cpp
@@ -11,19 +11,23 @@
 
 using namespace kotlin;
 
-extern "C" struct MemoryState {
-    mm::ThreadRegistry::Node data;
-    // Do not add any other fields: this struct is just a wrapper around ThreadDataNode.
+// Delete all means of creating this type directly as it only serves
+// as a typedef for `mm::ThreadRegistry::Node`.
+extern "C" struct MemoryState : Pinned {
+    MemoryState() = delete;
+    ~MemoryState() = delete;
 };
 
 namespace {
 
+// `reinterpret_cast` to it and back to the same type
+// will yield precisely the same pointer, so it's safe.
 ALWAYS_INLINE MemoryState* ToMemoryState(mm::ThreadRegistry::Node* data) {
-    return wrapper_cast(MemoryState, data, data);
+    return reinterpret_cast<MemoryState*>(data);
 }
 
 ALWAYS_INLINE mm::ThreadRegistry::Node* FromMemoryState(MemoryState* state) {
-    return &state->data;
+    return reinterpret_cast<mm::ThreadRegistry::Node*>(state);
 }
 
 } // namespace