[Squash] address feedback

nodejs · jasnell · Apr 11, 2020 · Apr 12, 2020 · Apr 13, 2020 · Apr 13, 2020
commit 6cfa7fa23079304dc81951320395210c48f7e0dc
diff --git a/node.gyp b/node.gyp
@@ -632,6 +632,7 @@
         # headers to make for a more pleasant IDE experience
         'src/aliased_buffer.h',
         'src/aliased_struct.h',
+        'src/aliased_struct-inl.h',
         'src/async_wrap.h',
         'src/async_wrap-inl.h',
         'src/base_object.h',

diff --git a/src/aliased_struct-inl.h b/src/aliased_struct-inl.h
@@ -0,0 +1,54 @@
+#ifndef SRC_ALIASED_STRUCT_INL_H_
+#define SRC_ALIASED_STRUCT_INL_H_
+
+#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
+
+#include "aliased_struct.h"
+#include "v8.h"
+#include <memory>
+
+namespace node {
+
+template <typename T>
+template <typename... Args>
+AliasedStruct<T>::AliasedStruct(v8::Isolate* isolate, Args&&... args)
+    : isolate_(isolate) {
+  const v8::HandleScope handle_scope(isolate);
+
+  store_ = v8::ArrayBuffer::NewBackingStore(isolate, sizeof(T));
+  ptr_ = new (store_->Data()) T(std::forward<Args>(args)...);
+  DCHECK_NOT_NULL(ptr_);
+
+  v8::Local<v8::ArrayBuffer> buffer = v8::ArrayBuffer::New(isolate, store_);
+  buffer_ = v8::Global<v8::ArrayBuffer>(isolate, buffer);
+}
+
+template <typename T>
+AliasedStruct<T>::AliasedStruct(const AliasedStruct& that)
+    : AliasedStruct(that.isolate_, *that) {}
+
+template <typename T>
+AliasedStruct<T>& AliasedStruct<T>::operator=(
+    AliasedStruct<T>&& that) noexcept {
+  this->~AliasedStruct();
+  isolate_ = that.isolate_;
+  store_ = that.store_;
+  ptr_ = that.ptr_;
+
+  buffer_ = std::move(that.buffer_);
+
+  that.ptr_ = nullptr;
+  that.store_.reset();
+  return *this;
+}
+
+template <typename T>
+AliasedStruct<T>::~AliasedStruct() {
+  if (ptr_ != nullptr) ptr_->~T();
+}
+
+}  // namespace node
+
+#endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
+
+#endif  // SRC_ALIASED_STRUCT_H_
diff --git a/src/aliased_struct.h b/src/aliased_struct.h
@@ -9,55 +9,45 @@
 
 namespace node {
 
-template <typename T,
-          typename = std::enable_if_t<std::is_object<T>::value>>
-class AliasedStruct {
+// AliasedStruct is a utility that allows uses a V8 Backing Store
+// to be exposed to the C++/C side as a struct and to the
+// JavaScript side as an ArrayBuffer to efficiently share
+// data without marshalling. It is similar in nature to
+// AliasedBuffer.
+//
+//   struct Foo { int x; }
+//
+//   AliasedStruct<Foo> foo;
+//   foo->x = 1;
+//
+//   Local<ArrayBuffer> ab = foo.GetArrayBuffer();
+template <typename T>
+class AliasedStruct final {
  public:
-  explicit AliasedStruct(v8::Isolate* isolate) : isolate_(isolate) {
-    const v8::HandleScope handle_scope(isolate);
+  template <typename... Args>
+  explicit AliasedStruct(v8::Isolate* isolate, Args&&... args);
 
-    store_ = v8::ArrayBuffer::NewBackingStore(isolate, sizeof(T));
-    ptr_ = new (store_->Data()) T;
-    DCHECK_NOT_NULL(ptr_);
+  inline AliasedStruct(const AliasedStruct& that);
 
-    v8::Local<v8::ArrayBuffer> buffer = v8::ArrayBuffer::New(isolate, store_);
-    buffer_ = v8::Global<v8::ArrayBuffer>(isolate, buffer);
-  }
+  inline ~AliasedStruct();
 
-  AliasedStruct(const AliasedStruct& that)
-     : isolate_(that.isolate_),
-       store_(that.store_),
-       ptr_(that.ptr_) {
-    buffer_ = v8::Global<v8::ArrayBuffer>(that.isolate_, that.GetArrayBuffer());
-  }
+  inline AliasedStruct& operator=(AliasedStruct&& that) noexcept;
 
-  ~AliasedStruct() {
-    if (ptr_ != nullptr) ptr_->~T();
+  v8::Local<v8::ArrayBuffer> GetArrayBuffer() const {
+    return buffer_.Get(isolate_);
   }
 
-  AliasedStruct& operator=(AliasedStruct&& that) noexcept {
-    this->~AliasedStruct();
-    isolate_ = that.isolate_;
-    store_ = that.store_;
-    ptr_ = that.ptr_;
+  const T* Data() const { return ptr_; }
 
-    buffer_.Reset(isolate_, that.buffer_.Get(isolate_));
+  T* Data() { return ptr_; }
 
-    that.ptr_ = nullptr;
-    that.store_.reset();
-    that.buffer_.Reset();
-    return *this;
-  }
-
-  v8::Local<v8::ArrayBuffer> GetArrayBuffer() const {
-    return buffer_.Get(isolate_);
-  }
+  const T& operator*() const { return *ptr_; }
 
-  T* Data() const { return ptr_; }
+  T& operator*()  { return *ptr_; }
 
-  T* operator*() const { return ptr_; }
+  const T* operator->() const { return ptr_; }
 
-  T* operator->() const { return ptr_; }
+  T* operator->() { return ptr_; }
 
  private:
   v8::Isolate* isolate_;

diff --git a/src/node_http2.h b/src/node_http2.h
@@ -7,7 +7,7 @@
 #include <cstdint>
 #include "nghttp2/nghttp2.h"
 
-#include "aliased_struct.h"
+#include "aliased_struct-inl.h"
 #include "node_http2_state.h"
 #include "node_http_common.h"
 #include "node_mem.h"