TaggedPointer assertions and refactoring

src/bun.js/bindings/v8/V8Data.h

@@ -18,7 +18,7 @@ class Data {
     JSC::JSCell* localToCell()
     {
         TaggedPointer root = localToTagged();
-        RELEASE_ASSERT(root.type() != TaggedPointer::Type::Smi);
+        RELEASE_ASSERT(root.tag() != TaggedPointer::Tag::Smi);
         return root.getPtr<shim::ObjectLayout>()->asCell();
     }
 
@@ -44,7 +44,7 @@ class Data {
     JSC::JSValue localToJSValue() const
     {
         TaggedPointer root = localToTagged();
-        if (root.type() == TaggedPointer::Type::Smi) {
+        if (root.tag() == TaggedPointer::Tag::Smi) {
             return JSC::jsNumber(root.getSmiUnchecked());
         } else {
             using shim::InstanceType;
@@ -65,7 +65,7 @@ class Data {
     const JSC::JSCell* localToCell() const
     {
         TaggedPointer root = localToTagged();
-        RELEASE_ASSERT(root.type() != TaggedPointer::Type::Smi);
+        RELEASE_ASSERT(root.tag() != TaggedPointer::Tag::Smi);
         return root.getPtr<shim::ObjectLayout>()->asCell();
     }
 

src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp

@@ -24,7 +24,7 @@ uintptr_t* EscapableHandleScopeBase::EscapeSlot(uintptr_t* escape_value)
         m_isolate,
         m_escapeSlot);
     m_escapeSlot = nullptr;
-    return newHandle->asRawPtr();
+    return newHandle->asRawPtrLocation();
 }
 
 } // namespace v8

src/bun.js/bindings/v8/V8HandleScope.cpp

@@ -32,7 +32,7 @@ uintptr_t* HandleScope::CreateHandle(internal::Isolate* i_isolate, uintptr_t val
     auto* handleScope = isolate->globalInternals()->currentHandleScope();
     TaggedPointer* newSlot = handleScope->m_buffer->createHandleFromExistingObject(TaggedPointer::fromRaw(value), isolate);
     // basically a reinterpret
-    return newSlot->asRawPtr();
+    return newSlot->asRawPtrLocation();
 }
 
 } // namespace v8

src/bun.js/bindings/v8/real_v8.h

@@ -5,6 +5,11 @@
 // files from Bun's V8 implementation, and it should only be included in source files (not
 // header files).
 
+// Microsoft's C++ headers cause a compiler error if `private` has been redefined, like we do below.
+// These are all the standard library headers included by V8. We include them here so that they
+// are included while `private` is not redefined yet, and then when V8 includes them they will get
+// skipped by include guards.
+
 #include <array>
 #include <atomic>
 #include <bit>

src/bun.js/bindings/v8/shim/FunctionTemplate.cpp

@@ -98,7 +98,7 @@ JSC::EncodedJSValue FunctionTemplate::functionCall(JSC::JSGlobalObject* globalOb
 
     functionTemplate->m_callback(info);
 
-    if (implicit_args.return_value.type() != TaggedPointer::Type::Smi && implicit_args.return_value.getPtr() == nullptr) {
+    if (implicit_args.return_value.isEmpty()) {
         // callback forgot to set a return value, so return undefined
         return JSValue::encode(JSC::jsUndefined());
     } else {

src/bun.js/bindings/v8/shim/Handle.cpp

@@ -41,7 +41,7 @@ Handle::Handle(const ObjectLayout* that)
 Handle& Handle::operator=(const Handle& that)
 {
     m_object = that.m_object;
-    if (that.m_toV8Object.type() == TaggedPointer::Type::Smi) {
+    if (that.m_toV8Object.tag() == TaggedPointer::Tag::Smi) {
         m_toV8Object = that.m_toV8Object;
     } else {
         m_toV8Object = &this->m_object;

src/bun.js/bindings/v8/shim/Handle.h

@@ -75,7 +75,7 @@ struct Handle {
 
     bool isCell() const
     {
-        if (m_toV8Object.type() == TaggedPointer::Type::Smi) {
+        if (m_toV8Object.tag() == TaggedPointer::Tag::Smi) {
             return false;
         }
         const Map* map_ptr = m_object.map();

src/bun.js/bindings/v8/shim/TaggedPointer.cpp

@@ -0,0 +1,8 @@
+#include "TaggedPointer.h"
+#include "../real_v8.h"
+
+static_assert(sizeof(v8::shim::TaggedPointer) == sizeof(real_v8::internal::Address),
+    "TaggedPointer has wrong size");
+
+static_assert(alignof(v8::shim::TaggedPointer) == alignof(real_v8::internal::Address),
+    "TaggedPointer has wrong alignment");

src/bun.js/bindings/v8/shim/TaggedPointer.h

@@ -15,26 +15,30 @@ struct TaggedPointer {
     uintptr_t m_value;
 
 public:
-    enum class Type : uint8_t {
-        Smi,
-        StrongPointer,
-        WeakPointer,
+    enum class Tag : uint8_t {
+        Smi = 0,
+        StrongPointer = 1,
+        WeakPointer = 3,
     };
 
+    static constexpr uintptr_t TagMask = 0b11;
+
+    // empty
     TaggedPointer()
         : TaggedPointer(nullptr) {};
     TaggedPointer(const TaggedPointer&) = default;
     TaggedPointer& operator=(const TaggedPointer&) = default;
     bool operator==(const TaggedPointer& other) const { return m_value == other.m_value; }
 
     TaggedPointer(void* ptr, bool weak = false)
-        : m_value(reinterpret_cast<uintptr_t>(ptr) | (weak ? 3 : 1))
+        : m_value(reinterpret_cast<uintptr_t>(ptr) | static_cast<uintptr_t>(weak ? Tag::WeakPointer : Tag::StrongPointer))
     {
-        RELEASE_ASSERT((reinterpret_cast<uintptr_t>(ptr) & 3) == 0);
+        // check original pointer was aligned
+        RELEASE_ASSERT((reinterpret_cast<uintptr_t>(ptr) & TagMask) == 0);
     }
 
     TaggedPointer(int32_t smi)
-        : m_value(static_cast<uintptr_t>(smi) << 32)
+        : m_value((static_cast<uintptr_t>(smi) << 32) | static_cast<uintptr_t>(Tag::Smi))
     {
     }
 
@@ -46,37 +50,34 @@ struct TaggedPointer {
         return tagged;
     }
 
-    // Get a pointer to where this TaggedPointer is located
-    uintptr_t* asRawPtr()
+    bool isEmpty() const
+    {
+        return *this == TaggedPointer();
+    }
+
+    // Get a pointer to where this TaggedPointer is located (use ->asRawPtrLocation() to reinterpret
+    // TaggedPointer* as uintptr_t*)
+    uintptr_t* asRawPtrLocation()
     {
         return &m_value;
     }
 
-    Type type() const
+    Tag tag() const
     {
-        switch (m_value & 3) {
-        case 0:
-            return Type::Smi;
-        case 1:
-            return Type::StrongPointer;
-        case 3:
-            return Type::WeakPointer;
-        default:
-            RELEASE_ASSERT_NOT_REACHED();
-        }
+        return static_cast<Tag>(m_value & TagMask);
     }
 
     template<typename T = JSC::JSCell> T* getPtr() const
     {
-        if (type() == Type::Smi) {
+        if (tag() == Tag::Smi) {
             return nullptr;
         }
-        return reinterpret_cast<T*>(m_value & ~3ull);
+        return reinterpret_cast<T*>(m_value & ~TagMask);
     }
 
     bool getSmi(int32_t& smi) const
     {
-        if (type() != Type::Smi) {
+        if (tag() != Tag::Smi) {
             return false;
         }
         smi = static_cast<int32_t>(m_value >> 32);
@@ -85,7 +86,7 @@ struct TaggedPointer {
 
     int32_t getSmiUnchecked() const
     {
-        ASSERT(type() == Type::Smi);
+        ASSERT(tag() == Tag::Smi);
         return static_cast<int32_t>(m_value >> 32);
     }
 };

src/bun.js/bindings/v8/v8_api_internal.cpp

@@ -17,7 +17,7 @@ uintptr_t* GlobalizeReference(internal::Isolate* i_isolate, uintptr_t address)
     auto* isolate = reinterpret_cast<Isolate*>(i_isolate);
     auto* globalHandles = isolate->globalInternals()->globalHandles();
     TaggedPointer* newSlot = globalHandles->createHandleFromExistingObject(TaggedPointer::fromRaw(address), isolate);
-    return newSlot->asRawPtr();
+    return newSlot->asRawPtrLocation();
 }
 
 void DisposeGlobal(uintptr_t* location)