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
This commit is contained in:
David Aurelio
2019-03-01 04:18:42 -08:00
committed by Facebook Github Bot
parent 7c4da0a341
commit a935a222b5
2 changed files with 42 additions and 32 deletions

View File

@@ -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];

View File

@@ -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 {