commit 11cca7a: [Minor] Reduce size of the css value

[Vsevolod Stakhov]

Author: Vsevolod Stakhov
Date: 2021-06-09 11:44:09 +0100
URL: https://github.com/rspamd/rspamd/commit/11cca7a60c8a9bc87439a2027619ae90561357c5 (HEAD -> master)

[Minor] Reduce size of the css value

---
 src/libserver/css/css_rule.cxx  | 17 ++++++-------
 src/libserver/css/css_value.hxx | 55 ++++++++++++++---------------------------
 2 files changed, 26 insertions(+), 46 deletions(-)

diff --git a/src/libserver/css/css_rule.cxx b/src/libserver/css/css_rule.cxx
index 417770a08..238998009 100644
--- a/src/libserver/css/css_rule.cxx
+++ b/src/libserver/css/css_rule.cxx
@@ -28,15 +28,15 @@ void css_rule::override_values(const css_rule &other)
 {
 	int bits = 0;
 	/* Ensure that our bitset is large enough */
-	static_assert(static_cast<std::size_t>(css_value::css_value_type::CSS_VALUE_NYI) << 1 <
+	static_assert(1 << std::variant_size_v<decltype(css_value::value)> <
 				  std::numeric_limits<int>::max());
 
 	for (const auto &v : values) {
-		bits |= static_cast<int>(v.type);
+		bits |= static_cast<int>(1 << v.value.index());
 	}
 
 	for (const auto &ov : other.values) {
-		if (isset(&bits, static_cast<int>(ov.type))) {
+		if (isset(&bits, static_cast<int>(1 << ov.value.index()))) {
 			/* We need to override the existing value */
 			/*
 			 * The algorithm is not very efficient,
@@ -46,7 +46,7 @@ void css_rule::override_values(const css_rule &other)
 			 * is probably ok here
 			 */
 			for (auto &v : values) {
-				if (v.type == ov.type) {
+				if (v.value.index() == ov.value.index()) {
 					v = ov;
 				}
 			}
@@ -56,25 +56,22 @@ void css_rule::override_values(const css_rule &other)
 	/* Copy only not set values */
 	std::copy_if(other.values.begin(), other.values.end(), std::back_inserter(values),
 			[&bits](const auto &elt) -> bool {
-				return (bits & static_cast<int>(elt.type)) == 0;
+				return (bits & (1 << static_cast<int>(elt.value.index()))) == 0;
 			});
 }
 
 void css_rule::merge_values(const css_rule &other)
 {
 	unsigned int bits = 0;
-	/* Ensure that our bitset is large enough */
-	static_assert(static_cast<std::size_t>(css_value::css_value_type::CSS_VALUE_NYI) << 1 <
-		std::numeric_limits<int>::max());
 
 	for (const auto &v : values) {
-		bits |= static_cast<int>(v.type);
+		bits |= 1 << v.value.index();
 	}
 
 	/* Copy only not set values */
 	std::copy_if(other.values.begin(), other.values.end(), std::back_inserter(values),
 			[&bits](const auto &elt) -> bool {
-				return (bits & static_cast<int>(elt.type)) == 0;
+				return (bits & (1 << elt.value.index())) == 0;
 			});
 }
 
diff --git a/src/libserver/css/css_value.hxx b/src/libserver/css/css_value.hxx
index 9e192acb6..82f65e3e9 100644
--- a/src/libserver/css/css_value.hxx
+++ b/src/libserver/css/css_value.hxx
@@ -69,66 +69,40 @@ enum class css_display_value {
  * for simplicity
  */
 struct css_value {
-	/* Bitset of known types */
-	enum class css_value_type {
-		CSS_VALUE_COLOR = 1 << 0,
-		CSS_VALUE_NUMBER = 1 << 1,
-		CSS_VALUE_DISPLAY = 1 << 2,
-		CSS_VALUE_DIMENSION = 1 << 3,
-		CSS_VALUE_NYI = 1 << 4,
-	};
-
-	css_value_type type;
 	std::variant<css_color,
 			double,
 			css_display_value,
 			css_dimension,
 			std::monostate> value;
 
-	css_value() : type(css_value_type::CSS_VALUE_NYI) {}
+	css_value() {}
 	css_value(const css_color &color) :
-			type(css_value_type::CSS_VALUE_COLOR), value(color) {}
+			value(color) {}
 	css_value(double num) :
-			type(css_value_type::CSS_VALUE_NUMBER), value(num) {}
+			value(num) {}
 	css_value(css_dimension dim) :
-			type(css_value_type::CSS_VALUE_DIMENSION), value(dim) {}
+			value(dim) {}
 	css_value(css_display_value d) :
-			type(css_value_type::CSS_VALUE_DISPLAY), value(d) {}
+			value(d) {}
 
 	auto to_color(void) const -> std::optional<css_color> {
-		if (type == css_value_type::CSS_VALUE_COLOR) {
-			return std::get<css_color>(value);
-		}
-
-		return std::nullopt;
+		return extract_value_maybe<css_color>();
 	}
 
 	auto to_number(void) const -> std::optional<double> {
-		if (type == css_value_type::CSS_VALUE_NUMBER) {
-			return std::get<double>(value);
-		}
-
-		return std::nullopt;
+		return extract_value_maybe<double>();
 	}
 
 	auto to_dimension(void) const -> std::optional<css_dimension> {
-		if (type == css_value_type::CSS_VALUE_DIMENSION) {
-			return std::get<css_dimension>(value);
-		}
-
-		return std::nullopt;
+		return extract_value_maybe<css_dimension>();
 	}
 
 	auto to_display(void) const -> std::optional<css_display_value> {
-		if (type == css_value_type::CSS_VALUE_DISPLAY) {
-			return std::get<css_display_value>(value);
-		}
-
-		return std::nullopt;
+		return extract_value_maybe<css_display_value>();
 	}
 
 	auto is_valid(void) const -> bool {
-		return (type != css_value_type::CSS_VALUE_NYI);
+		return !(std::holds_alternative<std::monostate>(value));
 	}
 
 	auto debug_str() const -> std::string;
@@ -143,6 +117,15 @@ struct css_value {
 		-> std::optional<css_value>;
 	static auto maybe_display_from_string(const std::string_view &input)
 		-> std::optional<css_value>;
+private:
+	template<typename T>
+	auto extract_value_maybe(void) const -> std::optional<T> {
+		if (std::holds_alternative<T>(value)) {
+			return std::get<T>(value);
+		}
+
+		return std::nullopt;
+	}
 };
 
 }