From a935a222b5f0fcffb31b34550e41c42d13f1cb9c 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
---
 yoga/YGNode.cpp | 57 ++++++++++++++++++++++---------------------------
 yoga/YGNode.h   | 17 ++++++++++++++-
 2 files changed, 42 insertions(+), 32 deletions(-)

diff --git a/yoga/YGNode.cpp b/yoga/YGNode.cpp
index 7b741acf..2fb4510c 100644
--- a/yoga/YGNode.cpp
+++ b/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/yoga/YGNode.h b/yoga/YGNode.h
index 42c99f00..b5ff98a5 100644
--- a/yoga/YGNode.h
+++ b/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 {