Address review comments

9 files changed, 39 insertions, 51 deletions

diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs
index 9c71e694f4c..e77315f06a9 100644
--- a/src/librustc/mir/mod.rs
+++ b/src/librustc/mir/mod.rs
@@ -2995,7 +2995,7 @@ pub struct UnsafetyCheckResult {
 }
 
 newtype_index! {
-    pub struct GeneratorField {
+    pub struct GeneratorSavedLocal {
         derive [HashStable]
         DEBUG_FORMAT = "_{}",
     }
@@ -3005,18 +3005,18 @@ newtype_index! {
 #[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable)]
 pub struct GeneratorLayout<'tcx> {
     /// The type of every local stored inside the generator.
-    pub field_tys: IndexVec<GeneratorField, Ty<'tcx>>,
+    pub field_tys: IndexVec<GeneratorSavedLocal, Ty<'tcx>>,
 
     /// Which of the above fields are in each variant. Note that one field may
     /// be stored in multiple variants.
-    pub variant_fields: IndexVec<VariantIdx, IndexVec<Field, GeneratorField>>,
+    pub variant_fields: IndexVec<VariantIdx, IndexVec<Field, GeneratorSavedLocal>>,
 
     /// Names and scopes of all the stored generator locals.
     /// NOTE(tmandry) This is *strictly* a temporary hack for codegen
     /// debuginfo generation, and will be removed at some point.
     /// Do **NOT** use it for anything else, local information should not be
     /// in the MIR, please rely on local crate HIR or other side-channels.
-    pub __local_debuginfo_codegen_only_do_not_use: IndexVec<GeneratorField, LocalDecl<'tcx>>,
+    pub __local_debuginfo_codegen_only_do_not_use: IndexVec<GeneratorSavedLocal, LocalDecl<'tcx>>,
 }
 
 #[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable)]
@@ -3582,7 +3582,7 @@ impl<'tcx> TypeFoldable<'tcx> for Field {
     }
 }
 
-impl<'tcx> TypeFoldable<'tcx> for GeneratorField {
+impl<'tcx> TypeFoldable<'tcx> for GeneratorSavedLocal {
     fn super_fold_with<'gcx: 'tcx, F: TypeFolder<'gcx, 'tcx>>(&self, _: &mut F) -> Self {
         *self
     }
diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs
index 10afaebc91a..1dbb5a8f0bf 100644
--- a/src/librustc/ty/layout.rs
+++ b/src/librustc/ty/layout.rs
@@ -605,6 +605,12 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> {
             }
 
             ty::Generator(def_id, ref substs, _) => {
+                // FIXME(tmandry): For fields that are repeated in multiple
+                // variants in the GeneratorLayout, we need code to ensure that
+                // the offset of these fields never change. Right now this is
+                // not an issue since every variant has every field, but once we
+                // optimize this we have to be more careful.
+
                 let discr_index = substs.prefix_tys(def_id, tcx).count();
                 let prefix_tys = substs.prefix_tys(def_id, tcx)
                     .chain(iter::once(substs.discr_ty(tcx)));
@@ -1691,7 +1697,7 @@ impl<'a, 'tcx, C> TyLayoutMethods<'tcx, C> for Ty<'tcx>
 
     fn field(this: TyLayout<'tcx>, cx: &C, i: usize) -> C::TyLayout {
         let tcx = cx.tcx();
-        let handle_discriminant = |discr: &Scalar| -> C::TyLayout {
+        let discr_layout = |discr: &Scalar| -> C::TyLayout {
             let layout = LayoutDetails::scalar(cx, discr.clone());
             MaybeResult::from_ok(TyLayout {
                 details: tcx.intern_layout(layout),
@@ -1781,7 +1787,7 @@ impl<'a, 'tcx, C> TyLayoutMethods<'tcx, C> for Ty<'tcx>
                     }
                     Variants::Multiple { ref discr, discr_index, .. } => {
                         if i == discr_index {
-                            return handle_discriminant(discr);
+                            return discr_layout(discr);
                         }
                         substs.prefix_tys(def_id, tcx).nth(i).unwrap()
                     }
@@ -1805,7 +1811,7 @@ impl<'a, 'tcx, C> TyLayoutMethods<'tcx, C> for Ty<'tcx>
                     // Discriminant field for enums (where applicable).
                     Variants::Multiple { ref discr, .. } => {
                         assert_eq!(i, 0);
-                        return handle_discriminant(discr);
+                        return discr_layout(discr);
                     }
                 }
             }
diff --git a/src/librustc/ty/sty.rs b/src/librustc/ty/sty.rs
index a41209496bd..44e452d8373 100644
--- a/src/librustc/ty/sty.rs
+++ b/src/librustc/ty/sty.rs
@@ -16,6 +16,7 @@ use crate::util::captures::Captures;
 use crate::mir::interpret::{Scalar, Pointer};
 
 use smallvec::SmallVec;
+use std::borrow::Cow;
 use std::cmp::Ordering;
 use std::marker::PhantomData;
 use std::ops::Range;
@@ -513,16 +514,13 @@ impl<'a, 'gcx, 'tcx> GeneratorSubsts<'tcx> {
     /// Calls `f` with a reference to the name of the enumerator for the given
     /// variant `v`.
     #[inline]
-    pub fn map_variant_name<R>(&self, v: VariantIdx, f: impl FnOnce(&str) -> R) -> R {
-        let name = match v.as_usize() {
-            Self::UNRESUMED => Self::UNRESUMED_NAME,
-            Self::RETURNED => Self::RETURNED_NAME,
-            Self::POISONED => Self::POISONED_NAME,
-            _ => {
-                return f(&format!("variant#{}", v.as_usize()));
-            }
-        };
-        f(name)
+    pub fn variant_name(&self, v: VariantIdx) -> Cow<'static, str> {
+        match v.as_usize() {
+            Self::UNRESUMED => Cow::from(Self::UNRESUMED_NAME),
+            Self::RETURNED => Cow::from(Self::RETURNED_NAME),
+            Self::POISONED => Cow::from(Self::POISONED_NAME),
+            _ => Cow::from(format!("Suspend{}", v.as_usize() - 3))
+        }
     }
 
     /// The type of the state discriminant used in the generator type.
diff --git a/src/librustc_codegen_llvm/debuginfo/metadata.rs b/src/librustc_codegen_llvm/debuginfo/metadata.rs
index 5b18e821982..4ab197c6460 100644
--- a/src/librustc_codegen_llvm/debuginfo/metadata.rs
+++ b/src/librustc_codegen_llvm/debuginfo/metadata.rs
@@ -1575,7 +1575,7 @@ impl<'tcx> VariantInfo<'tcx> {
         match self {
             VariantInfo::Adt(variant) => f(&variant.ident.as_str()),
             VariantInfo::Generator(substs, _, variant_index) =>
-                substs.map_variant_name(*variant_index, f),
+                f(&substs.variant_name(*variant_index)),
         }
     }
 
@@ -1720,16 +1720,16 @@ fn prepare_enum_metadata(
                 .collect(),
             ty::Generator(_, substs, _) => substs
                 .variant_range(enum_def_id, cx.tcx)
-                .map(|v| substs.map_variant_name(v, |name| {
-                    let name = SmallCStr::new(name);
+                .map(|variant_index| {
+                    let name = SmallCStr::new(&substs.variant_name(variant_index));
                     unsafe {
                         Some(llvm::LLVMRustDIBuilderCreateEnumerator(
                             DIB(cx),
                             name.as_ptr(),
                             // FIXME: what if enumeration has i128 discriminant?
-                            v.as_usize() as u64))
+                            variant_index.as_usize() as u64))
                     }
-                }))
+                })
                 .collect(),
             _ => bug!(),
         };
diff --git a/src/librustc_codegen_llvm/type_of.rs b/src/librustc_codegen_llvm/type_of.rs
index 080f78ff112..cbcc457fda9 100644
--- a/src/librustc_codegen_llvm/type_of.rs
+++ b/src/librustc_codegen_llvm/type_of.rs
@@ -63,10 +63,10 @@ fn uncached_llvm_type<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>,
                     write!(&mut name, "::{}", def.variants[index].ident).unwrap();
                 }
             }
-            if let (&ty::Generator(..), &layout::Variants::Single { index })
+            if let (&ty::Generator(_, substs, _), &layout::Variants::Single { index })
                  = (&layout.ty.sty, &layout.variants)
             {
-                write!(&mut name, "::variant#{:?}", index).unwrap();
+                write!(&mut name, "::{}", substs.variant_name(index)).unwrap();
             }
             Some(name)
         }
diff --git a/src/librustc_mir/borrow_check/nll/type_check/mod.rs b/src/librustc_mir/borrow_check/nll/type_check/mod.rs
index 94900b98a52..a076f4aab9d 100644
--- a/src/librustc_mir/borrow_check/nll/type_check/mod.rs
+++ b/src/librustc_mir/borrow_check/nll/type_check/mod.rs
@@ -684,25 +684,8 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> {
                         }
                     }
                 }
-                ty::Generator(def_id, substs, _) => {
-                    let variants = substs.state_tys(def_id, tcx).count();
-                    if index.as_usize() >= variants {
-                        PlaceTy::from_ty(
-                            span_mirbug_and_err!(
-                                self,
-                                place,
-                                "cast to variant #{:?} but generator only has {:?}",
-                                index,
-                                variants
-                            ),
-                        )
-                    } else {
-                        PlaceTy {
-                            ty: base_ty,
-                            variant_index: Some(index),
-                        }
-                    }
-                }
+                // We do not need to handle generators here, because this runs
+                // before the generator transform stage.
                 _ => {
                     let ty = if let Some(name) = maybe_name {
                         span_mirbug_and_err!(
diff --git a/src/librustc_mir/transform/deaggregator.rs b/src/librustc_mir/transform/deaggregator.rs
index 9f8d40bf4cd..9215356daa4 100644
--- a/src/librustc_mir/transform/deaggregator.rs
+++ b/src/librustc_mir/transform/deaggregator.rs
@@ -58,7 +58,7 @@ impl MirPass for Deaggregator {
                     }
                     AggregateKind::Generator(..) => {
                         // Right now we only support initializing generators to
-                        // variant#0.
+                        // variant 0 (Unresumed).
                         let variant_index = VariantIdx::new(0);
                         set_discriminant = Some(Statement {
                             kind: StatementKind::SetDiscriminant {
diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs
index 77bf789d2dd..e35787a9871 100644
--- a/src/librustc_mir/transform/generator.rs
+++ b/src/librustc_mir/transform/generator.rs
@@ -561,12 +561,13 @@ fn compute_layout<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
         remap.insert(local, (var.ty, variant_index, idx));
         decls.push(var);
     }
-    let field_tys = decls.iter().map(|field| field.ty).collect::<IndexVec<GeneratorField, _>>();
+    let field_tys = decls.iter().map(|field| field.ty).collect::<IndexVec<_, _>>();
 
     // Put every var in each variant, for now.
-    let all_vars = (0..field_tys.len()).map(GeneratorField::from).collect();
+    let all_vars = (0..field_tys.len()).map(GeneratorSavedLocal::from).collect();
     let empty_variants = iter::repeat(IndexVec::new()).take(3);
     let state_variants = iter::repeat(all_vars).take(suspending_blocks.count());
+
     let layout = GeneratorLayout {
         field_tys,
         variant_fields: empty_variants.chain(state_variants).collect(),
diff --git a/src/test/debuginfo/generator-objects.rs b/src/test/debuginfo/generator-objects.rs
index 682c213f332..c6f98e5782b 100644
--- a/src/test/debuginfo/generator-objects.rs
+++ b/src/test/debuginfo/generator-objects.rs
@@ -10,16 +10,16 @@
 
 // gdb-command:run
 // gdb-command:print b
-// gdb-check:$1 = generator_objects::main::generator {__0: 0x[...], <<variant>>: {__state: 0, 0: generator_objects::main::generator::Unresumed, 1: generator_objects::main::generator::Returned, 2: generator_objects::main::generator::Panicked, 3: generator_objects::main::generator::variant#3 {[...]}, 4: generator_objects::main::generator::variant#4 {[...]}}}
+// gdb-check:$1 = generator_objects::main::generator {__0: 0x[...], <<variant>>: {__state: 0, 0: generator_objects::main::generator::Unresumed, 1: generator_objects::main::generator::Returned, 2: generator_objects::main::generator::Panicked, 3: generator_objects::main::generator::Suspend0 {[...]}, 4: generator_objects::main::generator::Suspend1 {[...]}}}
 // gdb-command:continue
 // gdb-command:print b
-// gdb-check:$2 = generator_objects::main::generator {__0: 0x[...], <<variant>>: {__state: 3, 0: generator_objects::main::generator::Unresumed, 1: generator_objects::main::generator::Returned, 2: generator_objects::main::generator::Panicked, 3: generator_objects::main::generator::variant#3 {c: 6, d: 7}, 4: generator_objects::main::generator::variant#4 {[...]}}}
+// gdb-check:$2 = generator_objects::main::generator {__0: 0x[...], <<variant>>: {__state: 3, 0: generator_objects::main::generator::Unresumed, 1: generator_objects::main::generator::Returned, 2: generator_objects::main::generator::Panicked, 3: generator_objects::main::generator::Suspend0 {c: 6, d: 7}, 4: generator_objects::main::generator::Suspend1 {[...]}}}
 // gdb-command:continue
 // gdb-command:print b
-// gdb-check:$3 = generator_objects::main::generator {__0: 0x[...], <<variant>>: {__state: 4, 0: generator_objects::main::generator::Unresumed, 1: generator_objects::main::generator::Returned, 2: generator_objects::main::generator::Panicked, 3: generator_objects::main::generator::variant#3 {[...]}, 4: generator_objects::main::generator::variant#4 {c: 7, d: 8}}}
+// gdb-check:$3 = generator_objects::main::generator {__0: 0x[...], <<variant>>: {__state: 4, 0: generator_objects::main::generator::Unresumed, 1: generator_objects::main::generator::Returned, 2: generator_objects::main::generator::Panicked, 3: generator_objects::main::generator::Suspend0 {[...]}, 4: generator_objects::main::generator::Suspend1 {c: 7, d: 8}}}
 // gdb-command:continue
 // gdb-command:print b
-// gdb-check:$4 = generator_objects::main::generator {__0: 0x[...], <<variant>>: {__state: 1, 0: generator_objects::main::generator::Unresumed, 1: generator_objects::main::generator::Returned, 2: generator_objects::main::generator::Panicked, 3: generator_objects::main::generator::variant#3 {[...]}, 4: generator_objects::main::generator::variant#4 {[...]}}}
+// gdb-check:$4 = generator_objects::main::generator {__0: 0x[...], <<variant>>: {__state: 1, 0: generator_objects::main::generator::Unresumed, 1: generator_objects::main::generator::Returned, 2: generator_objects::main::generator::Panicked, 3: generator_objects::main::generator::Suspend0 {[...]}, 4: generator_objects::main::generator::Suspend1 {[...]}}}
 
 // === LLDB TESTS ==================================================================================