Auto merge of rust-lang#59651 - tmandry:discr-index, r=eddyb

 Add discr_index to multi-variant layouts

We remove the assumption that the discriminant is always field 0, in
preparations for layouts like generators where this is not (always) going to be
the case.

Specifically, upvars are going to go before the discriminant. In theory, it's possible to remove _that_ assumption instead and keep the discriminant at field index 0, but one assumption or the other had to go :)

There is one place I know of in the debuginfo code where we'll still need to remove assumptions that the discriminant is the _only_ field. I was planning on doing this along with the upcoming generator change, which will also include tests that exercise the code changing in this PR.

r? @eddyb
cc @oli-obk
cc @cramertj

src/librustc/ty/layout.rs

@@ -920,6 +920,7 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> {
                                         niche_variants,
                                         niche_start,
                                     },
+                                    discr_index: 0,
                                     variants: st,
                                 },
                                 fields: FieldPlacement::Arbitrary {
@@ -1142,6 +1143,7 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> {
                     variants: Variants::Multiple {
                         discr: tag,
                         discr_kind: DiscriminantKind::Tag,
+                        discr_index: 0,
                         variants: layout_variants,
                     },
                     fields: FieldPlacement::Arbitrary {
@@ -1884,10 +1886,12 @@ impl<'a> HashStable<StableHashingContext<'a>> for Variants {
             Multiple {
                 ref discr,
                 ref discr_kind,
+                discr_index,
                 ref variants,
             } => {
                 discr.hash_stable(hcx, hasher);
                 discr_kind.hash_stable(hcx, hasher);
+                discr_index.hash_stable(hcx, hasher);
                 variants.hash_stable(hcx, hasher);
             }
         }

src/librustc_codegen_llvm/debuginfo/metadata.rs

@@ -22,6 +22,7 @@ use rustc::hir::CodegenFnAttrFlags;
 use rustc::hir::def::CtorKind;
 use rustc::hir::def_id::{DefId, CrateNum, LOCAL_CRATE};
 use rustc::ich::NodeIdHashingMode;
+use rustc::mir::Field;
 use rustc::mir::interpret::truncate;
 use rustc_data_structures::fingerprint::Fingerprint;
 use rustc::ty::Instance;
@@ -1306,12 +1307,15 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> {
             }
             layout::Variants::Multiple {
                 discr_kind: layout::DiscriminantKind::Tag,
+                discr_index,
                 ref variants,
                 ..
             } => {
                 let discriminant_info = if fallback {
-                    RegularDiscriminant(self.discriminant_type_metadata
-                                        .expect(""))
+                    RegularDiscriminant {
+                        discr_field: Field::from(discr_index),
+                        discr_type_metadata: self.discriminant_type_metadata.unwrap()
+                    }
                 } else {
                     // This doesn't matter in this case.
                     NoDiscriminant
@@ -1358,6 +1362,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> {
                 },
                 ref discr,
                 ref variants,
+                discr_index,
             } => {
                 if fallback {
                     let variant = self.layout.for_variant(cx, dataful_variant);
@@ -1403,8 +1408,8 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> {
                     }
                     compute_field_path(cx, &mut name,
                                        self.layout,
-                                       self.layout.fields.offset(0),
-                                       self.layout.field(cx, 0).size);
+                                       self.layout.fields.offset(discr_index),
+                                       self.layout.field(cx, discr_index).size);
                     name.push_str(&adt.variants[*niche_variants.start()].ident.as_str());
 
                     // Create the (singleton) list of descriptions of union members.
@@ -1486,6 +1491,8 @@ impl VariantMemberDescriptionFactory<'ll, 'tcx> {
                 name: name.to_string(),
                 type_metadata: if use_enum_fallback(cx) {
                     match self.discriminant_type_metadata {
+                        // Discriminant is always the first field of our variant
+                        // when using the enum fallback.
                         Some(metadata) if i == 0 => metadata,
                         _ => type_metadata(cx, ty, self.span)
                     }
@@ -1504,7 +1511,7 @@ impl VariantMemberDescriptionFactory<'ll, 'tcx> {
 
 #[derive(Copy, Clone)]
 enum EnumDiscriminantInfo<'ll> {
-    RegularDiscriminant(&'ll DIType),
+    RegularDiscriminant{ discr_field: Field, discr_type_metadata: &'ll DIType },
     OptimizedDiscriminant,
     NoDiscriminant
 }
@@ -1535,15 +1542,26 @@ fn describe_enum_variant(
                                            unique_type_id,
                                            Some(containing_scope));
 
+    let arg_name = |i: usize| {
+        if variant.ctor_kind == CtorKind::Fn {
+            format!("__{}", i)
+        } else {
+            variant.fields[i].ident.to_string()
+        }
+    };
+
     // Build an array of (field name, field type) pairs to be captured in the factory closure.
     let (offsets, args) = if use_enum_fallback(cx) {
         // If this is not a univariant enum, there is also the discriminant field.
         let (discr_offset, discr_arg) = match discriminant_info {
-            RegularDiscriminant(_) => {
+            RegularDiscriminant { discr_field, .. } => {
                 // We have the layout of an enum variant, we need the layout of the outer enum
                 let enum_layout = cx.layout_of(layout.ty);
-                (Some(enum_layout.fields.offset(0)),
-                 Some(("RUST$ENUM$DISR".to_owned(), enum_layout.field(cx, 0).ty)))
+                let offset = enum_layout.fields.offset(discr_field.as_usize());
+                let args = (
+                    "RUST$ENUM$DISR".to_owned(),
+                    enum_layout.field(cx, discr_field.as_usize()).ty);
+                (Some(offset), Some(args))
             }
             _ => (None, None),
         };
@@ -1552,12 +1570,7 @@ fn describe_enum_variant(
                 layout.fields.offset(i)
             })).collect(),
             discr_arg.into_iter().chain((0..layout.fields.count()).map(|i| {
-                let name = if variant.ctor_kind == CtorKind::Fn {
-                    format!("__{}", i)
-                } else {
-                    variant.fields[i].ident.to_string()
-                };
-                (name, layout.field(cx, i).ty)
+                (arg_name(i), layout.field(cx, i).ty)
             })).collect()
         )
     } else {
@@ -1566,12 +1579,7 @@ fn describe_enum_variant(
                 layout.fields.offset(i)
             }).collect(),
             (0..layout.fields.count()).map(|i| {
-                let name = if variant.ctor_kind == CtorKind::Fn {
-                    format!("__{}", i)
-                } else {
-                    variant.fields[i].ident.to_string()
-                };
-                (name, layout.field(cx, i).ty)
+                (arg_name(i), layout.field(cx, i).ty)
             }).collect()
         )
     };
@@ -1581,8 +1589,8 @@ fn describe_enum_variant(
             offsets,
             args,
             discriminant_type_metadata: match discriminant_info {
-                RegularDiscriminant(discriminant_type_metadata) => {
-                    Some(discriminant_type_metadata)
+                RegularDiscriminant { discr_type_metadata, .. } => {
+                    Some(discr_type_metadata)
                 }
                 _ => None
             },
@@ -1732,6 +1740,7 @@ fn prepare_enum_metadata(
         layout::Variants::Multiple {
             discr_kind: layout::DiscriminantKind::Niche { .. },
             ref discr,
+            discr_index,
             ..
         } => {
             // Find the integer type of the correct size.
@@ -1755,7 +1764,7 @@ fn prepare_enum_metadata(
                     UNKNOWN_LINE_NUMBER,
                     size.bits(),
                     align.abi.bits() as u32,
-                    layout.fields.offset(0).bits(),
+                    layout.fields.offset(discr_index).bits(),
                     DIFlags::FlagArtificial,
                     discr_metadata))
             }
@@ -1764,6 +1773,7 @@ fn prepare_enum_metadata(
         layout::Variants::Multiple {
             discr_kind: layout::DiscriminantKind::Tag,
             ref discr,
+            discr_index,
             ..
         } => {
             let discr_type = discr.value.to_ty(cx.tcx);
@@ -1779,7 +1789,7 @@ fn prepare_enum_metadata(
                     UNKNOWN_LINE_NUMBER,
                     size.bits(),
                     align.bits() as u32,
-                    layout.fields.offset(0).bits(),
+                    layout.fields.offset(discr_index).bits(),
                     DIFlags::FlagArtificial,
                     discr_metadata))
             }

src/librustc_codegen_llvm/type_of.rs

@@ -452,31 +452,27 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyLayout<'tcx> {
 
             _ => {
                 let mut data_variant = match self.variants {
+                    // Within the discriminant field, only the niche itself is
+                    // always initialized, so we only check for a pointer at its
+                    // offset.
+                    //
+                    // If the niche is a pointer, it's either valid (according
+                    // to its type), or null (which the niche field's scalar
+                    // validity range encodes).  This allows using
+                    // `dereferenceable_or_null` for e.g., `Option<&T>`, and
+                    // this will continue to work as long as we don't start
+                    // using more niches than just null (e.g., the first page of
+                    // the address space, or unaligned pointers).
                     layout::Variants::Multiple {
                         discr_kind: layout::DiscriminantKind::Niche {
                             dataful_variant,
                             ..
                         },
+                        discr_index,
                         ..
-                    } => {
-                        // Only the niche itself is always initialized,
-                        // so only check for a pointer at its offset.
-                        //
-                        // If the niche is a pointer, it's either valid
-                        // (according to its type), or null (which the
-                        // niche field's scalar validity range encodes).
-                        // This allows using `dereferenceable_or_null`
-                        // for e.g., `Option<&T>`, and this will continue
-                        // to work as long as we don't start using more
-                        // niches than just null (e.g., the first page
-                        // of the address space, or unaligned pointers).
-                        if self.fields.offset(0) == offset {
-                            Some(self.for_variant(cx, dataful_variant))
-                        } else {
-                            None
-                        }
-                    }
-                    _ => Some(*self)
+                    } if self.fields.offset(discr_index) == offset =>
+                        Some(self.for_variant(cx, dataful_variant)),
+                    _ => Some(*self),
                 };
 
                 if let Some(variant) = data_variant {

src/librustc_codegen_ssa/mir/place.rs

@@ -216,19 +216,19 @@ impl<'a, 'tcx: 'a, V: CodegenObject> PlaceRef<'tcx, V> {
         if self.layout.abi.is_uninhabited() {
             return bx.cx().const_undef(cast_to);
         }
-        let (discr_scalar, discr_kind) = match self.layout.variants {
+        let (discr_scalar, discr_kind, discr_index) = match self.layout.variants {
             layout::Variants::Single { index } => {
                 let discr_val = self.layout.ty.ty_adt_def().map_or(
                     index.as_u32() as u128,
                     |def| def.discriminant_for_variant(bx.cx().tcx(), index).val);
                 return bx.cx().const_uint_big(cast_to, discr_val);
             }
-            layout::Variants::Multiple { ref discr, ref discr_kind, .. } => {
-                (discr, discr_kind)
+            layout::Variants::Multiple { ref discr, ref discr_kind, discr_index, .. } => {
+                (discr, discr_kind, discr_index)
             }
         };
 
-        let discr = self.project_field(bx, 0);
+        let discr = self.project_field(bx, discr_index);
         let lldiscr = bx.load_operand(discr).immediate();
         match *discr_kind {
             layout::DiscriminantKind::Tag => {
@@ -292,9 +292,10 @@ impl<'a, 'tcx: 'a, V: CodegenObject> PlaceRef<'tcx, V> {
             }
             layout::Variants::Multiple {
                 discr_kind: layout::DiscriminantKind::Tag,
+                discr_index,
                 ..
             } => {
-                let ptr = self.project_field(bx, 0);
+                let ptr = self.project_field(bx, discr_index);
                 let to = self.layout.ty.ty_adt_def().unwrap()
                     .discriminant_for_variant(bx.tcx(), variant_index)
                     .val;
@@ -309,6 +310,7 @@ impl<'a, 'tcx: 'a, V: CodegenObject> PlaceRef<'tcx, V> {
                     ref niche_variants,
                     niche_start,
                 },
+                discr_index,
                 ..
             } => {
                 if variant_index != dataful_variant {
@@ -321,7 +323,7 @@ impl<'a, 'tcx: 'a, V: CodegenObject> PlaceRef<'tcx, V> {
                         bx.memset(self.llval, fill_byte, size, self.align, MemFlags::empty());
                     }
 
-                    let niche = self.project_field(bx, 0);
+                    let niche = self.project_field(bx, discr_index);
                     let niche_llty = bx.cx().immediate_backend_type(niche.layout);
                     let niche_value = variant_index.as_u32() - niche_variants.start().as_u32();
                     let niche_value = (niche_value as u128)

src/librustc_lint/types.rs

@@ -820,6 +820,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for VariantSizeDifferences {
                     discr_kind: layout::DiscriminantKind::Tag,
                     ref discr,
                     ref variants,
+                    ..
                 } => (variants, discr),
                 _ => return,
             };

src/librustc_mir/interpret/operand.rs

@@ -588,18 +588,19 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tcx, M>
     ) -> EvalResult<'tcx, (u128, VariantIdx)> {
         trace!("read_discriminant_value {:#?}", rval.layout);
 
-        let discr_kind = match rval.layout.variants {
+        let (discr_kind, discr_index) = match rval.layout.variants {
             layout::Variants::Single { index } => {
                 let discr_val = rval.layout.ty.ty_adt_def().map_or(
                     index.as_u32() as u128,
                     |def| def.discriminant_for_variant(*self.tcx, index).val);
                 return Ok((discr_val, index));
             }
-            layout::Variants::Multiple { ref discr_kind, .. } => discr_kind,
+            layout::Variants::Multiple { ref discr_kind, discr_index, .. } =>
+                (discr_kind, discr_index),
         };
 
         // read raw discriminant value
-        let discr_op = self.operand_field(rval, 0)?;
+        let discr_op = self.operand_field(rval, discr_index as u64)?;
         let discr_val = self.read_immediate(discr_op)?;
         let raw_discr = discr_val.to_scalar_or_undef();
         trace!("discr value: {:?}", raw_discr);

src/librustc_mir/interpret/place.rs

@@ -997,6 +997,7 @@ where
             layout::Variants::Multiple {
                 discr_kind: layout::DiscriminantKind::Tag,
                 ref discr,
+                discr_index,
                 ..
             } => {
                 let adt_def = dest.layout.ty.ty_adt_def().unwrap();
@@ -1011,7 +1012,7 @@ where
                 let size = discr.value.size(self);
                 let discr_val = truncate(discr_val, size);
 
-                let discr_dest = self.place_field(dest, 0)?;
+                let discr_dest = self.place_field(dest, discr_index as u64)?;
                 self.write_scalar(Scalar::from_uint(discr_val, size), discr_dest)?;
             }
             layout::Variants::Multiple {
@@ -1020,14 +1021,15 @@ where
                     ref niche_variants,
                     niche_start,
                 },
+                discr_index,
                 ..
             } => {
                 assert!(
                     variant_index.as_usize() < dest.layout.ty.ty_adt_def().unwrap().variants.len(),
                 );
                 if variant_index != dataful_variant {
                     let niche_dest =
-                        self.place_field(dest, 0)?;
+                        self.place_field(dest, discr_index as u64)?;
                     let niche_value = variant_index.as_u32() - niche_variants.start().as_u32();
                     let niche_value = (niche_value as u128)
                         .wrapping_add(niche_start);

src/librustc_target/abi/mod.rs

@@ -828,12 +828,13 @@ pub enum Variants {
         index: VariantIdx,
     },
 
-    /// Enums with more than one inhabited variant: for each case there is
-    /// a struct, and they all have space reserved for the discriminant,
-    /// which is the sole field of the enum layout.
+    /// Enum-likes with more than one inhabited variant: for each case there is
+    /// a struct, and they all have space reserved for the discriminant.
+    /// For enums this is the sole field of the layout.
     Multiple {
         discr: Scalar,
         discr_kind: DiscriminantKind,
+        discr_index: usize,
         variants: IndexVec<VariantIdx, LayoutDetails>,
     },
 }
@@ -845,8 +846,9 @@ pub enum DiscriminantKind {
 
     /// Niche (values invalid for a type) encoding the discriminant:
     /// the variant `dataful_variant` contains a niche at an arbitrary
-    /// offset (field 0 of the enum), which for a variant with discriminant
-    /// `d` is set to `(d - niche_variants.start).wrapping_add(niche_start)`.
+    /// offset (field `discr_index` of the enum), which for a variant with
+    /// discriminant `d` is set to
+    /// `(d - niche_variants.start).wrapping_add(niche_start)`.
     ///
     /// For example, `Option<(usize, &T)>`  is represented such that
     /// `None` has a null pointer for the second tuple field, and