Use Bitfield in YGNode and YGStyle

Summary: @public Replaces the usage of C++ bitfields with our portable `Bitfield` class. Reviewed By: SidharthGuglani Differential Revision: D16649875 fbshipit-source-id: 539f016d5e1c9a8c48cc9bacbbf6ed985e385e69
2019-08-07 16:17:00 -07:00
parent 3ed9bec05c
commit 884e064a99
4 changed files with 125 additions and 159 deletions
--- a/yoga/Bitfield.h
+++ b/yoga/Bitfield.h
@@ -95,6 +95,20 @@ class Bitfield {
  template <size_t Idx>
  using TypeAt = typename detail::IndexedType<Idx, Fields...>::Type;

+  template <size_t Idx, typename Value, typename... Values>
+  static constexpr Storage initStorage(Value value, Values... values) {
+    return ((value << BitTraits::shift(Idx)) & BitTraits::mask(Idx)) |
+        initStorage<Idx + 1, Values...>(values...);
+  }
+
+  template <size_t Idx>
+  static constexpr Storage initStorage() {
+    return Storage{0};
+  }
+
+  Storage storage_ = 0;
+
+public:
  template <size_t Idx>
  class Ref {
    Bitfield& bitfield_;
@@ -111,20 +125,6 @@ class Bitfield {
    }
  };

-  template <size_t Idx, typename Value, typename... Values>
-  static constexpr Storage initStorage(Value value, Values... values) {
-    return ((value << BitTraits::shift(Idx)) & BitTraits::mask(Idx)) |
-        initStorage<Idx + 1, Values...>(values...);
-  }
-
-  template <size_t Idx>
-  static constexpr Storage initStorage() {
-    return Storage{0};
-  }
-
-  Storage storage_ = 0;
-
-public:
  constexpr Bitfield() = default;
  constexpr Bitfield(Fields... values) : storage_{initStorage<0>(values...)} {}

--- a/yoga/YGNode.cpp
+++ b/yoga/YGNode.cpp
@@ -15,14 +15,7 @@ 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_;
-  useWebDefaults_ = node.useWebDefaults_;
+  flags_ = node.flags_;
  measure_ = node.measure_;
  baseline_ = node.baseline_;
  print_ = node.print_;
@@ -48,7 +41,7 @@ YGNode::YGNode(const YGNode& node, YGConfigRef config) : YGNode{node} {

 void YGNode::print(void* printContext) {
  if (print_.noContext != nullptr) {
-    if (printUsesContext_) {
+    if (flags_.at<printUsesContext_>()) {
      print_.withContext(this, printContext);
    } else {
      print_.noContext(this);
@@ -154,14 +147,14 @@ YGSize YGNode::measure(
    YGMeasureMode heightMode,
    void* layoutContext) {

-  return measureUsesContext_
+  return flags_.at<measureUsesContext_>()
      ? measure_.withContext(
            this, width, widthMode, height, heightMode, layoutContext)
      : measure_.noContext(this, width, widthMode, height, heightMode);
 }

 float YGNode::baseline(float width, float height, void* layoutContext) {
-  return baselineUsesContext_
+  return flags_.at<baselineUsesContext_>()
      ? baseline_.withContext(this, width, height, layoutContext)
      : baseline_.noContext(this, width, height);
 }
@@ -172,7 +165,7 @@ void YGNode::setMeasureFunc(decltype(YGNode::measure_) measureFunc) {
  if (measureFunc.noContext == nullptr) {
    // TODO: t18095186 Move nodeType to opt-in function and mark appropriate
    // places in Litho
-    nodeType_ = YGNodeTypeDefault;
+    flags_.at<nodeType_>() = YGNodeTypeDefault;
  } else {
    YGAssertWithNode(
        this,
@@ -188,14 +181,14 @@ void YGNode::setMeasureFunc(decltype(YGNode::measure_) measureFunc) {
 }

 void YGNode::setMeasureFunc(YGMeasureFunc measureFunc) {
-  measureUsesContext_ = false;
+  flags_.at<measureUsesContext_>() = false;
  decltype(YGNode::measure_) m;
  m.noContext = measureFunc;
  setMeasureFunc(m);
 }

 void YGNode::setMeasureFunc(MeasureWithContextFn measureFunc) {
-  measureUsesContext_ = true;
+  flags_.at<measureUsesContext_>() = true;
  decltype(YGNode::measure_) m;
  m.withContext = measureFunc;
  setMeasureFunc(m);
@@ -214,10 +207,10 @@ void YGNode::insertChild(YGNodeRef child, uint32_t index) {
 }

 void YGNode::setDirty(bool isDirty) {
-  if (isDirty == isDirty_) {
+  if (isDirty == flags_.at<isDirty_>()) {
    return;
  }
-  isDirty_ = isDirty;
+  flags_.at<isDirty_>() = isDirty;
  if (isDirty && dirtied_) {
    dirtied_(this);
  }
@@ -357,7 +350,7 @@ YGValue YGNode::resolveFlexBasisPtr() const {
    return flexBasis;
  }
  if (!style_.flex().isUndefined() && style_.flex().unwrap() > 0.0f) {
-    return useWebDefaults_ ? YGValueAuto : YGValueZero;
+    return flags_.at<useWebDefaults_>() ? YGValueAuto : YGValueZero;
  }
  return YGValueAuto;
 }
@@ -396,7 +389,7 @@ void YGNode::cloneChildrenIfNeeded(void* cloneContext) {
 }

 void YGNode::markDirtyAndPropogate() {
-  if (!isDirty_) {
+  if (!flags_.at<isDirty_>()) {
    setDirty(true);
    setLayoutComputedFlexBasis(YGFloatOptional());
    if (owner_) {
@@ -406,7 +399,7 @@ void YGNode::markDirtyAndPropogate() {
 }

 void YGNode::markDirtyAndPropogateDownwards() {
-  isDirty_ = true;
+  flags_.at<isDirty_>() = true;
  for_each(children_.begin(), children_.end(), [](YGNodeRef childNode) {
    childNode->markDirtyAndPropogateDownwards();
  });
@@ -433,11 +426,12 @@ float YGNode::resolveFlexShrink() const {
  if (!style_.flexShrink().isUndefined()) {
    return style_.flexShrink().unwrap();
  }
-  if (!useWebDefaults_ && !style_.flex().isUndefined() &&
+  if (!flags_.at<useWebDefaults_>() && !style_.flex().isUndefined() &&
      style_.flex().unwrap() < 0.0f) {
    return -style_.flex().unwrap();
  }
-  return useWebDefaults_ ? kWebDefaultFlexShrink : kDefaultFlexShrink;
+  return flags_.at<useWebDefaults_>() ? kWebDefaultFlexShrink
+                                      : kDefaultFlexShrink;
 }

 bool YGNode::isNodeFlexible() {
@@ -582,7 +576,7 @@ void YGNode::reset() {

  clearChildren();

-  auto webDefaults = useWebDefaults_;
+  auto webDefaults = flags_.at<useWebDefaults_>();
  *this = YGNode{getConfig()};
  if (webDefaults) {
    useWebDefaults();
--- a/yoga/YGNode.h
+++ b/yoga/YGNode.h
@@ -7,6 +7,7 @@
 #pragma once
 #include <cstdint>
 #include <stdio.h>
+#include "Bitfield.h"
 #include "CompactValue.h"
 #include "YGConfig.h"
 #include "YGLayout.h"
@@ -23,15 +24,20 @@ struct YGNode {
  using PrintWithContextFn = void (*)(YGNode*, void*);

 private:
+  static constexpr size_t hasNewLayout_ = 0;
+  static constexpr size_t isReferenceBaseline_ = 1;
+  static constexpr size_t isDirty_ = 2;
+  static constexpr size_t nodeType_ = 3;
+  static constexpr size_t measureUsesContext_ = 4;
+  static constexpr size_t baselineUsesContext_ = 5;
+  static constexpr size_t printUsesContext_ = 6;
+  static constexpr size_t useWebDefaults_ = 7;
+
  void* context_ = nullptr;
-  bool hasNewLayout_ : 1;
-  bool isReferenceBaseline_ : 1;
-  bool isDirty_ : 1;
-  YGNodeType nodeType_ : 1;
-  bool measureUsesContext_ : 1;
-  bool baselineUsesContext_ : 1;
-  bool printUsesContext_ : 1;
-  bool useWebDefaults_ : 1;
+  using Flags = facebook::yoga::
+      Bitfield<uint8_t, bool, bool, bool, YGNodeType, bool, bool, bool, bool>;
+  Flags flags_ =
+      {true, false, false, YGNodeTypeDefault, false, false, false, false};
  uint8_t reserved_ = 0;
  union {
    YGMeasureFunc noContext;
@@ -63,7 +69,7 @@ private:
  void setBaselineFunc(decltype(baseline_));

  void useWebDefaults() {
-    useWebDefaults_ = true;
+    flags_.at<useWebDefaults_>() = true;
    style_.flexDirection() = YGFlexDirectionRow;
    style_.alignContent() = YGAlignStretch;
  }
@@ -79,17 +85,8 @@ private:

 public:
  YGNode() : YGNode{YGConfigGetDefault()} {}
-  explicit YGNode(const YGConfigRef config)
-      : hasNewLayout_{true},
-        isReferenceBaseline_{false},
-        isDirty_{false},
-        nodeType_{YGNodeTypeDefault},
-        measureUsesContext_{false},
-        baselineUsesContext_{false},
-        printUsesContext_{false},
-        useWebDefaults_{config->useWebDefaults},
-        config_{config} {
-    if (useWebDefaults_) {
+  explicit YGNode(const YGConfigRef config) : config_{config} {
+    if (config->useWebDefaults) {
      useWebDefaults();
    }
  };
@@ -116,9 +113,9 @@ public:

  void print(void*);

-  bool getHasNewLayout() const { return hasNewLayout_; }
+  bool getHasNewLayout() const { return flags_.at<hasNewLayout_>(); }

-  YGNodeType getNodeType() const { return nodeType_; }
+  YGNodeType getNodeType() const { return flags_.at<nodeType_>(); }

  bool hasMeasureFunc() const noexcept { return measure_.noContext != nullptr; }

@@ -144,7 +141,7 @@ public:

  uint32_t getLineIndex() const { return lineIndex_; }

-  bool isReferenceBaseline() { return isReferenceBaseline_; }
+  bool isReferenceBaseline() { return flags_.at<isReferenceBaseline_>(); }

  // returns the YGNodeRef that owns this YGNode. An owner is used to identify
  // the YogaTree that a YGNode belongs to. This method will return the parent
@@ -177,7 +174,7 @@ public:

  YGConfigRef getConfig() const { return config_; }

-  bool isDirty() const { return isDirty_; }
+  bool isDirty() const { return flags_.at<isDirty_>(); }

  std::array<YGValue, 2> getResolvedDimensions() const {
    return resolvedDimensions_;
@@ -225,17 +222,19 @@ public:

  void setPrintFunc(YGPrintFunc printFunc) {
    print_.noContext = printFunc;
-    printUsesContext_ = false;
+    flags_.at<printUsesContext_>() = false;
  }
  void setPrintFunc(PrintWithContextFn printFunc) {
    print_.withContext = printFunc;
-    printUsesContext_ = true;
+    flags_.at<printUsesContext_>() = true;
  }
  void setPrintFunc(std::nullptr_t) { setPrintFunc(YGPrintFunc{nullptr}); }

-  void setHasNewLayout(bool hasNewLayout) { hasNewLayout_ = hasNewLayout; }
+  void setHasNewLayout(bool hasNewLayout) {
+    flags_.at<hasNewLayout_>() = hasNewLayout;
+  }

-  void setNodeType(YGNodeType nodeType) { nodeType_ = nodeType; }
+  void setNodeType(YGNodeType nodeType) { flags_.at<nodeType_>() = nodeType; }

  void setMeasureFunc(YGMeasureFunc measureFunc);
  void setMeasureFunc(MeasureWithContextFn);
@@ -244,11 +243,11 @@ public:
  }

  void setBaselineFunc(YGBaselineFunc baseLineFunc) {
-    baselineUsesContext_ = false;
+    flags_.at<baselineUsesContext_>() = false;
    baseline_.noContext = baseLineFunc;
  }
  void setBaselineFunc(BaselineWithContextFn baseLineFunc) {
-    baselineUsesContext_ = true;
+    flags_.at<baselineUsesContext_>() = true;
    baseline_.withContext = baseLineFunc;
  }
  void setBaselineFunc(std::nullptr_t) {
@@ -264,7 +263,7 @@ public:
  void setLineIndex(uint32_t lineIndex) { lineIndex_ = lineIndex; }

  void setIsReferenceBaseline(bool isReferenceBaseline) {
-    isReferenceBaseline_ = isReferenceBaseline;
+    flags_.at<isReferenceBaseline_>() = isReferenceBaseline;
  }

  void setOwner(YGNodeRef owner) { owner_ = owner; }
--- a/yoga/YGStyle.h
+++ b/yoga/YGStyle.h
@@ -9,25 +9,13 @@
 #include <array>
 #include <cstdint>
 #include <type_traits>
+#include "Bitfield.h"
 #include "CompactValue.h"
 #include "YGEnums.h"
 #include "YGFloatOptional.h"
 #include "Yoga-internal.h"
 #include "Yoga.h"

-#if !defined(ENUM_BITFIELDS_NOT_SUPPORTED)
-#define BITFIELD_ENUM_SIZED(num) : num
-#else
-#define BITFIELD_ENUM_SIZED(num)
-#endif
-
-#define BITFIELD_ACCESSORS(FIELD)                             \
-  decltype(FIELD##_) get_##FIELD() const { return FIELD##_; } \
-  void set_##FIELD(decltype(FIELD##_) x) { FIELD##_ = x; }
-
-#define BITFIELD_REF(FIELD) \
-  BitfieldRef<decltype(FIELD##_), &YGStyle::get_##FIELD, &YGStyle::set_##FIELD>
-
 class YGStyle {
  template <typename Enum>
  using Values =
@@ -71,53 +59,43 @@ public:
    CompactValue operator[](Idx idx) const { return (style.*Prop)[idx]; }
  };

-  template <typename T, T (YGStyle::*Get)() const, void (YGStyle::*Set)(T)>
-  struct BitfieldRef {
-    YGStyle& style;
-
-    operator T() const { return (style.*Get)(); }
-    BitfieldRef<T, Get, Set>& operator=(T x) {
-      (style.*Set)(x);
-      return *this;
-    }
-  };
-
-#ifdef __clang__
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wbitfield-constant-conversion"
-#endif
-
-  YGStyle()
-      : direction_(YGDirectionInherit),
-        flexDirection_(YGFlexDirectionColumn),
-        justifyContent_(YGJustifyFlexStart),
-        alignContent_(YGAlignFlexStart),
-        alignItems_(YGAlignStretch),
-        alignSelf_(YGAlignAuto),
-        positionType_(YGPositionTypeRelative),
-        flexWrap_(YGWrapNoWrap),
-        overflow_(YGOverflowVisible),
-        display_(YGDisplayFlex) {}
-
-#ifdef __clang__
-#pragma clang diagnostic pop
-#endif
-
+  YGStyle() = default;
  ~YGStyle() = default;

 private:
-  /* Some platforms don't support enum bitfields,
-     so please use BITFIELD_ENUM_SIZED(BITS_COUNT) */
-  YGDirection direction_ BITFIELD_ENUM_SIZED(2);
-  YGFlexDirection flexDirection_ BITFIELD_ENUM_SIZED(2);
-  YGJustify justifyContent_ BITFIELD_ENUM_SIZED(3);
-  YGAlign alignContent_ BITFIELD_ENUM_SIZED(3);
-  YGAlign alignItems_ BITFIELD_ENUM_SIZED(3);
-  YGAlign alignSelf_ BITFIELD_ENUM_SIZED(3);
-  YGPositionType positionType_ BITFIELD_ENUM_SIZED(1);
-  YGWrap flexWrap_ BITFIELD_ENUM_SIZED(2);
-  YGOverflow overflow_ BITFIELD_ENUM_SIZED(2);
-  YGDisplay display_ BITFIELD_ENUM_SIZED(1);
+  static constexpr size_t directionIdx = 0;
+  static constexpr size_t flexDirectionIdx = 1;
+  static constexpr size_t justifyContentIdx = 2;
+  static constexpr size_t alignContentIdx = 3;
+  static constexpr size_t alignItemsIdx = 4;
+  static constexpr size_t alignSelfIdx = 5;
+  static constexpr size_t positionTypeIdx = 6;
+  static constexpr size_t flexWrapIdx = 7;
+  static constexpr size_t overflowIdx = 8;
+  static constexpr size_t displayIdx = 9;
+  using Flags = facebook::yoga::Bitfield<
+      uint32_t,
+      YGDirection,
+      YGFlexDirection,
+      YGJustify,
+      YGAlign,
+      YGAlign,
+      YGAlign,
+      YGPositionType,
+      YGWrap,
+      YGOverflow,
+      YGDisplay>;
+
+  Flags flags_ = {YGDirectionInherit,
+                  YGFlexDirectionColumn,
+                  YGJustifyFlexStart,
+                  YGAlignFlexStart,
+                  YGAlignStretch,
+                  YGAlignAuto,
+                  YGPositionTypeRelative,
+                  YGWrapNoWrap,
+                  YGOverflowVisible,
+                  YGDisplayFlex};
  YGFloatOptional flex_ = {};
  YGFloatOptional flexGrow_ = {};
  YGFloatOptional flexShrink_ = {};
@@ -132,50 +110,49 @@ private:
  // Yoga specific properties, not compatible with flexbox specification
  YGFloatOptional aspectRatio_ = {};

-  BITFIELD_ACCESSORS(direction)
-  BITFIELD_ACCESSORS(flexDirection)
-  BITFIELD_ACCESSORS(justifyContent)
-  BITFIELD_ACCESSORS(alignContent);
-  BITFIELD_ACCESSORS(alignItems);
-  BITFIELD_ACCESSORS(alignSelf);
-  BITFIELD_ACCESSORS(positionType);
-  BITFIELD_ACCESSORS(flexWrap);
-  BITFIELD_ACCESSORS(overflow);
-  BITFIELD_ACCESSORS(display);
-
 public:
  // for library users needing a type
  using ValueRepr = std::remove_reference<decltype(margin_[0])>::type;

-  YGDirection direction() const { return direction_; }
-  BITFIELD_REF(direction) direction() { return {*this}; }
+  YGDirection direction() const { return flags_.at<directionIdx>(); }
+  Flags::Ref<directionIdx> direction() { return flags_.at<directionIdx>(); }

-  YGFlexDirection flexDirection() const { return flexDirection_; }
-  BITFIELD_REF(flexDirection) flexDirection() { return {*this}; }
+  YGFlexDirection flexDirection() const {
+    return flags_.at<flexDirectionIdx>();
+  }
+  Flags::Ref<flexDirectionIdx> flexDirection() {
+    return flags_.at<flexDirectionIdx>();
+  }

-  YGJustify justifyContent() const { return justifyContent_; }
-  BITFIELD_REF(justifyContent) justifyContent() { return {*this}; }
+  YGJustify justifyContent() const { return flags_.at<justifyContentIdx>(); }
+  Flags::Ref<justifyContentIdx> justifyContent() {
+    return flags_.at<justifyContentIdx>();
+  }

-  YGAlign alignContent() const { return alignContent_; }
-  BITFIELD_REF(alignContent) alignContent() { return {*this}; }
+  YGAlign alignContent() const { return flags_.at<alignContentIdx>(); }
+  Flags::Ref<alignContentIdx> alignContent() {
+    return flags_.at<alignContentIdx>();
+  }

-  YGAlign alignItems() const { return alignItems_; }
-  BITFIELD_REF(alignItems) alignItems() { return {*this}; }
+  YGAlign alignItems() const { return flags_.at<alignItemsIdx>(); }
+  Flags::Ref<alignItemsIdx> alignItems() { return flags_.at<alignItemsIdx>(); }

-  YGAlign alignSelf() const { return alignSelf_; }
-  BITFIELD_REF(alignSelf) alignSelf() { return {*this}; }
+  YGAlign alignSelf() const { return flags_.at<alignSelfIdx>(); }
+  Flags::Ref<alignSelfIdx> alignSelf() { return flags_.at<alignSelfIdx>(); }

-  YGPositionType positionType() const { return positionType_; }
-  BITFIELD_REF(positionType) positionType() { return {*this}; }
+  YGPositionType positionType() const { return flags_.at<positionTypeIdx>(); }
+  Flags::Ref<positionTypeIdx> positionType() {
+    return flags_.at<positionTypeIdx>();
+  }

-  YGWrap flexWrap() const { return flexWrap_; }
-  BITFIELD_REF(flexWrap) flexWrap() { return {*this}; }
+  YGWrap flexWrap() const { return flags_.at<flexWrapIdx>(); }
+  Flags::Ref<flexWrapIdx> flexWrap() { return flags_.at<flexWrapIdx>(); }

-  YGOverflow overflow() const { return overflow_; }
-  BITFIELD_REF(overflow) overflow() { return {*this}; }
+  YGOverflow overflow() const { return flags_.at<overflowIdx>(); }
+  Flags::Ref<overflowIdx> overflow() { return flags_.at<overflowIdx>(); }

-  YGDisplay display() const { return display_; }
-  BITFIELD_REF(display) display() { return {*this}; }
+  YGDisplay display() const { return flags_.at<displayIdx>(); }
+  Flags::Ref<displayIdx> display() { return flags_.at<displayIdx>(); }

  YGFloatOptional flex() const { return flex_; }
  Ref<YGFloatOptional, &YGStyle::flex_> flex() { return {*this}; }
@@ -223,7 +200,3 @@ bool operator==(const YGStyle& lhs, const YGStyle& rhs);
 inline bool operator!=(const YGStyle& lhs, const YGStyle& rhs) {
  return !(lhs == rhs);
 }
-
-#undef BITFIELD_ENUM_SIZED
-#undef BITFIELD_ACCESSORS
-#undef BITFIELD_REF