Auto merge of #11884 - servo:antidiscrimination, r=nox

Replace usage of std::intrinsics::discriminant_value with `match`

<!-- Please describe your changes on the following line: -->

Replace usage of `std::intrinsics::discriminant_value` with per-enum generated code that uses `match` expressions. The LLVM IR shows that this optimizes well in release mode.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix part of #11815.

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require new tests because they shouldn’t change any behavior.

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11884)
<!-- Reviewable:end -->

components/style/lib.rs

@@ -23,9 +23,6 @@
 //! [cssparser]: ../cssparser/index.html
 //! [selectors]: ../selectors/index.html
 
-// FIXME: replace discriminant_value with per-enum methods that use `match`?
-#![feature(core_intrinsics)]
-
 #![cfg_attr(feature = "servo", feature(custom_attribute))]
 #![cfg_attr(feature = "servo", feature(custom_derive))]
 #![cfg_attr(feature = "servo", feature(plugin))]

components/style/properties/properties.mako.rs

@@ -15,8 +15,6 @@ use std::boxed::Box as StdBox;
 use std::collections::HashSet;
 use std::fmt;
 use std::fmt::Write;
-use std::intrinsics;
-use std::mem;
 use std::sync::Arc;
 
 use app_units::Au;
@@ -839,6 +837,16 @@ impl PropertyDeclaration {
         }
     }
 
+    #[inline]
+    pub fn discriminant_value(&self) -> usize {
+        match *self {
+            % for i, property in enumerate(data.longhands):
+                PropertyDeclaration::${property.camel_case}(..) => ${i},
+            % endfor
+            PropertyDeclaration::Custom(..) => ${len(data.longhands)}
+        }
+    }
+
     pub fn value(&self) -> String {
         let mut value = String::new();
         if let Err(_) = self.to_css(&mut value) {
@@ -1221,7 +1229,7 @@ pub trait ComputedValues : Clone + Send + Sync + 'static {
 
         fn initial_values() -> &'static Self;
 
-        fn do_cascade_property<F: FnOnce(&Vec<Option<CascadePropertyFn<Self>>>)>(f: F);
+        fn do_cascade_property<F: FnOnce(&Vec<CascadePropertyFn<Self>>)>(f: F);
 
     % for style_struct in data.active_style_structs():
         fn clone_${style_struct.trait_name_lower}(&self) ->
@@ -1292,7 +1300,7 @@ impl ComputedValues for ServoComputedValues {
 
         fn initial_values() -> &'static Self { &*INITIAL_SERVO_VALUES }
 
-        fn do_cascade_property<F: FnOnce(&Vec<Option<CascadePropertyFn<Self>>>)>(f: F) {
+        fn do_cascade_property<F: FnOnce(&Vec<CascadePropertyFn<Self>>)>(f: F) {
             CASCADE_PROPERTY.with(|x| f(x));
         }
 
@@ -1693,28 +1701,17 @@ pub type CascadePropertyFn<C /*: ComputedValues */> =
                      cacheable: &mut bool,
                      error_reporter: &mut StdBox<ParseErrorReporter + Send>);
 
-pub fn make_cascade_vec<C: ComputedValues>() -> Vec<Option<CascadePropertyFn<C>>> {
+pub fn make_cascade_vec<C: ComputedValues>() -> Vec<CascadePropertyFn<C>> {
-    let mut result: Vec<Option<CascadePropertyFn<C>>> = Vec::new();
+    vec![
-    % for style_struct in data.active_style_structs():
+        % for property in data.longhands:
-        % for property in style_struct.longhands:
+            longhands::${property.ident}::cascade_property,
-            let discriminant;
-            unsafe {
-                let variant = PropertyDeclaration::${property.camel_case}(mem::uninitialized());
-                discriminant = intrinsics::discriminant_value(&variant) as usize;
-                mem::forget(variant);
-            }
-            while result.len() < discriminant + 1 {
-                result.push(None)
-            }
-            result[discriminant] = Some(longhands::${property.ident}::cascade_property);
         % endfor
-    % endfor
+    ]
-    result
 }
 
 // This is a thread-local rather than a lazy static to avoid atomic operations when cascading
 // properties.
-thread_local!(static CASCADE_PROPERTY: Vec<Option<CascadePropertyFn<ServoComputedValues>>> = {
+thread_local!(static CASCADE_PROPERTY: Vec<CascadePropertyFn<ServoComputedValues>> = {
     make_cascade_vec::<ServoComputedValues>()
 });
 
@@ -1840,15 +1837,13 @@ pub fn cascade<C: ComputedValues>(
                     {
                         continue
                     }
-                    let discriminant = unsafe {
+                    let discriminant = declaration.discriminant_value();
-                        intrinsics::discriminant_value(declaration) as usize
+                    (cascade_property[discriminant])(declaration,
-                    };
+                                                     inherited_style,
-                    (cascade_property[discriminant].unwrap())(declaration,
+                                                     &mut context,
-                                                              inherited_style,
+                                                     &mut seen,
-                                                              &mut context,
+                                                     &mut cacheable,
-                                                              &mut seen,
+                                                     &mut error_reporter);
-                                                              &mut cacheable,
-                                                              &mut error_reporter);
                 }
             }
         % endfor

components/style/viewport.rs

@@ -15,9 +15,7 @@ use euclid::size::{Size2D, TypedSize2D};
 use parser::{ParserContext, log_css_error};
 use properties::{ComputedValues, ServoComputedValues};
 use std::ascii::AsciiExt;
-use std::collections::hash_map::{Entry, HashMap};
 use std::fmt;
-use std::intrinsics;
 use std::iter::Enumerate;
 use std::str::Chars;
 use style_traits::viewport::{Orientation, UserZoom, ViewportConstraints, Zoom};
@@ -26,9 +24,59 @@ use util::geometry::ViewportPx;
 use values::computed::{Context, ToComputedValue};
 use values::specified::{Length, LengthOrPercentageOrAuto, ViewportPercentageLength};
 
-#[derive(Copy, Clone, Debug, PartialEq)]
+macro_rules! declare_viewport_descriptor {
-#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+    ( $( $variant: ident($data: ident), )+ ) => {
-pub enum ViewportDescriptor {
+        declare_viewport_descriptor_inner!([] [ $( $variant($data), )+ ] 0);
+    };
+}
+
+macro_rules! declare_viewport_descriptor_inner {
+    (
+        [ $( $assigned_variant: ident($assigned_data: ident) = $assigned_discriminant: expr, )* ]
+        [
+            $next_variant: ident($next_data: ident),
+            $( $variant: ident($data: ident), )*
+        ]
+        $next_discriminant: expr
+    ) => {
+        declare_viewport_descriptor_inner! {
+            [
+                $( $assigned_variant($assigned_data) = $assigned_discriminant, )*
+                $next_variant($next_data) = $next_discriminant,
+            ]
+            [ $( $variant($data), )* ]
+            $next_discriminant + 1
+        }
+    };
+
+    (
+        [ $( $assigned_variant: ident($assigned_data: ident) = $assigned_discriminant: expr, )* ]
+        [ ]
+        $number_of_variants: expr
+    ) => {
+        #[derive(Copy, Clone, Debug, PartialEq)]
+        #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+        pub enum ViewportDescriptor {
+            $(
+                $assigned_variant($assigned_data),
+            )+
+        }
+
+        const VIEWPORT_DESCRIPTOR_VARIANTS: usize = $number_of_variants;
+
+        impl ViewportDescriptor {
+            fn discriminant_value(&self) -> usize {
+                match *self {
+                    $(
+                        ViewportDescriptor::$assigned_variant(..) => $assigned_discriminant,
+                    )*
+                }
+            }
+        }
+    };
+}
+
+declare_viewport_descriptor! {
     MinWidth(ViewportLength),
     MaxWidth(ViewportLength),
 
@@ -40,7 +88,7 @@ pub enum ViewportDescriptor {
     MaxZoom(Zoom),
 
     UserZoom(UserZoom),
-    Orientation(Orientation)
+    Orientation(Orientation),
 }
 
 trait FromMeta: Sized {
@@ -287,18 +335,15 @@ impl ViewportRule {
 
     #[allow(unsafe_code)]
     pub fn from_meta(content: &str) -> Option<ViewportRule> {
-        let mut declarations = HashMap::new();
+        let mut declarations = vec![None; VIEWPORT_DESCRIPTOR_VARIANTS];
         macro_rules! push_descriptor {
             ($descriptor:ident($value:expr)) => {{
                 let descriptor = ViewportDescriptor::$descriptor($value);
-                declarations.insert(
+                let discriminant = descriptor.discriminant_value();
-                    unsafe {
+                declarations[discriminant] = Some(ViewportDescriptorDeclaration::new(
-                        intrinsics::discriminant_value(&descriptor)
+                    Origin::Author,
-                    },
+                    descriptor,
-                    ViewportDescriptorDeclaration::new(
+                    false));
-                        Origin::Author,
-                        descriptor,
-                        false))
             }
         }}
 
@@ -374,7 +419,7 @@ impl ViewportRule {
             }
         }
 
-        let declarations: Vec<_> = declarations.into_iter().map(|kv| kv.1).collect();
+        let declarations: Vec<_> = declarations.into_iter().filter_map(|entry| entry).collect();
         if !declarations.is_empty() {
             Some(ViewportRule { declarations: declarations })
         } else {
@@ -471,34 +516,33 @@ impl ViewportDescriptorDeclaration {
 fn cascade<'a, I>(iter: I) -> Vec<ViewportDescriptorDeclaration>
     where I: Iterator<Item=&'a ViewportDescriptorDeclaration>
 {
-    let mut declarations: HashMap<u64, (usize, &'a ViewportDescriptorDeclaration)> = HashMap::new();
+    let mut declarations: Vec<Option<(usize, &'a ViewportDescriptorDeclaration)>> =
+        vec![None; VIEWPORT_DESCRIPTOR_VARIANTS];
 
     // index is used to reconstruct order of appearance after all declarations
     // have been added to the map
     let mut index = 0;
     for declaration in iter {
-        let descriptor = unsafe {
+        let descriptor = declaration.descriptor.discriminant_value();
-            intrinsics::discriminant_value(&declaration.descriptor)
-        };
 
-        match declarations.entry(descriptor) {
+        match declarations[descriptor] {
-            Entry::Occupied(mut entry) => {
+            Some((ref mut entry_index, ref mut entry_declaration)) => {
-                if declaration.higher_or_equal_precendence(entry.get().1) {
+                if declaration.higher_or_equal_precendence(entry_declaration) {
-                    entry.insert((index, declaration));
+                    *entry_declaration = declaration;
+                    *entry_index = index;
                     index += 1;
                 }
             }
-            Entry::Vacant(entry) => {
+            ref mut entry @ None => {
-                entry.insert((index, declaration));
+                *entry = Some((index, declaration));
                 index += 1;
             }
         }
     }
 
-    // convert to a list and sort the descriptors by order of appearance
+    // sort the descriptors by order of appearance
-    let mut declarations: Vec<_> = declarations.into_iter().map(|kv| kv.1).collect();
+    declarations.sort_by_key(|entry| entry.map(|(index, _)| index));
-    declarations.sort_by(|a, b| a.0.cmp(&b.0));
+    declarations.into_iter().filter_map(|entry| entry.map(|(_, decl)| *decl)).collect::<Vec<_>>()
-    declarations.into_iter().map(|id| *id.1).collect::<Vec<_>>()
 }
 
 impl<'a, I> ViewportDescriptorDeclarationCascade for I

ports/geckolib/properties.mako.rs

@@ -106,7 +106,7 @@ impl ComputedValues for GeckoComputedValues {
 
     fn initial_values() -> &'static Self { &*INITIAL_GECKO_VALUES }
 
-    fn do_cascade_property<F: FnOnce(&Vec<Option<CascadePropertyFn<Self>>>)>(f: F) {
+    fn do_cascade_property<F: FnOnce(&Vec<CascadePropertyFn<Self>>)>(f: F) {
         CASCADE_PROPERTY.with(|x| f(x));
     }
 
@@ -1135,6 +1135,6 @@ lazy_static! {
 
 // This is a thread-local rather than a lazy static to avoid atomic operations when cascading
 // properties.
-thread_local!(static CASCADE_PROPERTY: Vec<Option<CascadePropertyFn<GeckoComputedValues>>> = {
+thread_local!(static CASCADE_PROPERTY: Vec<CascadePropertyFn<GeckoComputedValues>> = {
     make_cascade_vec::<GeckoComputedValues>()
 });