From 516c4ccb711b9b066a78b4f3448a44355dfabfec Mon Sep 17 00:00:00 2001
From: Nick Gerleman <ngerlem@meta.com>
Date: Tue, 13 Feb 2024 20:13:22 -0800
Subject: [PATCH] Fix CSSParser constexpr-iness

Summary:
I originally marked most of this as constexpr, since the code around the variant, and parsing, were already header only and avoiding allocations, and was pretty leaf node.

`reinterpret_cast` and similar is not allowed under constexpr until C++ 26, which `CSSValue` was using. We change our method of storage to a recursively defined union, which is the same underlying implementation of `std::variant` (and is how it is constexpr).

`std::pow` is also not constexpr until C++ 26 which means we can't assign a CSSValue which tokenizes a decimal number... But... constexpr here is more a bonus, and not worth pulling in constexpr math library to do it.

Changelog: [Internal]

Reviewed By: joevilches

Differential Revision: D53696265

fbshipit-source-id: 01442ab9222b3f3ef8ccb01383648fb2e1f747dd
---
 .../react/renderer/css/CSSKeywords.h          |  8 +-
 .../ReactCommon/react/renderer/css/CSSValue.h | 85 +++++++++++++++----
 .../renderer/css/tests/CSSParserTest.cpp      | 14 +++
 .../ReactCommon/react/utils/PackTraits.h      |  9 --
 4 files changed, 87 insertions(+), 29 deletions(-)

diff --git a/packages/react-native/ReactCommon/react/renderer/css/CSSKeywords.h b/packages/react-native/ReactCommon/react/renderer/css/CSSKeywords.h
index a584ef023b2..7279c36c7fd 100644
--- a/packages/react-native/ReactCommon/react/renderer/css/CSSKeywords.h
+++ b/packages/react-native/ReactCommon/react/renderer/css/CSSKeywords.h
@@ -8,7 +8,6 @@
 #pragma once
 
 #include <cstdint>
-#include <locale>
 #include <optional>
 #include <string_view>
 
@@ -172,8 +171,11 @@ CSS_DEFINE_KEYWORD_CONEPTS(WrapReverse)
 template <CSSKeywordSet KeywordT>
 constexpr std::optional<KeywordT> parseCSSKeyword(std::string_view ident) {
   struct LowerCaseTransform {
-    char operator()(char c) const {
-      return static_cast<char>(tolower(c));
+    constexpr char operator()(char c) const {
+      if (c >= 'A' && c <= 'Z') {
+        return c + static_cast<char>('a' - 'A');
+      }
+      return c;
     }
   };
 
diff --git a/packages/react-native/ReactCommon/react/renderer/css/CSSValue.h b/packages/react-native/ReactCommon/react/renderer/css/CSSValue.h
index 4663dd9e818..5d7d9b1ee41 100644
--- a/packages/react-native/ReactCommon/react/renderer/css/CSSValue.h
+++ b/packages/react-native/ReactCommon/react/renderer/css/CSSValue.h
@@ -7,7 +7,6 @@
 
 #pragma once
 
-#include <array>
 #include <cstdint>
 #include <string_view>
 #include <type_traits>
@@ -91,15 +90,6 @@ struct CSSRatio {
 #pragma pack(push, 1)
 template <CSSDataType... AllowedTypesT>
 class CSSValueVariant {
-  template <CSSValueType Type, CSSDataType ValueT>
-  constexpr ValueT getIf() const {
-    if (type_ == Type) {
-      return *std::launder(reinterpret_cast<const ValueT*>(data_.data()));
-    } else {
-      return ValueT{};
-    }
-  }
-
   template <CSSDataType ValueT>
   static constexpr bool canRepresent() {
     return traits::containsType<ValueT, AllowedTypesT...>();
@@ -201,20 +191,81 @@ class CSSValueVariant {
   }
 
   constexpr operator bool() const requires(canRepresent<CSSWideKeyword>()) {
-    return *this != CSSValueVariant{};
+    return type() != CSSValueType::CSSWideKeyword ||
+        getCSSWideKeyword() != CSSWideKeyword::Unset;
   }
 
-  constexpr bool operator==(const CSSValueVariant& rhs) const = default;
+  constexpr bool operator==(const CSSValueVariant& other) const {
+    if (type() != other.type()) {
+      return false;
+    }
+    switch (type()) {
+      case CSSValueType::CSSWideKeyword:
+        return getCSSWideKeyword() == other.getCSSWideKeyword();
+      case CSSValueType::Keyword:
+        return getKeyword() == other.getKeyword();
+      case CSSValueType::Length:
+        return getLength() == other.getLength();
+      case CSSValueType::Number:
+        return getNumber() == other.getNumber();
+      case CSSValueType::Percentage:
+        return getPercentage() == other.getPercentage();
+      case CSSValueType::Ratio:
+        return getRatio() == other.getRatio();
+    }
+
+    return false;
+  }
 
  private:
-  constexpr CSSValueVariant(CSSValueType type, CSSDataType auto&& value)
-      : type_(type) {
-    new (data_.data()) std::remove_cvref_t<decltype(value)>{
-        std::forward<decltype(value)>(value)};
+  template <CSSValueType Type, CSSDataType ValueT>
+  constexpr ValueT getIf() const {
+    if (type_ == Type) {
+      return getFromUnion<ValueT>(data_);
+    } else {
+      return ValueT{};
+    }
+  }
+
+  template <CSSDataType ValueT, CSSDataType... RestT>
+  union RecursiveUnion {
+    ValueT first;
+    RecursiveUnion<RestT...> rest;
+  };
+
+  template <CSSDataType ValueT>
+  union RecursiveUnion<ValueT> {
+    ValueT first;
+  };
+
+  template <CSSDataType ValueT, typename UnionT>
+  constexpr const ValueT& getFromUnion(const UnionT& u) const {
+    if constexpr (std::is_same_v<ValueT, decltype(u.first)>) {
+      return u.first;
+    } else {
+      return getFromUnion<ValueT>(u.rest);
+    }
+  }
+
+  template <CSSDataType DataTypeT>
+  constexpr CSSValueVariant(CSSValueType type, DataTypeT&& value)
+      : type_{type},
+        data_{constructIntoUnion<decltype(data_)>(
+            std::forward<DataTypeT>(value))} {}
+
+  template <typename UnionT, CSSDataType DataTypeT>
+  constexpr UnionT constructIntoUnion(DataTypeT&& value) {
+    if constexpr (std::is_same_v<DataTypeT, decltype(UnionT{}.first)>) {
+      return UnionT{.first = std::forward<DataTypeT>(value)};
+    } else {
+      return UnionT{
+          .rest = constructIntoUnion<decltype(UnionT{}.rest)>(
+              std::forward<DataTypeT>(value))};
+    }
   }
 
   CSSValueType type_;
-  std::array<std::byte, traits::maxSizeof<AllowedTypesT...>()> data_;
+  RecursiveUnion<AllowedTypesT...> data_;
 };
 #pragma pack(pop)
 
diff --git a/packages/react-native/ReactCommon/react/renderer/css/tests/CSSParserTest.cpp b/packages/react-native/ReactCommon/react/renderer/css/tests/CSSParserTest.cpp
index 7a16be05bc0..2ff8760e0dc 100644
--- a/packages/react-native/ReactCommon/react/renderer/css/tests/CSSParserTest.cpp
+++ b/packages/react-native/ReactCommon/react/renderer/css/tests/CSSParserTest.cpp
@@ -300,4 +300,18 @@ TEST(CSSParser, parse_prop) {
   EXPECT_EQ(keywordlessValue.getLength().unit, CSSLengthUnit::Px);
 }
 
+TEST(CSSParser, parse_keyword_prop_constexpr) {
+  constexpr auto rowValue = parseCSSProp<CSSProp::FlexDirection>("row");
+  EXPECT_EQ(rowValue.type(), CSSValueType::Keyword);
+  EXPECT_EQ(
+      rowValue.getKeyword(), CSSAllowedKeywords<CSSProp::FlexDirection>::Row);
+}
+
+TEST(CSSParser, parse_length_prop_constexpr) {
+  constexpr auto pxValue = parseCSSProp<CSSProp::BorderWidth>("2px");
+  EXPECT_EQ(pxValue.type(), CSSValueType::Length);
+  EXPECT_EQ(pxValue.getLength().value, 2.0f);
+  EXPECT_EQ(pxValue.getLength().unit, CSSLengthUnit::Px);
+}
+
 } // namespace facebook::react
diff --git a/packages/react-native/ReactCommon/react/utils/PackTraits.h b/packages/react-native/ReactCommon/react/utils/PackTraits.h
index 0f14add9652..91d912f4a06 100644
--- a/packages/react-native/ReactCommon/react/utils/PackTraits.h
+++ b/packages/react-native/ReactCommon/react/utils/PackTraits.h
@@ -13,15 +13,6 @@
 
 namespace facebook::react::traits {
 
-template <typename T, typename... RestT>
-static constexpr size_t maxSizeof() {
-  if constexpr (sizeof...(RestT) > 0) {
-    return std::max(sizeof(T), maxSizeof<RestT...>());
-  } else {
-    return sizeof(T);
-  }
-}
-
 template <typename ExpectedT>
 static constexpr bool containsType() {
   return false;