From 37c382a74b82091f03abc5e7b6c338b85aa759f9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Sun, 23 Feb 2020 22:27:13 +0000
Subject: [PATCH] style: Minor LengthPercentage improvements.

* Use debug_unreachable for really unreachable code (having a release
   unreachable!() there gives us little to no benefit, as a borked union can
   already confuse us into reading an arbitrary pointer as a CalcPercentage).

 * Avoid a clone of the calc variant when clamping. We only need to mutate the
   clamping mode. This was the only clamp_to_non_negative function that didn't
   consume the value.

Differential Revision: https://phabricator.services.mozilla.com/D63584
---
 .../values/computed/length_percentage.rs      | 39 +++++++++++++------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/components/style/values/computed/length_percentage.rs b/components/style/values/computed/length_percentage.rs
index 4a796ba7b55..7c87fdef229 100644
--- a/components/style/values/computed/length_percentage.rs
+++ b/components/style/values/computed/length_percentage.rs
@@ -170,6 +170,13 @@ enum Unpacked<'a> {
     Percentage(Percentage),
 }
 
+/// An unpacked `<length-percentage>` that mutably borrows the `calc()` variant.
+enum UnpackedMut<'a> {
+    Calc(&'a mut CalcLengthPercentage),
+    Length(Length),
+    Percentage(Percentage),
+}
+
 /// An unpacked `<length-percentage>` that owns the `calc()` variant, for
 /// serialization purposes.
 #[derive(Deserialize, PartialEq, Serialize)]
@@ -294,7 +301,18 @@ impl LengthPercentage {
             LengthPercentageUnion::TAG_CALC => Tag::Calc,
             LengthPercentageUnion::TAG_LENGTH => Tag::Length,
             LengthPercentageUnion::TAG_PERCENTAGE => Tag::Percentage,
-            _ => unreachable!("Bogus tag?"),
+            _ => unsafe { debug_unreachable!("Bogus tag?") },
+        }
+    }
+
+    #[inline]
+    fn unpack_mut<'a>(&'a mut self) -> UnpackedMut<'a> {
+        unsafe {
+            match self.tag() {
+                Tag::Calc => UnpackedMut::Calc(&mut *self.calc_ptr()),
+                Tag::Length => UnpackedMut::Length(self.0.length.length),
+                Tag::Percentage => UnpackedMut::Percentage(self.0.percentage.percentage),
+            }
         }
     }
 
@@ -425,11 +443,14 @@ impl LengthPercentage {
 
     /// Returns the clamped non-negative values.
     #[inline]
-    pub fn clamp_to_non_negative(&self) -> Self {
-        match self.unpack() {
-            Unpacked::Length(l) => Self::new_length(l.clamp_to_non_negative()),
-            Unpacked::Percentage(p) => Self::new_percent(p.clamp_to_non_negative()),
-            Unpacked::Calc(c) => c.clamp_to_non_negative(),
+    pub fn clamp_to_non_negative(mut self) -> Self {
+        match self.unpack_mut() {
+            UnpackedMut::Length(l) => Self::new_length(l.clamp_to_non_negative()),
+            UnpackedMut::Percentage(p) => Self::new_percent(p.clamp_to_non_negative()),
+            UnpackedMut::Calc(ref mut c) => {
+                c.clamping_mode = AllowedNumericType::NonNegative;
+                self
+            }
         }
     }
 }
@@ -665,12 +686,6 @@ impl CalcLengthPercentage {
         }).unwrap();
         Length::new(self.clamping_mode.clamp(px))
     }
-
-    fn clamp_to_non_negative(&self) -> LengthPercentage {
-        let mut new = self.clone();
-        new.clamping_mode = AllowedNumericType::NonNegative;
-        LengthPercentage::new_calc_unchecked(Box::new(new))
-    }
 }
 
 // NOTE(emilio): We don't compare `clamping_mode` since we want to preserve the