Auto merge of #50750 - est31:master, r=eddyb

Remove ScopeTarget and LoopIdResult

* Remove ScopeTarget in preparation of label-break-value (PR #50045)
* Replace LoopIdResult by Result which is possible now thanks to commit 8ac65af81f " Implement Encodable and Decodable for Result."

r? @eddyb

src/librustc/cfg/construct.rs

@@ -582,19 +582,16 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
                   scope_cf_kind: ScopeCfKind) -> (region::Scope, CFGIndex) {
 
         match destination.target_id {
-            hir::ScopeTarget::Block(block_expr_id) => {
+            Ok(loop_id) => {
                 for b in &self.breakable_block_scopes {
-                    if b.block_expr_id == self.tcx.hir.node_to_hir_id(block_expr_id).local_id {
+                    if b.block_expr_id == self.tcx.hir.node_to_hir_id(loop_id).local_id {
-                        let scope_id = self.tcx.hir.node_to_hir_id(block_expr_id).local_id;
+                        let scope_id = self.tcx.hir.node_to_hir_id(loop_id).local_id;
                         return (region::Scope::Node(scope_id), match scope_cf_kind {
                             ScopeCfKind::Break => b.break_index,
                             ScopeCfKind::Continue => bug!("can't continue to block"),
                         });
                     }
                 }
-                span_bug!(expr.span, "no block expr for id {}", block_expr_id);
-            }
-            hir::ScopeTarget::Loop(hir::LoopIdResult::Ok(loop_id)) => {
                 for l in &self.loop_scopes {
                     if l.loop_id == self.tcx.hir.node_to_hir_id(loop_id).local_id {
                         let scope_id = self.tcx.hir.node_to_hir_id(loop_id).local_id;
@@ -604,10 +601,9 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
                         });
                     }
                 }
-                span_bug!(expr.span, "no loop scope for id {}", loop_id);
+                span_bug!(expr.span, "no scope for id {}", loop_id);
             }
-            hir::ScopeTarget::Loop(hir::LoopIdResult::Err(err)) =>
+            Err(err) => span_bug!(expr.span, "scope error: {}",  err),
-                span_bug!(expr.span, "loop scope error: {}",  err),
         }
     }
 }

src/librustc/hir/intravisit.rs

@@ -1039,10 +1039,8 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
             if let Some(ref label) = destination.label {
                 visitor.visit_label(label);
                 match destination.target_id {
-                    ScopeTarget::Block(node_id) |
+                    Ok(node_id) => visitor.visit_def_mention(Def::Label(node_id)),
-                    ScopeTarget::Loop(LoopIdResult::Ok(node_id)) =>
+                    Err(_) => {},
-                        visitor.visit_def_mention(Def::Label(node_id)),
-                    ScopeTarget::Loop(LoopIdResult::Err(_)) => {},
                 };
             }
             walk_list!(visitor, visit_expr, opt_expr);
@@ -1051,10 +1049,8 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
             if let Some(ref label) = destination.label {
                 visitor.visit_label(label);
                 match destination.target_id {
-                    ScopeTarget::Block(_) => bug!("can't `continue` to a non-loop block"),
+                    Ok(node_id) => visitor.visit_def_mention(Def::Label(node_id)),
-                    ScopeTarget::Loop(LoopIdResult::Ok(node_id)) =>
+                    Err(_) => {},
-                        visitor.visit_def_mention(Def::Label(node_id)),
-                    ScopeTarget::Loop(LoopIdResult::Err(_)) => {},
                 };
             }
         }

src/librustc/hir/lowering.rs

@@ -928,29 +928,27 @@ impl<'a> LoweringContext<'a> {
     fn lower_loop_destination(&mut self, destination: Option<(NodeId, Label)>) -> hir::Destination {
         match destination {
             Some((id, label)) => {
-                let target = if let Def::Label(loop_id) = self.expect_full_def(id) {
+                let target_id = if let Def::Label(loop_id) = self.expect_full_def(id) {
-                    hir::LoopIdResult::Ok(self.lower_node_id(loop_id).node_id)
+                    Ok(self.lower_node_id(loop_id).node_id)
                 } else {
-                    hir::LoopIdResult::Err(hir::LoopIdError::UnresolvedLabel)
+                    Err(hir::LoopIdError::UnresolvedLabel)
                 };
                 hir::Destination {
                     label: self.lower_label(Some(label)),
-                    target_id: hir::ScopeTarget::Loop(target),
+                    target_id,
                 }
             }
             None => {
-                let loop_id = self.loop_scopes
+                let target_id = self.loop_scopes
                     .last()
-                    .map(|innermost_loop_id| *innermost_loop_id);
+                    .map(|innermost_loop_id| *innermost_loop_id)
+                    .map(|id| Ok(self.lower_node_id(id).node_id))
+                    .unwrap_or(Err(hir::LoopIdError::OutsideLoopScope))
+                    .into();
 
                 hir::Destination {
                     label: None,
-                    target_id: hir::ScopeTarget::Loop(
+                    target_id,
-                        loop_id
-                            .map(|id| Ok(self.lower_node_id(id).node_id))
-                            .unwrap_or(Err(hir::LoopIdError::OutsideLoopScope))
-                            .into(),
-                    ),
                 }
             }
         }
@@ -3192,9 +3190,7 @@ impl<'a> LoweringContext<'a> {
                 let destination = if self.is_in_loop_condition && opt_label.is_none() {
                     hir::Destination {
                         label: None,
-                        target_id: hir::ScopeTarget::Loop(
+                        target_id: Err(hir::LoopIdError::UnlabeledCfInWhileCondition).into(),
-                            Err(hir::LoopIdError::UnlabeledCfInWhileCondition).into(),
-                        ),
                     }
                 } else {
                     self.lower_loop_destination(opt_label.map(|label| (e.id, label)))
@@ -3208,9 +3204,7 @@ impl<'a> LoweringContext<'a> {
                 hir::ExprAgain(if self.is_in_loop_condition && opt_label.is_none() {
                     hir::Destination {
                         label: None,
-                        target_id: hir::ScopeTarget::Loop(
+                        target_id: Err(hir::LoopIdError::UnlabeledCfInWhileCondition).into(),
-                            Err(hir::LoopIdError::UnlabeledCfInWhileCondition).into(),
-                        ),
                     }
                 } else {
                     self.lower_loop_destination(opt_label.map(|label| (e.id, label)))
@@ -3603,7 +3597,7 @@ impl<'a> LoweringContext<'a> {
                             hir::ExprBreak(
                                 hir::Destination {
                                     label: None,
-                                    target_id: hir::ScopeTarget::Block(catch_node),
+                                    target_id: Ok(catch_node),
                                 },
                                 Some(from_err_expr),
                             ),

src/librustc/hir/mod.rs

@@ -1502,46 +1502,6 @@ impl fmt::Display for LoopIdError {
     }
 }
 
-// FIXME(cramertj) this should use `Result` once master compiles w/ a vesion of Rust where
-// `Result` implements `Encodable`/`Decodable`
-#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)]
-pub enum LoopIdResult {
-    Ok(NodeId),
-    Err(LoopIdError),
-}
-impl Into<Result<NodeId, LoopIdError>> for LoopIdResult {
-    fn into(self) -> Result<NodeId, LoopIdError> {
-        match self {
-            LoopIdResult::Ok(ok) => Ok(ok),
-            LoopIdResult::Err(err) => Err(err),
-        }
-    }
-}
-impl From<Result<NodeId, LoopIdError>> for LoopIdResult {
-    fn from(res: Result<NodeId, LoopIdError>) -> Self {
-        match res {
-            Ok(ok) => LoopIdResult::Ok(ok),
-            Err(err) => LoopIdResult::Err(err),
-        }
-    }
-}
-
-#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)]
-pub enum ScopeTarget {
-    Block(NodeId),
-    Loop(LoopIdResult),
-}
-
-impl ScopeTarget {
-    pub fn opt_id(self) -> Option<NodeId> {
-        match self {
-            ScopeTarget::Block(node_id) |
-            ScopeTarget::Loop(LoopIdResult::Ok(node_id)) => Some(node_id),
-            ScopeTarget::Loop(LoopIdResult::Err(_)) => None,
-        }
-    }
-}
-
 #[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)]
 pub struct Destination {
     // This is `Some(_)` iff there is an explicit user-specified `label
@@ -1549,7 +1509,7 @@ pub struct Destination {
 
     // These errors are caught and then reported during the diagnostics pass in
     // librustc_passes/loops.rs
-    pub target_id: ScopeTarget,
+    pub target_id: Result<NodeId, LoopIdError>,
 }
 
 #[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)]

src/librustc/ich/impls_hir.rs

@@ -656,22 +656,12 @@ impl_stable_hash_for!(struct hir::Destination {
 
 impl_stable_hash_for_spanned!(ast::Ident);
 
-impl_stable_hash_for!(enum hir::LoopIdResult {
-    Ok(node_id),
-    Err(loop_id_error)
-});
-
 impl_stable_hash_for!(enum hir::LoopIdError {
     OutsideLoopScope,
     UnlabeledCfInWhileCondition,
     UnresolvedLabel
 });
 
-impl_stable_hash_for!(enum hir::ScopeTarget {
-    Block(node_id),
-    Loop(loop_id_result)
-});
-
 impl<'a> HashStable<StableHashingContext<'a>> for ast::Ident {
     fn hash_stable<W: StableHasherResult>(&self,
                                           hcx: &mut StableHashingContext<'a>,

src/librustc/middle/liveness.rs

@@ -571,9 +571,6 @@ struct Liveness<'a, 'tcx: 'a> {
     // it probably doesn't now)
     break_ln: NodeMap<LiveNode>,
     cont_ln: NodeMap<LiveNode>,
-
-    // mappings from node ID to LiveNode for "breakable" blocks-- currently only `catch {...}`
-    breakable_block_ln: NodeMap<LiveNode>,
 }
 
 impl<'a, 'tcx> Liveness<'a, 'tcx> {
@@ -601,7 +598,6 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
             users: vec![invalid_users(); num_live_nodes * num_vars],
             break_ln: NodeMap(),
             cont_ln: NodeMap(),
-            breakable_block_ln: NodeMap(),
         }
     }
 
@@ -870,7 +866,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
     fn propagate_through_block(&mut self, blk: &hir::Block, succ: LiveNode)
                                -> LiveNode {
         if blk.targeted_by_break {
-            self.breakable_block_ln.insert(blk.id, succ);
+            self.break_ln.insert(blk.id, succ);
         }
         let succ = self.propagate_through_opt_expr(blk.expr.as_ref().map(|e| &**e), succ);
         blk.stmts.iter().rev().fold(succ, |succ, stmt| {
@@ -1055,12 +1051,8 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
           hir::ExprBreak(label, ref opt_expr) => {
               // Find which label this break jumps to
               let target = match label.target_id {
-                    hir::ScopeTarget::Block(node_id) =>
+                    Ok(node_id) => self.break_ln.get(&node_id),
-                        self.breakable_block_ln.get(&node_id),
+                    Err(err) => span_bug!(expr.span, "loop scope error: {}", err),
-                    hir::ScopeTarget::Loop(hir::LoopIdResult::Ok(node_id)) =>
-                        self.break_ln.get(&node_id),
-                    hir::ScopeTarget::Loop(hir::LoopIdResult::Err(err)) =>
-                        span_bug!(expr.span, "loop scope error: {}", err),
               }.map(|x| *x);
 
               // Now that we know the label we're going to,
@@ -1075,10 +1067,8 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
           hir::ExprAgain(label) => {
               // Find which label this expr continues to
               let sc = match label.target_id {
-                    hir::ScopeTarget::Block(_) => bug!("can't `continue` to a non-loop block"),
+                    Ok(node_id) => node_id,
-                    hir::ScopeTarget::Loop(hir::LoopIdResult::Ok(node_id)) => node_id,
+                    Err(err) => span_bug!(expr.span, "loop scope error: {}", err),
-                    hir::ScopeTarget::Loop(hir::LoopIdResult::Err(err)) =>
-                        span_bug!(expr.span, "loop scope error: {}", err),
               };
 
               // Now that we know the label we're going to,

src/librustc_mir/hair/cx/expr.rs

@@ -536,23 +536,19 @@ fn make_mirror_unadjusted<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
         hir::ExprRet(ref v) => ExprKind::Return { value: v.to_ref() },
         hir::ExprBreak(dest, ref value) => {
             match dest.target_id {
-                hir::ScopeTarget::Block(target_id) |
+                Ok(target_id) => ExprKind::Break {
-                hir::ScopeTarget::Loop(hir::LoopIdResult::Ok(target_id)) => ExprKind::Break {
                     label: region::Scope::Node(cx.tcx.hir.node_to_hir_id(target_id).local_id),
                     value: value.to_ref(),
                 },
-                hir::ScopeTarget::Loop(hir::LoopIdResult::Err(err)) =>
+                Err(err) => bug!("invalid loop id for break: {}", err)
-                    bug!("invalid loop id for break: {}", err)
             }
         }
         hir::ExprAgain(dest) => {
             match dest.target_id {
-                hir::ScopeTarget::Block(_) => bug!("cannot continue to blocks"),
+                Ok(loop_id) => ExprKind::Continue {
-                hir::ScopeTarget::Loop(hir::LoopIdResult::Ok(loop_id)) => ExprKind::Continue {
                     label: region::Scope::Node(cx.tcx.hir.node_to_hir_id(loop_id).local_id),
                 },
-                hir::ScopeTarget::Loop(hir::LoopIdResult::Err(err)) =>
+                Err(err) => bug!("invalid loop id for continue: {}", err)
-                    bug!("invalid loop id for continue: {}", err)
             }
         }
         hir::ExprMatch(ref discr, ref arms, _) => {

src/librustc_passes/loops.rs

@@ -85,20 +85,21 @@ impl<'a, 'hir> Visitor<'hir> for CheckLoopVisitor<'a, 'hir> {
                 self.with_context(Closure, |v| v.visit_nested_body(b));
             }
             hir::ExprBreak(label, ref opt_expr) => {
-                let loop_id = match label.target_id {
+                let loop_id = match label.target_id.into() {
-                    hir::ScopeTarget::Block(_) => return,
+                    Ok(loop_id) => loop_id,
-                    hir::ScopeTarget::Loop(loop_res) => {
+                    Err(hir::LoopIdError::OutsideLoopScope) => ast::DUMMY_NODE_ID,
-                        match loop_res.into() {
+                    Err(hir::LoopIdError::UnlabeledCfInWhileCondition) => {
-                            Ok(loop_id) => loop_id,
+                        self.emit_unlabled_cf_in_while_condition(e.span, "break");
-                            Err(hir::LoopIdError::OutsideLoopScope) => ast::DUMMY_NODE_ID,
+                        ast::DUMMY_NODE_ID
-                            Err(hir::LoopIdError::UnlabeledCfInWhileCondition) => {
+                    },
-                                self.emit_unlabled_cf_in_while_condition(e.span, "break");
+                    Err(hir::LoopIdError::UnresolvedLabel) => ast::DUMMY_NODE_ID,
-                                ast::DUMMY_NODE_ID
-                            },
-                            Err(hir::LoopIdError::UnresolvedLabel) => ast::DUMMY_NODE_ID,
-                        }
-                    }
                 };
+                if loop_id != ast::DUMMY_NODE_ID {
+                    match self.hir_map.find(loop_id).unwrap() {
+                        hir::map::NodeBlock(_) => return,
+                        _=> (),
+                    }
+                }
 
                 if opt_expr.is_some() {
                     let loop_kind = if loop_id == ast::DUMMY_NODE_ID {
@@ -132,9 +133,7 @@ impl<'a, 'hir> Visitor<'hir> for CheckLoopVisitor<'a, 'hir> {
                 self.require_loop("break", e.span);
             }
             hir::ExprAgain(label) => {
-                if let hir::ScopeTarget::Loop(
+                if let Err(hir::LoopIdError::UnlabeledCfInWhileCondition) = label.target_id {
-                    hir::LoopIdResult::Err(
-                        hir::LoopIdError::UnlabeledCfInWhileCondition)) = label.target_id {
                     self.emit_unlabled_cf_in_while_condition(e.span, "continue");
                 }
                 self.require_loop("continue", e.span)

src/librustc_typeck/check/mod.rs

@@ -3726,7 +3726,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
               tcx.mk_nil()
           }
           hir::ExprBreak(destination, ref expr_opt) => {
-              if let Some(target_id) = destination.target_id.opt_id() {
+              if let Ok(target_id) = destination.target_id {
                   let (e_ty, cause);
                   if let Some(ref e) = *expr_opt {
                       // If this is a break with a value, we need to type-check