Auto merge of #14509 - Veykril:label-lower-body, r=Veykril

internal: Resolve labels in body lowering

Fixes #14503 (comment)

Author: bors

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,7 +189,7 @@ pub enum InferenceDiagnostic {
         /// Contains the type the field resolves to
         field_with_same_name: Option<Ty>,
     },
-    // FIXME: Make this proper
+    // FIXME: This should be emitted in body lowering
     BreakOutsideOfLoop {
         expr: ExprId,
         is_break: bool,
@@ -468,7 +469,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,21 +484,21 @@ 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>,
+    label: Option<LabelId>,
 ) -> Option<&'c mut BreakableContext> {
     match label {
         Some(_) => find_breakable(ctxs, label).filter(|it| matches!(it.kind, BreakableKind::Loop)),

crates/hir-ty/src/infer.rs

@@ -459,8 +459,8 @@ 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()) {
+            &Expr::Continue { label } => {
+                if let None = find_continuable(&mut self.breakables, label) {
                     self.push_diagnostic(InferenceDiagnostic::BreakOutsideOfLoop {
                         expr: tgt_expr,
                         is_break: false,
@@ -469,9 +469,9 @@ impl<'a> InferenceContext<'a> {
                 };
                 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::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 => {
@@ -490,13 +490,13 @@ 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);
@@ -1900,7 +1900,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

@@ -50,7 +50,9 @@ diagnostics![
     PrivateField,
     ReplaceFilterMapNextWithFindMap,
     TypeMismatch,
+    UndeclaredLabel,
     UnimplementedBuiltinMacro,
+    UnreachableLabel,
     UnresolvedExternCrate,
     UnresolvedField,
     UnresolvedImport,
@@ -61,6 +63,13 @@ diagnostics![
     UnusedMut,
 ];
 
+#[derive(Debug)]
+pub struct BreakOutsideOfLoop {
+    pub expr: InFile<AstPtr<ast::Expr>>,
+    pub is_break: bool,
+    pub bad_value_break: bool,
+}
+
 #[derive(Debug)]
 pub struct UnresolvedModule {
     pub decl: InFile<AstPtr<ast::Module>>,
@@ -84,6 +93,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 +186,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

@@ -88,9 +88,10 @@ pub use crate::{
         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,
+        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 }
@@ -1484,6 +1483,14 @@ impl DefWithBody {
                         .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())
+                }
             }
         }
         for (pat_or_expr, mismatch) in infer.type_mismatches() {