about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMazdak Farrokhzad <twingoow@gmail.com>2020-03-26 13:32:17 +0100
committerGitHub <noreply@github.com>2020-03-26 13:32:17 +0100
commitf9d1378dd497a1e48ec63adc7a704937f9a5a108 (patch)
tree9d221fbe0c96d1626060331a4aef0f559e7797d2
parent20771ae5b9bf2601b39585444d808eaf8ab97460 (diff)
parent0b00c20465c2cacf34b4d3d1a5f4d0427f384cb2 (diff)
downloadrust-f9d1378dd497a1e48ec63adc7a704937f9a5a108.tar.gz
rust-f9d1378dd497a1e48ec63adc7a704937f9a5a108.zip
Rollup merge of #70411 - ogoffart:fix-62691, r=eddyb
Fix for #62691: use the largest niche across all fields

fixes #62691

(The second commit is a small optimization but it makes the code less pretty and i don't know if it is worth it.)
-rw-r--r--src/librustc/ty/layout.rs27
-rw-r--r--src/test/ui/type-sizes.rs7
2 files changed, 21 insertions, 13 deletions
diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs
index 1ce3d4d6bec..ad4352c0105 100644
--- a/src/librustc/ty/layout.rs
+++ b/src/librustc/ty/layout.rs
@@ -282,8 +282,6 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
 
         let mut align = if pack.is_some() { dl.i8_align } else { dl.aggregate_align };
 
-        let mut sized = true;
-        let mut offsets = vec![Size::ZERO; fields.len()];
         let mut inverse_memory_index: Vec<u32> = (0..fields.len() as u32).collect();
 
         let mut optimize = !repr.inhibit_struct_field_reordering_opt();
@@ -320,6 +318,8 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
         // At the bottom of this function, we invert `inverse_memory_index` to
         // produce `memory_index` (see `invert_mapping`).
 
+        let mut sized = true;
+        let mut offsets = vec![Size::ZERO; fields.len()];
         let mut offset = Size::ZERO;
         let mut largest_niche = None;
         let mut largest_niche_available = 0;
@@ -900,18 +900,19 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
                         let count = (niche_variants.end().as_u32()
                             - niche_variants.start().as_u32()
                             + 1) as u128;
-                        // FIXME(#62691) use the largest niche across all fields,
-                        // not just the first one.
-                        for (field_index, &field) in variants[i].iter().enumerate() {
-                            let niche = match &field.largest_niche {
-                                Some(niche) => niche,
-                                _ => continue,
-                            };
-                            let (niche_start, niche_scalar) = match niche.reserve(self, count) {
-                                Some(pair) => pair,
-                                None => continue,
-                            };
 
+                        // Find the field with the largest niche
+                        let niche_candidate = variants[i]
+                            .iter()
+                            .enumerate()
+                            .filter_map(|(j, &field)| Some((j, field.largest_niche.as_ref()?)))
+                            .max_by_key(|(_, niche)| niche.available(dl));
+
+                        if let Some((field_index, niche, (niche_start, niche_scalar))) =
+                            niche_candidate.and_then(|(field_index, niche)| {
+                                Some((field_index, niche, niche.reserve(self, count)?))
+                            })
+                        {
                             let mut align = dl.aggregate_align;
                             let st = variants
                                 .iter_enumerated()
diff --git a/src/test/ui/type-sizes.rs b/src/test/ui/type-sizes.rs
index 27433fd770b..1d332cc3bf7 100644
--- a/src/test/ui/type-sizes.rs
+++ b/src/test/ui/type-sizes.rs
@@ -74,6 +74,11 @@ enum NicheFilledEnumWithAbsentVariant {
     C,
 }
 
+enum Option2<A, B> {
+    Some(A, B),
+    None
+}
+
 pub fn main() {
     assert_eq!(size_of::<u8>(), 1 as usize);
     assert_eq!(size_of::<u32>(), 4 as usize);
@@ -113,4 +118,6 @@ pub fn main() {
 
     assert_eq!(size_of::<Option<Option<(bool, &())>>>(), size_of::<(bool, &())>());
     assert_eq!(size_of::<Option<Option<(&(), bool)>>>(), size_of::<(bool, &())>());
+    assert_eq!(size_of::<Option<Option2<bool, &()>>>(), size_of::<(bool, &())>());
+    assert_eq!(size_of::<Option<Option2<&(), bool>>>(), size_of::<(bool, &())>());
 }