Resolve PR comments

wesleywiser · wesleywiser · commit e63b9920302e · 2019-08-22T06:36:31.000-04:00
diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs
@@ -924,8 +924,11 @@ impl<'a, 'tcx> CrateMetadata {
         }
     }
 
-    pub fn maybe_get_promoted_mir(&self, tcx: TyCtxt<'tcx>, id: DefIndex) ->
-        Option<IndexVec<Promoted, Body<'tcx>>> {
+    pub fn maybe_get_promoted_mir(
+        &self,
+        tcx: TyCtxt<'tcx>,
+        id: DefIndex,
+    ) -> Option<IndexVec<Promoted, Body<'tcx>>> {
         match self.is_proc_macro(id) {
             true => None,
             false => self.entry(id).promoted_mir.map(|promoted| promoted.decode((self, tcx)),)
diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs
@@ -1060,8 +1060,10 @@ impl EncodeContext<'tcx> {
         }
     }
 
-    fn encode_promoted_mir(&mut self, def_id: DefId) ->
-        Option<Lazy<IndexVec<mir::Promoted, mir::Body<'tcx>>>> {
+    fn encode_promoted_mir(
+        &mut self,
+        def_id: DefId,
+    ) -> Option<Lazy<IndexVec<mir::Promoted, mir::Body<'tcx>>>> {
         debug!("EncodeContext::encode_promoted_mir({:?})", def_id);
         if self.tcx.mir_keys(LOCAL_CRATE).contains(&def_id) {
             let promoted = self.tcx.promoted_mir(def_id);
diff --git a/src/librustc_mir/borrow_check/nll/renumber.rs b/src/librustc_mir/borrow_check/nll/renumber.rs
@@ -16,7 +16,11 @@ pub fn renumber_mir<'tcx>(
     debug!("renumber_mir: body.arg_count={:?}", body.arg_count);
 
     let mut visitor = NLLVisitor { infcx };
-    visitor.visit_promoted(promoted);
+
+    for body in promoted.iter_mut() {
+        visitor.visit_body(body);
+    }
+
     visitor.visit_body(body);
 }
 
@@ -47,13 +51,6 @@ impl<'a, 'tcx> NLLVisitor<'a, 'tcx> {
     {
         renumber_regions(self.infcx, value)
     }
-
-    fn visit_promoted(&mut self, promoted: &mut IndexVec<Promoted, Body<'tcx>>) {
-        debug!("visiting promoted mir");
-        for body in promoted.iter_mut() {
-            self.visit_body(body);
-        }
-    }
 }
 
 impl<'a, 'tcx> MutVisitor<'tcx> for NLLVisitor<'a, 'tcx> {
diff --git a/src/librustc_mir/shim.rs b/src/librustc_mir/shim.rs
@@ -112,7 +112,7 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> &'tcx
     };
     debug!("make_shim({:?}) = untransformed {:?}", instance, result);
 
-    run_passes(tcx, &mut result, instance, MirPhase::Const, &[
+    run_passes(tcx, &mut result, instance, None, MirPhase::Const, &[
         &add_moves_for_packed_drops::AddMovesForPackedDrops,
         &no_landing_pads::NoLandingPads,
         &remove_noop_landing_pads::RemoveNoopLandingPads,
diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs
@@ -149,49 +149,46 @@ pub fn run_passes(
     tcx: TyCtxt<'tcx>,
     body: &mut Body<'tcx>,
     instance: InstanceDef<'tcx>,
+    promoted: Option<Promoted>,
     mir_phase: MirPhase,
     passes: &[&dyn MirPass<'tcx>],
 ) {
     let phase_index = mir_phase.phase_index();
 
-    let run_passes = |body: &mut Body<'tcx>, promoted| {
-        if body.phase >= mir_phase {
-            return;
-        }
+    if body.phase >= mir_phase {
+        return;
+    }
 
-        let source = MirSource {
-            instance,
-            promoted,
-        };
-        let mut index = 0;
-        let mut run_pass = |pass: &dyn MirPass<'tcx>| {
-            let run_hooks = |body: &_, index, is_after| {
-                dump_mir::on_mir_pass(tcx, &format_args!("{:03}-{:03}", phase_index, index),
-                                      &pass.name(), source, body, is_after);
-            };
-            run_hooks(body, index, false);
-            pass.run_pass(tcx, source, body);
-            run_hooks(body, index, true);
-
-            index += 1;
+    let source = MirSource {
+        instance,
+        promoted,
+    };
+    let mut index = 0;
+    let mut run_pass = |pass: &dyn MirPass<'tcx>| {
+        let run_hooks = |body: &_, index, is_after| {
+            dump_mir::on_mir_pass(tcx, &format_args!("{:03}-{:03}", phase_index, index),
+                                    &pass.name(), source, body, is_after);
         };
+        run_hooks(body, index, false);
+        pass.run_pass(tcx, source, body);
+        run_hooks(body, index, true);
 
-        for pass in passes {
-            run_pass(*pass);
-        }
-
-        body.phase = mir_phase;
+        index += 1;
     };
 
-    run_passes(body, None);
+    for pass in passes {
+        run_pass(*pass);
+    }
+
+    body.phase = mir_phase;
 }
 
 fn mir_const(tcx: TyCtxt<'_>, def_id: DefId) -> &Steal<Body<'_>> {
     // Unsafety check uses the raw mir, so make sure it is run
     let _ = tcx.unsafety_check_result(def_id);
 
     let mut body = tcx.mir_built(def_id).steal();
-    run_passes(tcx, &mut body, InstanceDef::Item(def_id), MirPhase::Const, &[
+    run_passes(tcx, &mut body, InstanceDef::Item(def_id), None, MirPhase::Const, &[
         // What we need to do constant evaluation.
         &simplify::SimplifyCfg::new("initial"),
         &rustc_peek::SanityCheck,
@@ -213,7 +210,7 @@ fn mir_validated(
 
     let mut body = tcx.mir_const(def_id).steal();
     let qualify_and_promote_pass = qualify_consts::QualifyAndPromoteConstants::default();
-    run_passes(tcx, &mut body, InstanceDef::Item(def_id), MirPhase::Validated, &[
+    run_passes(tcx, &mut body, InstanceDef::Item(def_id), None, MirPhase::Validated, &[
         // What we need to run borrowck etc.
         &qualify_and_promote_pass,
         &simplify::SimplifyCfg::new("qualify-consts"),
@@ -222,26 +219,13 @@ fn mir_validated(
     (tcx.alloc_steal_mir(body), tcx.alloc_steal_promoted(promoted))
 }
 
-fn optimized_mir(tcx: TyCtxt<'_>, def_id: DefId) -> &Body<'_> {
-    if tcx.is_constructor(def_id) {
-        // There's no reason to run all of the MIR passes on constructors when
-        // we can just output the MIR we want directly. This also saves const
-        // qualification and borrow checking the trouble of special casing
-        // constructors.
-        return shim::build_adt_ctor(tcx, def_id);
-    }
-
-    // (Mir-)Borrowck uses `mir_validated`, so we have to force it to
-    // execute before we can steal.
-    tcx.ensure().mir_borrowck(def_id);
-
-    if tcx.use_ast_borrowck() {
-        tcx.ensure().borrowck(def_id);
-    }
-
-    let (body, _) = tcx.mir_validated(def_id);
-    let mut body = body.steal();
-    run_passes(tcx, &mut body, InstanceDef::Item(def_id), MirPhase::Optimized, &[
+fn run_optimization_passes<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    body: &mut Body<'tcx>,
+    def_id: DefId,
+    promoted: Option<Promoted>,
+) {
+    run_passes(tcx, body, InstanceDef::Item(def_id), promoted, MirPhase::Optimized, &[
         // Remove all things only needed by analysis
         &no_landing_pads::NoLandingPads,
         &simplify_branches::SimplifyBranches::new("initial"),
@@ -292,6 +276,28 @@ fn optimized_mir(tcx: TyCtxt<'_>, def_id: DefId) -> &Body<'_> {
         &add_call_guards::CriticalCallEdges,
         &dump_mir::Marker("PreCodegen"),
     ]);
+}
+
+fn optimized_mir(tcx: TyCtxt<'_>, def_id: DefId) -> &Body<'_> {
+    if tcx.is_constructor(def_id) {
+        // There's no reason to run all of the MIR passes on constructors when
+        // we can just output the MIR we want directly. This also saves const
+        // qualification and borrow checking the trouble of special casing
+        // constructors.
+        return shim::build_adt_ctor(tcx, def_id);
+    }
+
+    // (Mir-)Borrowck uses `mir_validated`, so we have to force it to
+    // execute before we can steal.
+    tcx.ensure().mir_borrowck(def_id);
+
+    if tcx.use_ast_borrowck() {
+        tcx.ensure().borrowck(def_id);
+    }
+
+    let (body, _) = tcx.mir_validated(def_id);
+    let mut body = body.steal();
+    run_optimization_passes(tcx, &mut body, def_id, None);
     tcx.arena.alloc(body)
 }
 
@@ -304,57 +310,8 @@ fn promoted_mir<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> &'tcx IndexVec<Promot
     let (_, promoted) = tcx.mir_validated(def_id);
     let mut promoted = promoted.steal();
 
-    for mut body in promoted.iter_mut() {
-        run_passes(tcx, &mut body, InstanceDef::Item(def_id), MirPhase::Optimized, &[
-            // Remove all things only needed by analysis
-            &no_landing_pads::NoLandingPads,
-            &simplify_branches::SimplifyBranches::new("initial"),
-            &remove_noop_landing_pads::RemoveNoopLandingPads,
-            &cleanup_post_borrowck::CleanupNonCodegenStatements,
-
-            &simplify::SimplifyCfg::new("early-opt"),
-
-            // These next passes must be executed together
-            &add_call_guards::CriticalCallEdges,
-            &elaborate_drops::ElaborateDrops,
-            &no_landing_pads::NoLandingPads,
-            // AddMovesForPackedDrops needs to run after drop
-            // elaboration.
-            &add_moves_for_packed_drops::AddMovesForPackedDrops,
-            // AddRetag needs to run after ElaborateDrops, and it needs
-            // an AllCallEdges pass right before it.  Otherwise it should
-            // run fairly late, but before optimizations begin.
-            &add_call_guards::AllCallEdges,
-            &add_retag::AddRetag,
-
-            &simplify::SimplifyCfg::new("elaborate-drops"),
-
-            // No lifetime analysis based on borrowing can be done from here on out.
-
-            // From here on out, regions are gone.
-            &erase_regions::EraseRegions,
-
-            // Optimizations begin.
-            &uniform_array_move_out::RestoreSubsliceArrayMoveOut,
-            &inline::Inline,
-
-            // Lowering generator control-flow and variables
-            // has to happen before we do anything else to them.
-            &generator::StateTransform,
-
-            &instcombine::InstCombine,
-            &const_prop::ConstProp,
-            &simplify_branches::SimplifyBranches::new("after-const-prop"),
-            &deaggregator::Deaggregator,
-            &copy_prop::CopyPropagation,
-            &simplify_branches::SimplifyBranches::new("after-copy-prop"),
-            &remove_noop_landing_pads::RemoveNoopLandingPads,
-            &simplify::SimplifyCfg::new("final"),
-            &simplify::SimplifyLocals,
-
-            &add_call_guards::CriticalCallEdges,
-            &dump_mir::Marker("PreCodegen"),
-        ]);
+    for (p, mut body) in promoted.iter_enumerated_mut() {
+        run_optimization_passes(tcx, &mut body, def_id, Some(p));
     }
 
     tcx.intern_promoted(promoted)
diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs
@@ -304,12 +304,17 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> {
         let mut operand = {
             let promoted = &mut self.promoted;
             let promoted_id = Promoted::new(next_promoted_id);
-            let mut promoted_place = |ty, substs, span| {
+            let tcx = self.tcx;
+            let mut promoted_place = |ty, span| {
                 promoted.span = span;
                 promoted.local_decls[RETURN_PLACE] = LocalDecl::new_return_place(ty, span);
                 Place {
                     base: PlaceBase::Static(box Static {
-                        kind: StaticKind::Promoted(promoted_id, substs),
+                        kind:
+                            StaticKind::Promoted(
+                                promoted_id,
+                                InternalSubsts::identity_for_item(tcx, def_id),
+                            ),
                         ty,
                         def_id,
                     }),
@@ -329,11 +334,7 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> {
                             Operand::Move(Place {
                                 base: mem::replace(
                                     &mut place.base,
-                                    promoted_place(
-                                        ty,
-                                        InternalSubsts::identity_for_item(self.tcx, def_id),
-                                        span,
-                                    ).base
+                                    promoted_place(ty, span).base
                                 ),
                                 projection: None,
                             })
@@ -349,13 +350,7 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> {
                             let span = statement.source_info.span;
                             mem::replace(
                                 operand,
-                                Operand::Copy(
-                                    promoted_place(
-                                        ty,
-                                        InternalSubsts::identity_for_item(self.tcx, def_id),
-                                        span,
-                                    )
-                                )
+                                Operand::Copy(promoted_place(ty, span))
                             )
                         }
                         _ => bug!()
@@ -367,12 +362,7 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> {
                         TerminatorKind::Call { ref mut args, .. } => {
                             let ty = args[index].ty(local_decls, self.tcx);
                             let span = terminator.source_info.span;
-                            let operand =
-                                Operand::Copy(
-                                    promoted_place(
-                                        ty,
-                                        InternalSubsts::identity_for_item(self.tcx, def_id),
-                                        span));
+                            let operand = Operand::Copy(promoted_place(ty, span));
                             mem::replace(&mut args[index], operand)
                         }
                         // We expected a `TerminatorKind::Call` for which we'd like to promote an
@@ -472,6 +462,7 @@ pub fn promote_candidates<'tcx>(
             keep_original: false
         };
 
+        //FIXME(oli-obk): having a `maybe_push()` method on `IndexVec` might be nice
         if let Some(promoted) = promoter.promote_candidate(def_id, candidate, promotions.len()) {
             promotions.push(promoted);
         }

Original file line number	Diff line number	Diff line change
`@@ -924,8 +924,11 @@ impl<'a, 'tcx> CrateMetadata {`
`924`	`924`	`}`
`925`	`925`	`}`
`926`	`926`
`927`		`- pub fn maybe_get_promoted_mir(&self, tcx: TyCtxt<'tcx>, id: DefIndex) ->`
`928`		`- Option<IndexVec<Promoted, Body<'tcx>>> {`
	`927`	`+ pub fn maybe_get_promoted_mir(`
	`928`	`+ &self,`
	`929`	`+ tcx: TyCtxt<'tcx>,`
	`930`	`+ id: DefIndex,`
	`931`	`+ ) -> Option<IndexVec<Promoted, Body<'tcx>>> {`
`929`	`932`	`match self.is_proc_macro(id) {`
`930`	`933`	`true => None,`
`931`	`934`	`false => self.entry(id).promoted_mir.map(\|promoted\| promoted.decode((self, tcx)),)`
Original file line number	Diff line number	Diff line change
`@@ -1060,8 +1060,10 @@ impl EncodeContext<'tcx> {`
`1060`	`1060`	`}`
`1061`	`1061`	`}`
`1062`	`1062`
`1063`		`- fn encode_promoted_mir(&mut self, def_id: DefId) ->`
`1064`		`- Option<Lazy<IndexVec<mir::Promoted, mir::Body<'tcx>>>> {`
	`1063`	`+ fn encode_promoted_mir(`
	`1064`	`+ &mut self,`
	`1065`	`+ def_id: DefId,`
	`1066`	`+ ) -> Option<Lazy<IndexVec<mir::Promoted, mir::Body<'tcx>>>> {`
`1065`	`1067`	`debug!("EncodeContext::encode_promoted_mir({:?})", def_id);`
`1066`	`1068`	`if self.tcx.mir_keys(LOCAL_CRATE).contains(&def_id) {`
`1067`	`1069`	`let promoted = self.tcx.promoted_mir(def_id);`
Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,11 @@ pub fn renumber_mir<'tcx>(`
`16`	`16`	`debug!("renumber_mir: body.arg_count={:?}", body.arg_count);`
`17`	`17`
`18`	`18`	`let mut visitor = NLLVisitor { infcx };`
`19`		`- visitor.visit_promoted(promoted);`
	`19`	`+`
	`20`	`+ for body in promoted.iter_mut() {`
	`21`	`+ visitor.visit_body(body);`
	`22`	`+ }`
	`23`	`+`
`20`	`24`	`visitor.visit_body(body);`
`21`	`25`	`}`
`22`	`26`
`@@ -47,13 +51,6 @@ impl<'a, 'tcx> NLLVisitor<'a, 'tcx> {`
`47`	`51`	`{`
`48`	`52`	`renumber_regions(self.infcx, value)`
`49`	`53`	`}`
`50`		`-`
`51`		`- fn visit_promoted(&mut self, promoted: &mut IndexVec<Promoted, Body<'tcx>>) {`
`52`		`- debug!("visiting promoted mir");`
`53`		`- for body in promoted.iter_mut() {`
`54`		`- self.visit_body(body);`
`55`		`- }`
`56`		`- }`
`57`	`54`	`}`
`58`	`55`
`59`	`56`	`impl<'a, 'tcx> MutVisitor<'tcx> for NLLVisitor<'a, 'tcx> {`