Skip to content

internal: Resolve labels in body lowering #14509

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion crates/hir-def/src/body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
275 changes: 199 additions & 76 deletions crates/hir-def/src/body/lower.rs

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions crates/hir-def/src/body/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions crates/hir-def/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
3 changes: 1 addition & 2 deletions crates/hir-expand/src/name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
11 changes: 6 additions & 5 deletions crates/hir-ty/src/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
}

Expand All @@ -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)),
Expand Down
17 changes: 8 additions & 9 deletions crates/hir-ty/src/infer/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 => {
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
18 changes: 8 additions & 10 deletions crates/hir-ty/src/mir/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
};
Expand All @@ -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)
}
Expand Down
27 changes: 20 additions & 7 deletions crates/hir/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ diagnostics![
PrivateField,
ReplaceFilterMapNextWithFindMap,
TypeMismatch,
UndeclaredLabel,
UnimplementedBuiltinMacro,
UnreachableLabel,
UnresolvedExternCrate,
UnresolvedField,
UnresolvedImport,
Expand All @@ -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>>,
Expand All @@ -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 {
Expand Down Expand Up @@ -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>>,
Expand Down
29 changes: 18 additions & 11 deletions crates/hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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())
}
}
}

Expand All @@ -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 }
Expand Down Expand Up @@ -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() {
Expand Down
Loading