Refactoring and bug fixes in the V8 API

Merged
19
2y ago
+1917-1244

What does this PR do?

Refactors and bugfixes in our V8 API implementation. So far:

  • Fix some places that did not handle oddballs (booleans or nullish JS values) properly (EscapableHandleScope, Global)
  • Support NewStringType::kInternalized
    • Previously the type was ignored. Now internalized strings are created with WTF::AtomString.
  • Remove or merge many functions that convert between V8 and JSC values
  • Make v8::Context be a Zig::GlobalObject instead of the same thing as v8::Isolate, which lets us access the global object with less indirection in functions that take a Context
    • side effect: there are now only 2 kinds of V8 handle, for cells and doubles, instead of 3. Context was the only place a void * handle was used.
  • Make v8::Isolate an actual class containing everything v8::Roots used to, and delete v8::Roots, instead of reinterpreting between Isolate and Roots
    • v8::Roots's only purpose was to hold values at the offsets from an Isolate pointer that V8 expects. Now we actually store those values in the Isolate like V8 does.
  • Test case and bug fix for EscapableHandleScope
  • Use m_ on C++ class field names for clarity
  • Move Bun-specific classes (i.e. classes that we use to implement the V8 API, but that don't actually exist in the real V8 API) into the namespace v8::shim.
    • All V8 classes that are kept in a Local (String, ObjectTemplate, etc.) now have zero fields, matching how V8's API works and making it less likely to crash by accessing a member while this is the invalid pointer that we get from V8. The cell pointer stored in the handle is either a built-in type (JSString, JSObject) or a type in v8::shim (v8::shim::InternalFieldObject, v8::shim::Function)
  • Move isosubspace declarations for V8 types next to the ones for other Bun-specific types
  • Support Function::GetName and Value::IsFunction
  • Make the this value received by native functions correct (it should be coerced to an object, or overridden by globalThis if null or undefined, like in JavaScript sloppy mode)
  • Add a ton of static assertions to check that we match important V8 implementation details

How did you verify your code works?

Most of the V8 API is covered by existing tests. A couple new tests were added.