commit cb1cd2f12898d7cdc0d9435be2244ce27f88afe3
author Tom Sepez <tsepez@chromium.org> Tue Dec 12 02:46:12 2023 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Dec 12 02:46:12 2023 +0000
tree 54efecb0d3e58c4027ec78202713899e4781aafd
parent cb19b4be609e2b2809d936ab061161eea5cd45ad

Convert FX_Number to absl::variant<>.

Be stricter than is possible with just a union, and avoid tracking
the type of the value with two discrete booleans.

Change-Id: I71beb99c90a2c0fda42cb6d4ffbfeed29d8a2bf7
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/114570
Commit-Queue: Thomas Sepez <tsepez@google.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>

core/fxcrt/fx_number.cpp

diff --git a/core/fxcrt/fx_number.cpp b/core/fxcrt/fx_number.cpp
index d577531..1d6c129 100644
--- a/core/fxcrt/fx_number.cpp
+++ b/core/fxcrt/fx_number.cpp
@@ -15,24 +15,18 @@
 #include "core/fxcrt/fx_string.h"
 #include "third_party/base/numerics/safe_conversions.h"
 
-FX_Number::FX_Number()
-    : m_bIsInteger(true), m_bIsSigned(false), m_UnsignedValue(0) {}
+FX_Number::FX_Number() = default;
 
-FX_Number::FX_Number(int32_t value)
-    : m_bIsInteger(true), m_bIsSigned(true), m_SignedValue(value) {}
+FX_Number::FX_Number(int32_t value) : value_(value) {}
 
-FX_Number::FX_Number(float value)
-    : m_bIsInteger(false), m_bIsSigned(true), m_FloatValue(value) {}
+FX_Number::FX_Number(float value) : value_(value) {}
 
-FX_Number::FX_Number(ByteStringView strc)
-    : m_bIsInteger(true), m_bIsSigned(false), m_UnsignedValue(0) {
+FX_Number::FX_Number(ByteStringView strc) {
   if (strc.IsEmpty())
     return;
 
   if (strc.Contains('.')) {
-    m_bIsInteger = false;
-    m_bIsSigned = true;
-    m_FloatValue = StringToFloat(strc);
+    value_ = StringToFloat(strc);
     return;
   }
 
@@ -42,14 +36,15 @@
   // unsigned and then check for overflow if the user actually signed the value.
   // The Permissions flag is listed in Table 3.20 PDF 1.7 spec.
   FX_SAFE_UINT32 unsigned_val = 0;
+  bool bIsSigned = false;
   bool bNegative = false;
   size_t cc = 0;
   if (strc[0] == '+') {
+    bIsSigned = true;
     cc++;
-    m_bIsSigned = true;
   } else if (strc[0] == '-') {
+    bIsSigned = true;
     bNegative = true;
-    m_bIsSigned = true;
     cc++;
   }
 
@@ -61,8 +56,8 @@
   }
 
   uint32_t uValue = unsigned_val.ValueOrDefault(0);
-  if (!m_bIsSigned) {
-    m_UnsignedValue = uValue;
+  if (!bIsSigned) {
+    value_ = uValue;
     return;
   }
 
@@ -82,24 +77,38 @@
     // becomes -2147483648. For this case, avoid undefined behavior, because
     // an int32_t cannot represent 2147483648.
     static constexpr int kMinInt = std::numeric_limits<int>::min();
-    m_SignedValue = LIKELY(value != kMinInt) ? -value : kMinInt;
+    value_ = LIKELY(value != kMinInt) ? -value : kMinInt;
   } else {
-    m_SignedValue = value;
+    value_ = value;
   }
 }
 
+bool FX_Number::IsInteger() const {
+  return absl::holds_alternative<uint32_t>(value_) ||
+         absl::holds_alternative<int32_t>(value_);
+}
+
+bool FX_Number::IsSigned() const {
+  return absl::holds_alternative<int32_t>(value_) ||
+         absl::holds_alternative<float>(value_);
+}
+
 int32_t FX_Number::GetSigned() const {
-  if (m_bIsInteger) {
-    return m_SignedValue;
+  if (absl::holds_alternative<uint32_t>(value_)) {
+    return static_cast<int32_t>(absl::get<uint32_t>(value_));
   }
-
-  return pdfium::base::saturated_cast<int32_t>(m_FloatValue);
+  if (absl::holds_alternative<int32_t>(value_)) {
+    return absl::get<int32_t>(value_);
+  }
+  return pdfium::base::saturated_cast<int32_t>(absl::get<float>(value_));
 }
 
 float FX_Number::GetFloat() const {
-  if (!m_bIsInteger)
-    return m_FloatValue;
-
-  return m_bIsSigned ? static_cast<float>(m_SignedValue)
-                     : static_cast<float>(m_UnsignedValue);
+  if (absl::holds_alternative<uint32_t>(value_)) {
+    return static_cast<float>(absl::get<uint32_t>(value_));
+  }
+  if (absl::holds_alternative<int32_t>(value_)) {
+    return static_cast<float>(absl::get<int32_t>(value_));
+  }
+  return absl::get<float>(value_);
 }

core/fxcrt/fx_number.h

diff --git a/core/fxcrt/fx_number.h b/core/fxcrt/fx_number.h
index 3eda3eb..c943dec 100644
--- a/core/fxcrt/fx_number.h
+++ b/core/fxcrt/fx_number.h
@@ -10,29 +10,24 @@
 #include <stdint.h>
 
 #include "core/fxcrt/bytestring.h"
+#include "third_party/abseil-cpp/absl/types/variant.h"
 
 class FX_Number {
  public:
   FX_Number();
-  explicit FX_Number(uint32_t value) = delete;
+  explicit FX_Number(uint32_t value) = delete;  // catch misuse.
   explicit FX_Number(int32_t value);
   explicit FX_Number(float value);
   explicit FX_Number(ByteStringView str);
 
-  bool IsInteger() const { return m_bIsInteger; }
-  bool IsSigned() const { return m_bIsSigned; }
+  bool IsInteger() const;
+  bool IsSigned() const;
 
   int32_t GetSigned() const;  // Underflow/Overflow possible.
   float GetFloat() const;
 
  private:
-  bool m_bIsInteger;  // One of the two integers vs. float type.
-  bool m_bIsSigned;   // Only valid if |m_bInteger|.
-  union {
-    uint32_t m_UnsignedValue;
-    int32_t m_SignedValue;
-    float m_FloatValue;
-  };
+  absl::variant<uint32_t, int32_t, float> value_ = 0u;
 };
 
 #endif  // CORE_FXCRT_FX_NUMBER_H_