internal: Resolve labels in body lowering

Author: Veykril

crates/hir-def/src/body.rs

@@ -13,7 +13,8 @@ use cfg::{CfgExpr, CfgOptions};
 use drop_bomb::DropBomb;
 use either::Either;
 use hir_expand::{
-    attrs::RawAttrs, hygiene::Hygiene, ExpandError, ExpandResult, HirFileId, InFile, MacroCallId,
+    attrs::RawAttrs, hygiene::Hygiene, name::Name, ExpandError, ExpandResult, HirFileId, InFile,
+    MacroCallId,
 };
 use la_arena::{Arena, ArenaMap};
 use limit::Limit;
@@ -343,6 +344,8 @@ pub enum BodyDiagnostic {
     MacroError { node: InFile<AstPtr<ast::MacroCall>>, message: String },
     UnresolvedProcMacro { node: InFile<AstPtr<ast::MacroCall>>, krate: CrateId },
     UnresolvedMacroCall { node: InFile<AstPtr<ast::MacroCall>>, path: ModPath },
+    UnreachableLabel { node: InFile<AstPtr<ast::Lifetime>>, name: Name },
+    UndeclaredLabel { node: InFile<AstPtr<ast::Lifetime>>, name: Name },
 }
 
 impl Body {

crates/hir-def/src/body/lower.rs

@@ -219,14 +219,14 @@ impl<'a> Printer<'a> {
             }
             Expr::Continue { label } => {
                 w!(self, "continue");
-                if let Some(label) = label {
-                    w!(self, " {}", label);
+                if let Some(lbl) = label {
+                    w!(self, " {}", self.body[*lbl].name);
                 }
             }
             Expr::Break { expr, label } => {
                 w!(self, "break");
-                if let Some(label) = label {
-                    w!(self, " {}", label);
+                if let Some(lbl) = label {
+                    w!(self, " {}", self.body[*lbl].name);
                 }
                 if let Some(expr) = expr {
                     self.whitespace();

crates/hir-def/src/body/pretty.rs

@@ -168,11 +168,11 @@ pub enum Expr {
         arms: Box<[MatchArm]>,
     },
     Continue {
-        label: Option<Name>,
+        label: Option<LabelId>,
     },
     Break {
         expr: Option<ExprId>,
-        label: Option<Name>,
+        label: Option<LabelId>,
     },
     Return {
         expr: Option<ExprId>,

crates/hir-def/src/expr.rs

@@ -120,8 +120,7 @@ impl Name {
         use std::sync::atomic::{AtomicUsize, Ordering};
         static CNT: AtomicUsize = AtomicUsize::new(0);
         let c = CNT.fetch_add(1, Ordering::Relaxed);
-        // FIXME: Currently a `__RA_generated_name` in user code will break our analysis
-        Name::new_text(format!("__RA_geneated_name_{c}").into())
+        Name::new_text(format!("<ra@gennew>{c}").into())
     }
 
     /// Returns the tuple index this name represents if it is a tuple field.

crates/hir-expand/src/name.rs

@@ -18,6 +18,7 @@ use std::{convert::identity, ops::Index};
 
 use chalk_ir::{cast::Cast, DebruijnIndex, Mutability, Safety, Scalar, TypeFlags};
 use either::Either;
+use hir_def::expr::LabelId;
 use hir_def::{
     body::Body,
     builtin_type::{BuiltinInt, BuiltinType, BuiltinUint},
@@ -188,12 +189,6 @@ pub enum InferenceDiagnostic {
         /// Contains the type the field resolves to
         field_with_same_name: Option<Ty>,
     },
-    // FIXME: Make this proper
-    BreakOutsideOfLoop {
-        expr: ExprId,
-        is_break: bool,
-        bad_value_break: bool,
-    },
     MismatchedArgCount {
         call_expr: ExprId,
         expected: usize,
@@ -468,7 +463,7 @@ struct BreakableContext {
     /// The coercion target of the context.
     coerce: Option<CoerceMany>,
     /// The optional label of the context.
-    label: Option<name::Name>,
+    label: Option<LabelId>,
     kind: BreakableKind,
 }
 
@@ -483,28 +478,18 @@ enum BreakableKind {
 
 fn find_breakable<'c>(
     ctxs: &'c mut [BreakableContext],
-    label: Option<&name::Name>,
+    label: Option<LabelId>,
 ) -> Option<&'c mut BreakableContext> {
     let mut ctxs = ctxs
         .iter_mut()
         .rev()
         .take_while(|it| matches!(it.kind, BreakableKind::Block | BreakableKind::Loop));
     match label {
-        Some(_) => ctxs.find(|ctx| ctx.label.as_ref() == label),
+        Some(_) => ctxs.find(|ctx| ctx.label == label),
         None => ctxs.find(|ctx| matches!(ctx.kind, BreakableKind::Loop)),
     }
 }
 
-fn find_continuable<'c>(
-    ctxs: &'c mut [BreakableContext],
-    label: Option<&name::Name>,
-) -> Option<&'c mut BreakableContext> {
-    match label {
-        Some(_) => find_breakable(ctxs, label).filter(|it| matches!(it.kind, BreakableKind::Loop)),
-        None => find_breakable(ctxs, label),
-    }
-}
-
 impl<'a> InferenceContext<'a> {
     fn new(
         db: &'a dyn HirDatabase,

crates/hir-ty/src/infer.rs

@@ -25,9 +25,7 @@ use syntax::ast::RangeOp;
 use crate::{
     autoderef::{builtin_deref, deref_by_trait, Autoderef},
     consteval,
-    infer::{
-        coerce::CoerceMany, find_continuable, pat::contains_explicit_ref_binding, BreakableKind,
-    },
+    infer::{coerce::CoerceMany, pat::contains_explicit_ref_binding, BreakableKind},
     lang_items::lang_items_for_bin_op,
     lower::{
         const_or_path_to_chalk, generic_arg_to_chalk, lower_to_chalk_mutability, ParamLoweringMode,
@@ -459,29 +457,13 @@ impl<'a> InferenceContext<'a> {
                 self.resolver.reset_to_guard(g);
                 ty
             }
-            Expr::Continue { label } => {
-                if let None = find_continuable(&mut self.breakables, label.as_ref()) {
-                    self.push_diagnostic(InferenceDiagnostic::BreakOutsideOfLoop {
-                        expr: tgt_expr,
-                        is_break: false,
-                        bad_value_break: false,
-                    });
-                };
-                self.result.standard_types.never.clone()
-            }
-            Expr::Break { expr, label } => {
-                let val_ty = if let Some(expr) = *expr {
-                    let opt_coerce_to = match find_breakable(&mut self.breakables, label.as_ref()) {
+            Expr::Continue { .. } => self.result.standard_types.never.clone(),
+            &Expr::Break { expr, label } => {
+                let val_ty = if let Some(expr) = expr {
+                    let opt_coerce_to = match find_breakable(&mut self.breakables, label) {
                         Some(ctxt) => match &ctxt.coerce {
                             Some(coerce) => coerce.expected_ty(),
-                            None => {
-                                self.push_diagnostic(InferenceDiagnostic::BreakOutsideOfLoop {
-                                    expr: tgt_expr,
-                                    is_break: true,
-                                    bad_value_break: true,
-                                });
-                                self.err_ty()
-                            }
+                            None => self.err_ty(),
                         },
                         None => self.err_ty(),
                     };
@@ -490,26 +472,20 @@ impl<'a> InferenceContext<'a> {
                     TyBuilder::unit()
                 };
 
-                match find_breakable(&mut self.breakables, label.as_ref()) {
+                match find_breakable(&mut self.breakables, label) {
                     Some(ctxt) => match ctxt.coerce.take() {
                         Some(mut coerce) => {
-                            coerce.coerce(self, *expr, &val_ty);
+                            coerce.coerce(self, expr, &val_ty);
 
                             // Avoiding borrowck
-                            let ctxt = find_breakable(&mut self.breakables, label.as_ref())
+                            let ctxt = find_breakable(&mut self.breakables, label)
                                 .expect("breakable stack changed during coercion");
                             ctxt.may_break = true;
                             ctxt.coerce = Some(coerce);
                         }
                         None => ctxt.may_break = true,
                     },
-                    None => {
-                        self.push_diagnostic(InferenceDiagnostic::BreakOutsideOfLoop {
-                            expr: tgt_expr,
-                            is_break: true,
-                            bad_value_break: false,
-                        });
-                    }
+                    None => {}
                 }
                 self.result.standard_types.never.clone()
             }
@@ -1900,7 +1876,6 @@ impl<'a> InferenceContext<'a> {
         cb: impl FnOnce(&mut Self) -> T,
     ) -> (Option<Ty>, T) {
         self.breakables.push({
-            let label = label.map(|label| self.body[label].name.clone());
             BreakableContext { kind, may_break: false, coerce: ty.map(CoerceMany::new), label }
         });
         let res = cb(self);

crates/hir-ty/src/infer/expr.rs

@@ -47,7 +47,7 @@ struct MirLowerCtx<'a> {
     current_loop_blocks: Option<LoopBlocks>,
     // FIXME: we should resolve labels in HIR lowering and always work with label id here, not
     // with raw names.
-    labeled_loop_blocks: FxHashMap<Name, LoopBlocks>,
+    labeled_loop_blocks: FxHashMap<LabelId, LoopBlocks>,
     discr_temp: Option<Place>,
     db: &'a dyn HirDatabase,
     body: &'a Body,
@@ -579,19 +579,19 @@ impl MirLowerCtx<'_> {
                     Ok(None)
                 }
             },
-            Expr::Break { expr, label } => {
+            &Expr::Break { expr, label } => {
                 if let Some(expr) = expr {
                     let loop_data = match label {
-                        Some(l) => self.labeled_loop_blocks.get(l).ok_or(MirLowerError::UnresolvedLabel)?,
+                        Some(l) => self.labeled_loop_blocks.get(&l).ok_or(MirLowerError::UnresolvedLabel)?,
                         None => self.current_loop_blocks.as_ref().ok_or(MirLowerError::BreakWithoutLoop)?,
                     };
-                    let Some(c) = self.lower_expr_to_place(*expr, loop_data.place.clone(), current)? else {
+                    let Some(c) = self.lower_expr_to_place(expr, loop_data.place.clone(), current)? else {
                         return Ok(None);
                     };
                     current = c;
                 }
                 let end = match label {
-                    Some(l) => self.labeled_loop_blocks.get(l).ok_or(MirLowerError::UnresolvedLabel)?.end.expect("We always generate end for labeled loops"),
+                    Some(l) => self.labeled_loop_blocks.get(&l).ok_or(MirLowerError::UnresolvedLabel)?.end.expect("We always generate end for labeled loops"),
                     None => self.current_loop_end()?,
                 };
                 self.set_goto(current, end);
@@ -1119,10 +1119,8 @@ impl MirLowerCtx<'_> {
             // bad as we may emit end (unneccessary unreachable block) for unterminating loop, but
             // it should not affect correctness.
             self.current_loop_end()?;
-            self.labeled_loop_blocks.insert(
-                self.body.labels[label].name.clone(),
-                self.current_loop_blocks.as_ref().unwrap().clone(),
-            )
+            self.labeled_loop_blocks
+                .insert(label, self.current_loop_blocks.as_ref().unwrap().clone())
         } else {
             None
         };
@@ -1131,7 +1129,7 @@ impl MirLowerCtx<'_> {
         let my = mem::replace(&mut self.current_loop_blocks, prev)
             .ok_or(MirLowerError::ImplementationError("current_loop_blocks is corrupt"))?;
         if let Some(prev) = prev_label {
-            self.labeled_loop_blocks.insert(self.body.labels[label.unwrap()].name.clone(), prev);
+            self.labeled_loop_blocks.insert(label.unwrap(), prev);
         }
         Ok(my.end)
     }

crates/hir-ty/src/mir/lower.rs

@@ -32,7 +32,6 @@ macro_rules! diagnostics {
 }
 
 diagnostics![
-    BreakOutsideOfLoop,
     ExpectedFunction,
     InactiveCode,
     IncorrectCase,
@@ -50,7 +49,9 @@ diagnostics![
     PrivateField,
     ReplaceFilterMapNextWithFindMap,
     TypeMismatch,
+    UndeclaredLabel,
     UnimplementedBuiltinMacro,
+    UnreachableLabel,
     UnresolvedExternCrate,
     UnresolvedField,
     UnresolvedImport,
@@ -84,6 +85,17 @@ pub struct UnresolvedMacroCall {
     pub path: ModPath,
     pub is_bang: bool,
 }
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub struct UnreachableLabel {
+    pub node: InFile<AstPtr<ast::Lifetime>>,
+    pub name: Name,
+}
+
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub struct UndeclaredLabel {
+    pub node: InFile<AstPtr<ast::Lifetime>>,
+    pub name: Name,
+}
 
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct InactiveCode {
@@ -166,13 +178,6 @@ pub struct PrivateField {
     pub field: Field,
 }
 
-#[derive(Debug)]
-pub struct BreakOutsideOfLoop {
-    pub expr: InFile<AstPtr<ast::Expr>>,
-    pub is_break: bool,
-    pub bad_value_break: bool,
-}
-
 #[derive(Debug)]
 pub struct MissingUnsafe {
     pub expr: InFile<AstPtr<ast::Expr>>,

crates/hir/src/diagnostics.rs

@@ -85,12 +85,13 @@ use crate::db::{DefDatabase, HirDatabase};
 pub use crate::{
     attrs::{HasAttrs, Namespace},
     diagnostics::{
-        AnyDiagnostic, BreakOutsideOfLoop, ExpectedFunction, InactiveCode, IncoherentImpl,
-        IncorrectCase, InvalidDeriveTarget, MacroError, MalformedDerive, MismatchedArgCount,
-        MissingFields, MissingMatchArms, MissingUnsafe, NeedMut, NoSuchField, PrivateAssocItem,
-        PrivateField, ReplaceFilterMapNextWithFindMap, TypeMismatch, UnimplementedBuiltinMacro,
-        UnresolvedExternCrate, UnresolvedField, UnresolvedImport, UnresolvedMacroCall,
-        UnresolvedMethodCall, UnresolvedModule, UnresolvedProcMacro, UnusedMut,
+        AnyDiagnostic, ExpectedFunction, InactiveCode, IncoherentImpl, IncorrectCase,
+        InvalidDeriveTarget, MacroError, MalformedDerive, MismatchedArgCount, MissingFields,
+        MissingMatchArms, MissingUnsafe, NeedMut, NoSuchField, PrivateAssocItem, PrivateField,
+        ReplaceFilterMapNextWithFindMap, TypeMismatch, UndeclaredLabel, UnimplementedBuiltinMacro,
+        UnreachableLabel, UnresolvedExternCrate, UnresolvedField, UnresolvedImport,
+        UnresolvedMacroCall, UnresolvedMethodCall, UnresolvedModule, UnresolvedProcMacro,
+        UnusedMut,
     },
     has_source::HasSource,
     semantics::{PathResolution, Semantics, SemanticsScope, TypeInfo, VisibleTraits},
@@ -1393,6 +1394,12 @@ impl DefWithBody {
                     }
                     .into(),
                 ),
+                BodyDiagnostic::UnreachableLabel { node, name } => {
+                    acc.push(UnreachableLabel { node: node.clone(), name: name.clone() }.into())
+                }
+                BodyDiagnostic::UndeclaredLabel { node, name } => {
+                    acc.push(UndeclaredLabel { node: node.clone(), name: name.clone() }.into())
+                }
             }
         }
 
@@ -1405,14 +1412,6 @@ impl DefWithBody {
                     let field = source_map.field_syntax(expr);
                     acc.push(NoSuchField { field }.into())
                 }
-                &hir_ty::InferenceDiagnostic::BreakOutsideOfLoop {
-                    expr,
-                    is_break,
-                    bad_value_break,
-                } => {
-                    let expr = expr_syntax(expr);
-                    acc.push(BreakOutsideOfLoop { expr, is_break, bad_value_break }.into())
-                }
                 &hir_ty::InferenceDiagnostic::MismatchedArgCount { call_expr, expected, found } => {
                     acc.push(
                         MismatchedArgCount { call_expr: expr_syntax(call_expr), expected, found }