src: refactor `BaseObject` internal field management

- Instead of storing a pointer whose type refers to the specific
  subclass of `BaseObject`, just store a `BaseObject*` directly.
  This means in particular that one can cast to classes along
  the way of the inheritance chain without issues, and that
  `BaseObject*` no longer needs to be the first superclass
  in the case of multiple inheritance.

  In particular, this renders hack-y solutions to this problem (like
  ddc19be6de1ba263d9c175b2760696e7b9918b25) obsolete and addresses
  a `TODO` comment of mine.

- Move wrapping/unwrapping methods to the `BaseObject` class.
  We use these almost exclusively for `BaseObject`s, and I hope
  that this gives a better idea of how (and for what) these are used
  in our code.

- Perform initialization/deinitialization of the internal field
  in the `BaseObject*` constructor/destructor. This makes the code
  a bit more obviously correct, avoids explicit calls for this
  in subclass constructors, and in particular allows us to avoid
  crash situations when we previously called `ClearWrap()`
  during GC.

  This also means that we enforce that the object passed to the
  `BaseObject` constructor needs to have an internal field.
  This is the only reason for the test change.

- Change the signature of `MakeWeak()` to not require a pointer
  argument. Previously, this would always have been the same
  as `this`, and no other value made sense. Also, the parameter
  was something that I personally found somewhat confusing
  when becoming familiar with Node’s code.

- Add a `TODO` comment that motivates switching to real inheritance
  for the JS types we expose from the native side. This patch
  brings us a lot closer to being able to do that.

- Some less significant drive-by cleanup.

Since we *effectively* already store the `BaseObject*` pointer
anyway since ddc19be6de1ba263d9c175b2760696e7b9918b25, I do not
think that this is going to have any impact on diagnostic tooling.

Fixes: https://github.com/nodejs/node/issues/18897
PR-URL: https://github.com/nodejs/node/pull/20455
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>

1 files changed, 0 insertions, 18 deletions

diff --git a/src/util.h b/src/util.h
index c8bad8171e..133899008f 100644
--- a/src/util.h
+++ b/src/util.h
@@ -35,7 +35,6 @@
 #include <string.h>
 
 #include <functional>  // std::function
-#include <type_traits>  // std::remove_reference
 
 namespace node {
 
@@ -84,8 +83,6 @@ NO_RETURN void Abort();
 NO_RETURN void Assert(const char* const (*args)[4]);
 void DumpBacktrace(FILE* fp);
 
-template <typename T> using remove_reference = std::remove_reference<T>;
-
 #define FIXED_ONE_BYTE_STRING(isolate, string)                                \
   (node::OneByteString((isolate), (string), sizeof(string) - 1))
 
@@ -135,14 +132,6 @@ template <typename T> using remove_reference = std::remove_reference<T>;
 
 #define UNREACHABLE() ABORT()
 
-#define ASSIGN_OR_RETURN_UNWRAP(ptr, obj, ...)                                \
-  do {                                                                        \
-    *ptr =                                                                    \
-        Unwrap<typename node::remove_reference<decltype(**ptr)>::type>(obj);  \
-    if (*ptr == nullptr)                                                      \
-      return __VA_ARGS__;                                                     \
-  } while (0)
-
 // TAILQ-style intrusive list node.
 template <typename T>
 class ListNode;
@@ -250,13 +239,6 @@ inline v8::Local<v8::String> OneByteString(v8::Isolate* isolate,
                                            const unsigned char* data,
                                            int length = -1);
 
-inline void Wrap(v8::Local<v8::Object> object, void* pointer);
-
-inline void ClearWrap(v8::Local<v8::Object> object);
-
-template <typename TypeName>
-inline TypeName* Unwrap(v8::Local<v8::Object> object);
-
 // Swaps bytes in place. nbytes is the number of bytes to swap and must be a
 // multiple of the word size (checked by function).
 inline void SwapBytes16(char* data, size_t nbytes);