From 3080265bb005154c1b3948fdc45e1a2d45feda9b Mon Sep 17 00:00:00 2001
From: David Aurelio <davidaurelio@fb.com>
Date: Fri, 1 Mar 2019 04:18:42 -0800
Subject: [PATCH] `YGNode` remove assignment operators

Summary:
@public

Having assignment operators for `YGNode` means that existing children on the node assigned to would have to be handled somehow.

Deallocating might be incorrect. Ignoring might leak.

Here, we `delete` copy assignment, and make move assignment private (it is used in `YGNode::reset()`).

Copy and move constructors *can* be implemented. The move constructor has to take ownership of the children, while the copy constructor leaves ownership untouched. Since children are copied lazily during layout, this does not expose true value semantics. We should consider removing the copy constructor, too.

Reviewed By: SidharthGuglani

Differential Revision: D14241663

fbshipit-source-id: 39ffdb07f1028bfcf2710c0674a06cdebf3bd650
---
 ReactCommon/yoga/yoga/YGNode.cpp | 57 +++++++++++++++-----------------
 ReactCommon/yoga/yoga/YGNode.h   | 17 +++++++++-
 2 files changed, 42 insertions(+), 32 deletions(-)

diff --git a/ReactCommon/yoga/yoga/YGNode.cpp b/ReactCommon/yoga/yoga/YGNode.cpp
index 7b741acfae9..2fb4510c590 100644
--- a/ReactCommon/yoga/yoga/YGNode.cpp
+++ b/ReactCommon/yoga/yoga/YGNode.cpp
@@ -5,6 +5,7 @@
  * file in the root directory of this source tree.
  */
 #include "YGNode.h"
+#include <algorithm>
 #include <iostream>
 #include "CompactValue.h"
 #include "Utils.h"
@@ -12,6 +13,31 @@
 using namespace facebook;
 using facebook::yoga::detail::CompactValue;
 
+YGNode::YGNode(YGNode&& node) {
+  context_ = node.context_;
+  hasNewLayout_ = node.hasNewLayout_;
+  isReferenceBaseline_ = node.isReferenceBaseline_;
+  isDirty_ = node.isDirty_;
+  nodeType_ = node.nodeType_;
+  measureUsesContext_ = node.measureUsesContext_;
+  baselineUsesContext_ = node.baselineUsesContext_;
+  printUsesContext_ = node.printUsesContext_;
+  measure_ = node.measure_;
+  baseline_ = node.baseline_;
+  print_ = node.print_;
+  dirtied_ = node.dirtied_;
+  style_ = node.style_;
+  layout_ = node.layout_;
+  lineIndex_ = node.lineIndex_;
+  owner_ = node.owner_;
+  children_ = std::move(node.children_);
+  config_ = node.config_;
+  resolvedDimensions_ = node.resolvedDimensions_;
+  for (auto c : children_) {
+    c->setOwner(c);
+  }
+}
+
 void YGNode::print(void* printContext) {
   if (print_.noContext != nullptr) {
     if (printUsesContext_) {
@@ -298,37 +324,6 @@ void YGNode::setPosition(
       trailing[crossAxis]);
 }
 
-YGNode& YGNode::operator=(const YGNode& node) {
-  if (&node == this) {
-    return *this;
-  }
-
-  for (auto child : children_) {
-    delete child;
-  }
-
-  context_ = node.getContext();
-  hasNewLayout_ = node.getHasNewLayout();
-  nodeType_ = node.getNodeType();
-  measureUsesContext_ = node.measureUsesContext_;
-  baselineUsesContext_ = node.baselineUsesContext_;
-  printUsesContext_ = node.printUsesContext_;
-  measure_ = node.measure_;
-  baseline_ = node.baseline_;
-  print_ = node.print_;
-  dirtied_ = node.getDirtied();
-  style_ = node.style_;
-  layout_ = node.layout_;
-  lineIndex_ = node.getLineIndex();
-  owner_ = node.getOwner();
-  children_ = node.getChildren();
-  config_ = node.getConfig();
-  isDirty_ = node.isDirty();
-  resolvedDimensions_ = node.getResolvedDimensions();
-
-  return *this;
-}
-
 YGValue YGNode::marginLeadingValue(const YGFlexDirection axis) const {
   if (YGFlexDirectionIsRow(axis) && !style_.margin[YGEdgeStart].isUndefined()) {
     return style_.margin[YGEdgeStart];
diff --git a/ReactCommon/yoga/yoga/YGNode.h b/ReactCommon/yoga/yoga/YGNode.h
index 42c99f005aa..b5ff98a5c02 100644
--- a/ReactCommon/yoga/yoga/YGNode.h
+++ b/ReactCommon/yoga/yoga/YGNode.h
@@ -55,6 +55,13 @@ private:
   void setMeasureFunc(decltype(measure_));
   void setBaselineFunc(decltype(baseline_));
 
+  // DANGER DANGER DANGER!
+  // If the the node assigned to has children, we'd either have to deallocate
+  // them (potentially incorrect) or ignore them (danger of leaks). Only ever
+  // use this after checking that there are no children.
+  // DO NOT CHANGE THE VISIBILITY OF THIS METHOD!
+  YGNode& operator=(YGNode&&) = default;
+
 public:
   YGNode()
       : hasNewLayout_{true},
@@ -66,8 +73,16 @@ public:
         printUsesContext_{false} {}
   ~YGNode() = default; // cleanup of owner/children relationships in YGNodeFree
   explicit YGNode(const YGConfigRef newConfig) : config_(newConfig){};
+
+  YGNode(YGNode&&);
+
+  // Does not expose true value semantics, as children are not cloned eagerly.
+  // Should we remove this?
   YGNode(const YGNode& node) = default;
-  YGNode& operator=(const YGNode& node);
+
+  // assignment means potential leaks of existing children, or alternatively
+  // freeing unowned memory, double free, or freeing stack memory.
+  YGNode& operator=(const YGNode&) = delete;
 
   // Getters
   void* getContext() const {