From 926039bda99fe8d89d61ff70dfcbd09886cef7a3 Mon Sep 17 00:00:00 2001 From: John-John Tedro Date: Wed, 14 Jun 2023 13:28:35 +0200 Subject: [PATCH] Validate loop continue and break during HIR lowering (#565) --- crates/rune/src/ast.rs | 2 +- crates/rune/src/ast/expr_break.rs | 36 +- crates/rune/src/compile/compile.rs | 11 +- crates/rune/src/compile/ir.rs | 52 +-- crates/rune/src/compile/ir/compiler.rs | 16 +- crates/rune/src/compile/ir/eval.rs | 57 ++- crates/rune/src/compile/ir/interpreter.rs | 4 +- crates/rune/src/compile/v1/assemble.rs | 48 +-- crates/rune/src/compile/v1/loops.rs | 41 +- crates/rune/src/fmt/printer.rs | 17 +- crates/rune/src/hir/hir.rs | 42 +- crates/rune/src/hir/lowering.rs | 482 +++++++++++++--------- crates/rune/src/hir/scopes.rs | 52 ++- crates/rune/src/indexing.rs | 5 - crates/rune/src/indexing/index.rs | 2 +- crates/rune/src/tests/compiler_general.rs | 8 + crates/rune/src/tests/continue_.rs | 2 +- 17 files changed, 459 insertions(+), 418 deletions(-) diff --git a/crates/rune/src/ast.rs b/crates/rune/src/ast.rs index 6a3aa7a81..2c8df70f1 100644 --- a/crates/rune/src/ast.rs +++ b/crates/rune/src/ast.rs @@ -168,7 +168,7 @@ pub use self::expr_assign::ExprAssign; pub use self::expr_await::ExprAwait; pub use self::expr_binary::{BinOp, ExprBinary}; pub use self::expr_block::ExprBlock; -pub use self::expr_break::{ExprBreak, ExprBreakValue}; +pub use self::expr_break::ExprBreak; pub use self::expr_call::ExprCall; pub use self::expr_closure::{ExprClosure, ExprClosureArgs}; pub use self::expr_continue::ExprContinue; diff --git a/crates/rune/src/ast/expr_break.rs b/crates/rune/src/ast/expr_break.rs index cf2a994ae..86744aec9 100644 --- a/crates/rune/src/ast/expr_break.rs +++ b/crates/rune/src/ast/expr_break.rs @@ -21,38 +21,12 @@ pub struct ExprBreak { pub attributes: Vec, /// The return token. pub break_token: T![break], - /// An optional expression to break with. + /// A label to break to. #[rune(iter)] - pub expr: Option>, + pub label: Option, + /// An expression to break with. + #[rune(iter)] + pub expr: Option>, } expr_parse!(Break, ExprBreak, "break expression"); - -/// Things that we can break on. -#[derive(Debug, Clone, PartialEq, Eq, ToTokens, Spanned)] -#[non_exhaustive] -#[allow(clippy::large_enum_variant)] -pub enum ExprBreakValue { - /// Breaking a value out of a loop. - Expr(ast::Expr), - /// Break and jump to the given label. - Label(ast::Label), -} - -impl Parse for ExprBreakValue { - fn parse(p: &mut Parser<'_>) -> Result { - Ok(match p.nth(0)? { - K!['label] => Self::Label(p.parse()?), - _ => Self::Expr(p.parse()?), - }) - } -} - -impl Peek for ExprBreakValue { - fn peek(p: &mut Peeker<'_>) -> bool { - match p.nth(0) { - K!['label] => true, - _ => ast::Expr::peek(p), - } - } -} diff --git a/crates/rune/src/compile/compile.rs b/crates/rune/src/compile/compile.rs index 2ee1d2705..9546ec416 100644 --- a/crates/rune/src/compile/compile.rs +++ b/crates/rune/src/compile/compile.rs @@ -264,14 +264,15 @@ impl<'arena> CompileBuildEntry<'_, 'arena> { closure.ast.args.as_slice().iter().map(|(a, _)| a), )?; + let captures = self.q.pool.item_type_hash(item_meta.item); + let arena = hir::Arena::new(); let mut cx = hir::lowering::Ctxt::with_query( &arena, self.q.borrow(), item_meta.location.source_id, ); - let hir = - hir::lowering::expr_closure_secondary(&mut cx, &closure.ast, closure.captures)?; + let hir = hir::lowering::expr_closure_secondary(&mut cx, &closure.ast, captures)?; let mut c = self.compiler1(location, &closure.ast, &mut asm); assemble::expr_closure_secondary(&mut c, &hir, &closure.ast)?; @@ -295,7 +296,7 @@ impl<'arena> CompileBuildEntry<'_, 'arena> { use self::v1::assemble; - let span = &b.ast; + let captures = self.q.pool.item_type_hash(item_meta.item); let arena = hir::Arena::new(); let mut cx = hir::lowering::Ctxt::with_query( @@ -303,8 +304,8 @@ impl<'arena> CompileBuildEntry<'_, 'arena> { self.q.borrow(), item_meta.location.source_id, ); - let hir = hir::lowering::async_block_secondary(&mut cx, &b.ast, b.captures)?; - let mut c = self.compiler1(location, span, &mut asm); + let hir = hir::lowering::async_block_secondary(&mut cx, &b.ast, captures)?; + let mut c = self.compiler1(location, &b.ast, &mut asm); assemble::async_block_secondary(&mut c, &hir)?; if used.is_unused() { diff --git a/crates/rune/src/compile/ir.rs b/crates/rune/src/compile/ir.rs index 100b6b299..ab4448eb0 100644 --- a/crates/rune/src/compile/ir.rs +++ b/crates/rune/src/compile/ir.rs @@ -16,7 +16,6 @@ use crate::no_std::prelude::*; use crate as rune; use crate::ast::{self, Span, Spanned}; use crate::compile::ir; -use crate::compile::ir::eval::IrEvalBreak; use crate::compile::{self, WithSpan}; use crate::hir; use crate::indexing::index; @@ -389,27 +388,26 @@ pub(crate) struct IrBreak { /// The span of the break. #[rune(span)] pub(crate) span: Span, - /// The kind of the break. - pub(crate) kind: BreakKind, + /// The label of the break. + pub(crate) label: Option>, + /// The value of the break. + pub(crate) expr: Option>, } impl IrBreak { fn compile_ast( span: Span, cx: &mut Ctxt<'_, '_>, - hir: Option<&hir::ExprBreakValue>, + hir: &hir::ExprBreak, ) -> compile::Result { - let kind = match hir { - Some(expr) => match *expr { - hir::ExprBreakValue::Expr(e) => ir::BreakKind::Ir(Box::new(compiler::expr(e, cx)?)), - hir::ExprBreakValue::Label(label) => { - ir::BreakKind::Label(cx.resolve(label)?.into()) - } - }, - None => ir::BreakKind::Inherent, + let label = hir.label.map(Into::into); + + let expr = match hir.expr { + Some(e) => Some(Box::new(compiler::expr(e, cx)?)), + None => None, }; - Ok(ir::IrBreak { span, kind }) + Ok(ir::IrBreak { span, label, expr }) } /// Evaluate the break into an [ir::EvalOutcome]. @@ -420,28 +418,16 @@ impl IrBreak { return e.into(); } - match &self.kind { - BreakKind::Ir(ir) => match ir::eval_ir(ir, interp, used) { - Ok(value) => ir::EvalOutcome::Break(span, IrEvalBreak::Value(value)), - Err(err) => err, + let expr = match &self.expr { + Some(ir) => match ir::eval_ir(ir, interp, used) { + Ok(value) => Some(value), + Err(err) => return err, }, - BreakKind::Label(label) => { - ir::EvalOutcome::Break(span, IrEvalBreak::Label(label.clone())) - } - BreakKind::Inherent => ir::EvalOutcome::Break(span, IrEvalBreak::Inherent), - } - } -} + None => None, + }; -/// The kind of a break expression. -#[derive(Debug, Clone)] -pub(crate) enum BreakKind { - /// Break to the next loop. - Inherent, - /// Break to the given label. - Label(Box), - /// Break with the value acquired from evaluating the ir. - Ir(Box), + ir::EvalOutcome::Break(span, self.label.clone(), expr) + } } /// Tuple expression. diff --git a/crates/rune/src/compile/ir/compiler.rs b/crates/rune/src/compile/ir/compiler.rs index f45517564..905126d43 100644 --- a/crates/rune/src/compile/ir/compiler.rs +++ b/crates/rune/src/compile/ir/compiler.rs @@ -6,7 +6,6 @@ use crate::ast::{self, Span, Spanned}; use crate::compile::ir; use crate::compile::{self, ErrorKind}; use crate::hir; -use crate::parse::Resolve; use crate::query::Query; use crate::runtime::{Bytes, Shared}; use crate::SourceId; @@ -21,16 +20,6 @@ pub(crate) struct Ctxt<'a, 'arena> { pub(crate) q: Query<'a, 'arena>, } -impl Ctxt<'_, '_> { - /// Resolve the given resolvable value. - pub(crate) fn resolve<'s, T>(&'s self, value: &T) -> compile::Result - where - T: Resolve<'s>, - { - value.resolve(resolve_context!(self.q)) - } -} - #[instrument] pub(crate) fn expr(hir: &hir::Expr<'_>, c: &mut Ctxt<'_, '_>) -> compile::Result { let span = hir.span(); @@ -427,10 +416,7 @@ fn expr_loop( ) -> compile::Result { Ok(ir::IrLoop { span, - label: match hir.label { - Some(label) => Some(c.resolve(label)?.into()), - None => None, - }, + label: hir.label.map(|l| l.into()), condition: match hir.condition { Some(hir) => Some(Box::new(condition(hir, c)?)), None => None, diff --git a/crates/rune/src/compile/ir/eval.rs b/crates/rune/src/compile/ir/eval.rs index 1da982a51..ce7525975 100644 --- a/crates/rune/src/compile/ir/eval.rs +++ b/crates/rune/src/compile/ir/eval.rs @@ -24,7 +24,7 @@ pub enum EvalOutcome { /// A compile error. Error(compile::Error), /// Break until the next loop, or the optional label. - Break(Span, IrEvalBreak), + Break(Span, Option>, Option), } impl EvalOutcome { @@ -46,16 +46,6 @@ where } } -/// The value of a break. -pub enum IrEvalBreak { - /// Break the next nested loop. - Inherent, - /// The break had a value. - Value(ir::Value), - /// The break had a label. - Label(Box), -} - fn eval_ir_assign( ir: &ir::IrAssign, interp: &mut ir::Interpreter<'_, '_>, @@ -241,47 +231,46 @@ fn eval_ir_loop( let guard = interp.scopes.push(); - loop { + let value = loop { if let Some(condition) = &ir.condition { interp.scopes.clear_current().with_span(condition)?; let value = eval_ir_condition(condition, interp, used)?; if !as_bool(condition.span(), value)? { - break; + break None; } } match eval_ir_scope(&ir.body, interp, used) { Ok(..) => (), Err(outcome) => match outcome { - EvalOutcome::Break(span, b) => match b { - IrEvalBreak::Inherent => break, - IrEvalBreak::Label(l) => { - if ir.label.as_ref() == Some(&l) { - break; - } - - return Err(EvalOutcome::Break(span, IrEvalBreak::Label(l))); - } - IrEvalBreak::Value(value) => { - if ir.condition.is_none() { - return Ok(value); - } - - return Err(EvalOutcome::from(compile::Error::msg( - span, - "break with value is not supported for unconditional loops", - ))); + EvalOutcome::Break(span, label, expr) => { + if label.as_deref() == ir.label.as_deref() { + break expr; + } else { + return Err(EvalOutcome::Break(span, label, expr)); } - }, + } outcome => return Err(outcome), }, }; - } + }; interp.scopes.pop(guard).with_span(ir)?; - Ok(ir::Value::Unit) + + if let Some(value) = value { + if ir.condition.is_some() { + return Err(EvalOutcome::from(compile::Error::msg( + span, + "break with value is not supported for unconditional loops", + ))); + } + + Ok(value) + } else { + Ok(ir::Value::Unit) + } } fn eval_ir_object( diff --git a/crates/rune/src/compile/ir/interpreter.rs b/crates/rune/src/compile/ir/interpreter.rs index ad627dd62..92cbf2f35 100644 --- a/crates/rune/src/compile/ir/interpreter.rs +++ b/crates/rune/src/compile/ir/interpreter.rs @@ -47,7 +47,7 @@ impl Interpreter<'_, '_> { ir::EvalOutcome::NotConst(span) => { return Err(compile::Error::new(span, IrErrorKind::NotConst)) } - ir::EvalOutcome::Break(span, _) => { + ir::EvalOutcome::Break(span, _, _) => { return Err(compile::Error::new(span, IrErrorKind::BreakOutsideOfLoop)) } }, @@ -76,7 +76,7 @@ impl Interpreter<'_, '_> { ir::EvalOutcome::NotConst(span) => { Err(compile::Error::new(span, IrErrorKind::NotConst)) } - ir::EvalOutcome::Break(span, _) => { + ir::EvalOutcome::Break(span, _, _) => { Err(compile::Error::new(span, IrErrorKind::BreakOutsideOfLoop)) } }, diff --git a/crates/rune/src/compile/v1/assemble.rs b/crates/rune/src/compile/v1/assemble.rs index faf34cfac..56c58eef2 100644 --- a/crates/rune/src/compile/v1/assemble.rs +++ b/crates/rune/src/compile/v1/assemble.rs @@ -46,7 +46,7 @@ pub(crate) struct Ctxt<'a, 'hir, 'arena> { /// Context for which to emit warnings. pub(crate) contexts: Vec, /// The nesting of loop we are currently in. - pub(crate) loops: Loops, + pub(crate) loops: Loops<'hir>, /// Enabled optimizations. pub(crate) options: &'a Options, } @@ -423,7 +423,6 @@ fn pat<'hir>( pat_object(cx, hir, span, false_label, &load)?; Ok(true) } - _ => Err(compile::Error::new(hir, ErrorKind::UnsupportedPatternExpr)), } } @@ -1339,7 +1338,7 @@ fn const_item<'hir>( #[instrument(span = span)] fn expr_break<'hir>( cx: &mut Ctxt<'_, 'hir, '_>, - hir: Option<&hir::ExprBreakValue<'hir>>, + hir: &hir::ExprBreak<'hir>, span: &dyn Spanned, _: Needs, ) -> compile::Result> { @@ -1350,22 +1349,25 @@ fn expr_break<'hir>( )); }; - let (last_loop, to_drop, has_value) = if let Some(e) = hir { - match e { - hir::ExprBreakValue::Expr(e) => { - expr(cx, e, current_loop.needs)?.apply(cx)?; - let to_drop = current_loop.drop.into_iter().collect(); - (current_loop, to_drop, true) - } - hir::ExprBreakValue::Label(label) => { - let (last_loop, to_drop) = - cx.loops.walk_until_label(resolve_context!(cx.q), label)?; - (last_loop, to_drop, false) - } + let (last_loop, to_drop, has_value) = match (hir.label, hir.expr) { + (None, Some(e)) => { + expr(cx, e, current_loop.needs)?.apply(cx)?; + let to_drop = current_loop.drop.into_iter().collect(); + (current_loop, to_drop, true) + } + (Some(label), None) => { + let (last_loop, to_drop) = cx.loops.walk_until_label(label, span)?; + (last_loop, to_drop, false) + } + (Some(label), Some(e)) => { + expr(cx, e, current_loop.needs)?.apply(cx)?; + let (last_loop, to_drop) = cx.loops.walk_until_label(label, span)?; + (last_loop, to_drop, true) + } + (None, None) => { + let to_drop = current_loop.drop.into_iter().collect(); + (current_loop, to_drop, false) } - } else { - let to_drop = current_loop.drop.into_iter().collect(); - (current_loop, to_drop, false) }; // Drop loop temporaries. Typically an iterator. @@ -1514,7 +1516,7 @@ fn expr_call_closure<'hir>( #[instrument(span = span)] fn expr_continue<'hir>( cx: &mut Ctxt<'_, 'hir, '_>, - hir: Option<&ast::Label>, + hir: &hir::ExprContinue<'hir>, span: &dyn Spanned, _: Needs, ) -> compile::Result> { @@ -1525,8 +1527,8 @@ fn expr_continue<'hir>( )); }; - let last_loop = if let Some(label) = hir { - let (last_loop, _) = cx.loops.walk_until_label(resolve_context!(cx.q), label)?; + let last_loop = if let Some(label) = hir.label { + let (last_loop, _) = cx.loops.walk_until_label(label, span)?; last_loop } else { current_loop @@ -1676,7 +1678,7 @@ fn expr_for<'hir>( cx.asm.label(&continue_label)?; let _guard = cx.loops.push(Loop { - label: hir.label.copied(), + label: hir.label, continue_label: continue_label.clone(), continue_var_count, break_label: break_label.clone(), @@ -2399,7 +2401,7 @@ fn expr_loop<'hir>( let var_count = cx.scopes.total(span)?; let _guard = cx.loops.push(Loop { - label: hir.label.copied(), + label: hir.label, continue_label: continue_label.clone(), continue_var_count: var_count, break_label: break_label.clone(), diff --git a/crates/rune/src/compile/v1/loops.rs b/crates/rune/src/compile/v1/loops.rs index 33330c3f7..b715857d6 100644 --- a/crates/rune/src/compile/v1/loops.rs +++ b/crates/rune/src/compile/v1/loops.rs @@ -3,18 +3,16 @@ use core::cell::RefCell; use crate::no_std::prelude::*; use crate::no_std::rc::Rc; -use crate::ast; use crate::ast::Spanned; use crate::compile::v1::Needs; use crate::compile::{self, ErrorKind}; -use crate::parse::ResolveContext; use crate::runtime::Label; -pub(crate) struct LoopGuard { - loops: Rc>>, +pub(crate) struct LoopGuard<'hir> { + loops: Rc>>>, } -impl Drop for LoopGuard { +impl<'hir> Drop for LoopGuard<'hir> { fn drop(&mut self) { let empty = self.loops.borrow_mut().pop().is_some(); debug_assert!(empty); @@ -23,9 +21,9 @@ impl Drop for LoopGuard { /// Loops we are inside. #[derive(Clone)] -pub(crate) struct Loop { +pub(crate) struct Loop<'hir> { /// The optional label of the start of the loop. - pub(crate) label: Option, + pub(crate) label: Option<&'hir str>, /// The start label of the loop, used for `continue`. pub(crate) continue_label: Label, /// The number of local variables inside the loop. @@ -40,11 +38,11 @@ pub(crate) struct Loop { pub(crate) drop: Option, } -pub(crate) struct Loops { - loops: Rc>>, +pub(crate) struct Loops<'hir> { + loops: Rc>>>, } -impl Loops { +impl<'hir> Loops<'hir> { /// Construct a new collection of loops. pub(crate) fn new() -> Self { Self { @@ -53,12 +51,12 @@ impl Loops { } /// Get the last loop context. - pub(crate) fn last(&self) -> Option { + pub(crate) fn last(&self) -> Option> { self.loops.borrow().last().cloned() } /// Push loop information. - pub(crate) fn push(&mut self, l: Loop) -> LoopGuard { + pub(crate) fn push(&mut self, l: Loop<'hir>) -> LoopGuard<'hir> { self.loops.borrow_mut().push(l); LoopGuard { @@ -69,27 +67,18 @@ impl Loops { /// Find the loop with the matching label. pub(crate) fn walk_until_label( &self, - cx: ResolveContext<'_>, - expected: &ast::Label, - ) -> compile::Result<(Loop, Vec)> { - use crate::parse::Resolve; - - let span = expected.span(); - let expected = expected.resolve(cx)?; + expected: &str, + span: &dyn Spanned, + ) -> compile::Result<(Loop<'hir>, Vec)> { let mut to_drop = Vec::new(); for l in self.loops.borrow().iter().rev() { to_drop.extend(l.drop); - let label = match l.label { - Some(label) => label, - None => { - continue; - } + let Some(label) = l.label else { + continue; }; - let label = label.resolve(cx)?; - if expected == label { return Ok((l.clone(), to_drop)); } diff --git a/crates/rune/src/fmt/printer.rs b/crates/rune/src/fmt/printer.rs index 76030eb75..806de5185 100644 --- a/crates/rune/src/fmt/printer.rs +++ b/crates/rune/src/fmt/printer.rs @@ -1187,6 +1187,7 @@ impl<'a> Printer<'a> { let ast::ExprBreak { attributes, break_token, + label, expr, } = ast; @@ -1197,20 +1198,14 @@ impl<'a> Printer<'a> { self.writer .write_spanned_raw(break_token.span, false, false)?; - if let Some(expr) = expr { + if let Some(label) = label { self.writer.write_unspanned(" ")?; - self.visit_expr_break_value(expr)?; + self.writer.write_spanned_raw(label.span, false, false)?; } - Ok(()) - } - - fn visit_expr_break_value(&mut self, ast: &ast::ExprBreakValue) -> Result<()> { - match ast { - ast::ExprBreakValue::Expr(expr) => self.visit_expr(expr)?, - ast::ExprBreakValue::Label(label) => { - self.writer.write_spanned_raw(label.span, false, false)? - } + if let Some(expr) = expr { + self.writer.write_unspanned(" ")?; + self.visit_expr(expr)?; } Ok(()) diff --git a/crates/rune/src/hir/hir.rs b/crates/rune/src/hir/hir.rs index 923dcf104..1bb40f990 100644 --- a/crates/rune/src/hir/hir.rs +++ b/crates/rune/src/hir/hir.rs @@ -102,8 +102,6 @@ pub(crate) enum PatPathKind<'hir> { pub(crate) enum PatKind<'hir> { /// An ignored binding. Ignore, - /// The rest pattern `..`. - Rest, /// A path pattern. Path(&'hir PatPathKind<'hir>), /// A literal pattern. This is represented as an expression. @@ -114,8 +112,6 @@ pub(crate) enum PatKind<'hir> { Tuple(&'hir PatItems<'hir>), /// An object pattern. Object(&'hir PatItems<'hir>), - /// A binding `a: pattern` or `"foo": pattern`. - Binding, } #[derive(Debug, Clone, Copy)] @@ -223,8 +219,8 @@ pub(crate) enum ExprKind<'hir> { Index(&'hir ExprIndex<'hir>), AsyncBlock(&'hir ExprAsyncBlock<'hir>), Block(&'hir Block<'hir>), - Break(Option<&'hir ExprBreakValue<'hir>>), - Continue(Option<&'hir ast::Label>), + Break(&'hir ExprBreak<'hir>), + Continue(&'hir ExprContinue<'hir>), Yield(Option<&'hir Expr<'hir>>), Return(Option<&'hir Expr<'hir>>), Await(&'hir Expr<'hir>), @@ -290,7 +286,7 @@ pub(crate) struct ExprAssign<'hir> { #[non_exhaustive] pub(crate) struct ExprLoop<'hir> { /// A label. - pub(crate) label: Option<&'hir ast::Label>, + pub(crate) label: Option<&'hir str>, /// A condition to execute the loop, if a condition is necessary. pub(crate) condition: Option<&'hir Condition<'hir>>, /// The body of the loop. @@ -305,7 +301,7 @@ pub(crate) struct ExprLoop<'hir> { #[non_exhaustive] pub(crate) struct ExprFor<'hir> { /// The label of the loop. - pub(crate) label: Option<&'hir ast::Label>, + pub(crate) label: Option<&'hir str>, /// The pattern binding to use. /// Non-trivial pattern bindings will panic if the value doesn't match. pub(crate) binding: Pat<'hir>, @@ -493,16 +489,6 @@ pub(crate) struct ExprIndex<'hir> { pub(crate) index: Expr<'hir>, } -/// Things that we can break on. -#[derive(Debug, Clone, Copy)] -#[non_exhaustive] -pub(crate) enum ExprBreakValue<'hir> { - /// Breaking a value out of a loop. - Expr(&'hir Expr<'hir>), - /// Break and jump to the given label. - Label(&'hir ast::Label), -} - /// An async block being called. #[derive(Debug, Clone, Copy)] #[non_exhaustive] @@ -512,6 +498,26 @@ pub(crate) struct ExprAsyncBlock<'hir> { pub(crate) captures: &'hir [Name<'hir>], } +#[derive(Debug, Clone, Copy)] +pub(crate) struct ExprBreak<'hir> { + /// Label being continued. + pub(crate) label: Option<&'hir str>, + /// Value being broken with. + pub(crate) expr: Option<&'hir Expr<'hir>>, + /// Variables that goes out of scope. + #[allow(unused)] + pub(crate) drop: &'hir [Name<'hir>], +} + +#[derive(Debug, Clone, Copy)] +pub(crate) struct ExprContinue<'hir> { + /// Label being continued. + pub(crate) label: Option<&'hir str>, + /// Variables that goes out of scope. + #[allow(unused)] + pub(crate) drop: &'hir [Name<'hir>], +} + /// A `select` expression that selects over a collection of futures. #[derive(Debug, Clone, Copy)] #[non_exhaustive] diff --git a/crates/rune/src/hir/lowering.rs b/crates/rune/src/hir/lowering.rs index d737b4425..b9dfd5dcf 100644 --- a/crates/rune/src/hir/lowering.rs +++ b/crates/rune/src/hir/lowering.rs @@ -244,7 +244,6 @@ fn expr_call_closure<'hir>( build: Build::Closure(indexing::Closure { ast: Box::new(ast.clone()), call, - captures: meta.hash, }), used: Used::Used, }); @@ -435,30 +434,51 @@ pub(crate) fn expr<'hir>( // representation. We only do different ones here right now since it's // easier when refactoring. ast::Expr::While(ast) => { - cx.scopes.push(); + let label = match &ast.label { + Some((label, _)) => Some(alloc_str!(label.resolve(resolve_context!(cx.q))?)), + None => None, + }; + cx.scopes.push_loop(label); let condition = condition(cx, &ast.condition)?; let body = block(cx, &ast.body)?; - let layer = cx.scopes.pop().with_span(ast)?; hir::ExprKind::Loop(alloc!(hir::ExprLoop { - label: option!(&ast.label, |(ast, _)| label(cx, ast)?), + label, condition: Some(alloc!(condition)), body, drop: iter!(layer.into_drop_order()), })) } - ast::Expr::Loop(ast) => hir::ExprKind::Loop(alloc!(hir::ExprLoop { - label: option!(&ast.label, |(ast, _)| label(cx, ast)?), - condition: None, - body: block(cx, &ast.body)?, - drop: &[], - })), + ast::Expr::Loop(ast) => { + let label = match &ast.label { + Some((label, _)) => Some(alloc_str!(label.resolve(resolve_context!(cx.q))?)), + None => None, + }; + + cx.scopes.push_loop(label); + let body = block(cx, &ast.body)?; + let layer = cx.scopes.pop().with_span(ast)?; + + let kind = hir::ExprKind::Loop(alloc!(hir::ExprLoop { + label, + condition: None, + body, + drop: iter!(layer.into_drop_order()), + })); + + kind + } ast::Expr::For(ast) => { let iter = expr(cx, &ast.iter)?; - cx.scopes.push(); + let label = match &ast.label { + Some((label, _)) => Some(alloc_str!(label.resolve(resolve_context!(cx.q))?)), + None => None, + }; + + cx.scopes.push_loop(label); let binding = pat(cx, &ast.binding)?; let body = block(cx, &ast.body)?; @@ -466,7 +486,7 @@ pub(crate) fn expr<'hir>( let layer = cx.scopes.pop().with_span(ast)?; hir::ExprKind::For(alloc!(hir::ExprFor { - label: option!(&ast.label, |(ast, _)| label(cx, ast)?), + label, binding, iter, body, @@ -531,16 +551,8 @@ pub(crate) fn expr<'hir>( index: expr(cx, &ast.index)?, })), ast::Expr::Block(ast) => expr_block(cx, ast)?, - ast::Expr::Break(ast) => { - hir::ExprKind::Break(option!(ast.expr.as_deref(), |ast| match ast { - ast::ExprBreakValue::Expr(ast) => hir::ExprBreakValue::Expr(alloc!(expr(cx, ast)?)), - ast::ExprBreakValue::Label(ast) => - hir::ExprBreakValue::Label(alloc!(label(cx, ast)?)), - })) - } - ast::Expr::Continue(ast) => { - hir::ExprKind::Continue(option!(&ast.label, |ast| label(cx, ast)?)) - } + ast::Expr::Break(ast) => hir::ExprKind::Break(alloc!(expr_break(cx, ast)?)), + ast::Expr::Continue(ast) => hir::ExprKind::Continue(alloc!(expr_continue(cx, ast)?)), ast::Expr::Yield(ast) => hir::ExprKind::Yield(option!(&ast.expr, |ast| expr(cx, ast)?)), ast::Expr::Return(ast) => hir::ExprKind::Return(option!(&ast.expr, |ast| expr(cx, ast)?)), ast::Expr::Await(ast) => hir::ExprKind::Await(alloc!(expr(cx, &ast.expr)?)), @@ -817,7 +829,6 @@ pub(crate) fn expr_block<'hir>( build: Build::AsyncBlock(indexing::AsyncBlock { ast: ast.block.clone(), call, - captures: meta.hash, }), used: Used::Used, }); @@ -848,6 +859,70 @@ pub(crate) fn expr_block<'hir>( } } +/// Unroll a break expression, capturing all variables which are in scope at +/// the time of it. +fn expr_break<'hir>( + cx: &mut Ctxt<'hir, '_, '_>, + ast: &ast::ExprBreak, +) -> compile::Result> { + alloc_with!(cx, ast); + + let label = match &ast.label { + Some(label) => Some(label.resolve(resolve_context!(cx.q))?), + None => None, + }; + + let Some(drop) = cx.scopes.loop_drop(label) else { + if let Some(label) = label { + return Err(compile::Error::new(ast, ErrorKind::MissingLoopLabel { label: label.into() })); + } else { + return Err(compile::Error::new(ast, ErrorKind::BreakOutsideOfLoop)); + } + }; + + Ok(hir::ExprBreak { + label: match label { + Some(label) => Some(alloc_str!(label)), + None => None, + }, + expr: match &ast.expr { + Some(ast) => Some(alloc!(expr(cx, ast)?)), + None => None, + }, + drop: iter!(drop), + }) +} + +/// Unroll a continue expression, capturing all variables which are in scope at +/// the time of it. +fn expr_continue<'hir>( + cx: &mut Ctxt<'hir, '_, '_>, + ast: &ast::ExprContinue, +) -> compile::Result> { + alloc_with!(cx, ast); + + let label = match &ast.label { + Some(label) => Some(label.resolve(resolve_context!(cx.q))?), + None => None, + }; + + let Some(drop) = cx.scopes.loop_drop(label) else { + if let Some(label) = label { + return Err(compile::Error::new(ast, ErrorKind::MissingLoopLabel { label: label.into() })); + } else { + return Err(compile::Error::new(ast, ErrorKind::ContinueOutsideOfLoop)); + } + }; + + Ok(hir::ExprContinue { + label: match label { + Some(label) => Some(alloc_str!(label)), + None => None, + }, + drop: iter!(drop), + }) +} + /// Lower a function argument. fn fn_arg<'hir>( cx: &mut Ctxt<'hir, '_, '_>, @@ -891,203 +966,221 @@ fn stmt<'hir>(cx: &mut Ctxt<'hir, '_, '_>, ast: &ast::Stmt) -> compile::Result(cx: &mut Ctxt<'hir, '_, '_>, ast: &ast::Pat) -> compile::Result> { - alloc_with!(cx, ast); - - Ok(hir::Pat { - span: ast.span(), - kind: match ast { - ast::Pat::Ignore(..) => hir::PatKind::Ignore, - ast::Pat::Rest(..) => hir::PatKind::Rest, - ast::Pat::Path(ast) => { - let named = cx.q.convert_path(&ast.path)?; - let parameters = generics_parameters(cx, &named)?; + fn filter((ast, _): &(ast::Pat, Option)) -> Option<&ast::Pat> { + if matches!(ast, ast::Pat::Binding(..) | ast::Pat::Rest(..)) { + return None; + } - let kind = 'ok: { - if let Some(meta) = cx.try_lookup_meta(&ast, named.item, ¶meters)? { - if let Some((0, kind)) = tuple_match_for(cx, &meta) { - break 'ok hir::PatPathKind::Kind(alloc!(kind)); - } - } + Some(ast) + } - if let Some(ident) = ast.path.try_as_ident() { - let name = alloc_str!(ident.resolve(resolve_context!(cx.q))?); - cx.scopes.define(hir::Name::Str(name)).with_span(ast)?; - break 'ok hir::PatPathKind::Ident(name); - } + alloc_with!(cx, ast); - return Err(compile::Error::new(ast, ErrorKind::UnsupportedBinding)); - }; + let kind = match ast { + ast::Pat::Ignore(..) => hir::PatKind::Ignore, + ast::Pat::Path(ast) => { + let named = cx.q.convert_path(&ast.path)?; + let parameters = generics_parameters(cx, &named)?; - hir::PatKind::Path(alloc!(kind)) - } - ast::Pat::Lit(ast) => hir::PatKind::Lit(alloc!(expr(cx, &ast.expr)?)), - ast::Pat::Vec(ast) => { - let items = iter!(&ast.items, |(ast, _)| pat(cx, ast)?); - let (is_open, count) = pat_items_count(items)?; - - hir::PatKind::Vec(alloc!(hir::PatItems { - kind: hir::PatItemsKind::Anonymous { count, is_open }, - items, - is_open, - count, - bindings: &[], - })) - } - ast::Pat::Tuple(ast) => { - let items = iter!(&ast.items, |(ast, _)| pat(cx, ast)?); - let (is_open, count) = pat_items_count(items)?; + let kind = 'ok: { + if let Some(meta) = cx.try_lookup_meta(&ast, named.item, ¶meters)? { + if let Some((0, kind)) = tuple_match_for(cx, &meta) { + break 'ok hir::PatPathKind::Kind(alloc!(kind)); + } + } - let kind = if let Some(path) = &ast.path { - let named = cx.q.convert_path(path)?; - let parameters = generics_parameters(cx, &named)?; - let meta = cx.lookup_meta(path, named.item, parameters)?; + if let Some(ident) = ast.path.try_as_ident() { + let name = alloc_str!(ident.resolve(resolve_context!(cx.q))?); + cx.scopes.define(hir::Name::Str(name)).with_span(ast)?; + break 'ok hir::PatPathKind::Ident(name); + } - // Treat the current meta as a tuple and get the number of arguments it - // should receive and the type check that applies to it. - let Some((args, kind)) = tuple_match_for(cx, &meta) else { - return Err(compile::Error::expected_meta( - path, - meta.info(cx.q.pool), - "type that can be used in a tuple pattern", - )); - }; + return Err(compile::Error::new(ast, ErrorKind::UnsupportedBinding)); + }; - if !(args == count || count < args && is_open) { - return Err(compile::Error::new( - path, - ErrorKind::UnsupportedArgumentCount { - expected: args, - actual: count, - }, - )); - } + hir::PatKind::Path(alloc!(kind)) + } + ast::Pat::Lit(ast) => hir::PatKind::Lit(alloc!(expr(cx, &ast.expr)?)), + ast::Pat::Vec(ast) => { + let (is_open, count) = pat_items_count(ast.items.as_slice())?; + let items = iter!( + ast.items.iter().filter_map(filter), + ast.items.len(), + |ast| pat(cx, ast)? + ); + + hir::PatKind::Vec(alloc!(hir::PatItems { + kind: hir::PatItemsKind::Anonymous { count, is_open }, + items, + is_open, + count, + bindings: &[], + })) + } + ast::Pat::Tuple(ast) => { + let (is_open, count) = pat_items_count(ast.items.as_slice())?; + let items = iter!( + ast.items.iter().filter_map(filter), + ast.items.len(), + |ast| pat(cx, ast)? + ); + + let kind = if let Some(path) = &ast.path { + let named = cx.q.convert_path(path)?; + let parameters = generics_parameters(cx, &named)?; + let meta = cx.lookup_meta(path, named.item, parameters)?; - kind - } else { - hir::PatItemsKind::Anonymous { count, is_open } + // Treat the current meta as a tuple and get the number of arguments it + // should receive and the type check that applies to it. + let Some((args, kind)) = tuple_match_for(cx, &meta) else { + return Err(compile::Error::expected_meta( + path, + meta.info(cx.q.pool), + "type that can be used in a tuple pattern", + )); }; - hir::PatKind::Tuple(alloc!(hir::PatItems { - kind, - items, - is_open, - count, - bindings: &[], - })) - } - ast::Pat::Object(ast) => { - let items = iter!(&ast.items, |(ast, _)| pat(cx, ast)?); - let (is_open, count) = pat_items_count(items)?; + if !(args == count || count < args && is_open) { + return Err(compile::Error::new( + path, + ErrorKind::UnsupportedArgumentCount { + expected: args, + actual: count, + }, + )); + } - let mut keys_dup = HashMap::new(); + kind + } else { + hir::PatItemsKind::Anonymous { count, is_open } + }; - let bindings = iter!(ast.items.iter().take(count), |(pat, _)| { - let (key, binding) = match pat { - ast::Pat::Binding(binding) => { - let (span, key) = object_key(cx, &binding.key)?; - ( + hir::PatKind::Tuple(alloc!(hir::PatItems { + kind, + items, + is_open, + count, + bindings: &[], + })) + } + ast::Pat::Object(ast) => { + let items = iter!( + ast.items.iter().filter_map(filter), + ast.items.len(), + |ast| pat(cx, ast)? + ); + let (is_open, count) = pat_items_count(ast.items.as_slice())?; + + let mut keys_dup = HashMap::new(); + + let bindings = iter!(ast.items.iter().take(count), |(pat, _)| { + let (key, binding) = match pat { + ast::Pat::Binding(binding) => { + let (span, key) = object_key(cx, &binding.key)?; + ( + key, + hir::Binding::Binding( + span.span(), key, - hir::Binding::Binding( - span.span(), - key, - alloc!(self::pat(cx, &binding.pat)?), - ), - ) - } - ast::Pat::Path(path) => { - let Some(ident) = path.path.try_as_ident() else { - return Err(compile::Error::new( - path, - ErrorKind::UnsupportedPatternExpr, - )); - }; - - let key = alloc_str!(ident.resolve(resolve_context!(cx.q))?); - cx.scopes.define(hir::Name::Str(key)).with_span(ident)?; - (key, hir::Binding::Ident(path.span(), key)) - } - _ => { + alloc!(self::pat(cx, &binding.pat)?), + ), + ) + } + ast::Pat::Path(path) => { + let Some(ident) = path.path.try_as_ident() else { return Err(compile::Error::new( - pat, + path, ErrorKind::UnsupportedPatternExpr, )); - } - }; + }; - if let Some(existing) = keys_dup.insert(key, pat) { - return Err(compile::Error::new( - pat, - ErrorKind::DuplicateObjectKey { - existing: existing.span(), - object: pat.span(), - }, - )); + let key = alloc_str!(ident.resolve(resolve_context!(cx.q))?); + cx.scopes.define(hir::Name::Str(key)).with_span(ident)?; + (key, hir::Binding::Ident(path.span(), key)) } + _ => { + return Err(compile::Error::new(pat, ErrorKind::UnsupportedPatternExpr)); + } + }; - binding - }); + if let Some(existing) = keys_dup.insert(key, pat) { + return Err(compile::Error::new( + pat, + ErrorKind::DuplicateObjectKey { + existing: existing.span(), + object: pat.span(), + }, + )); + } - let kind = match &ast.ident { - ast::ObjectIdent::Named(path) => { - let named = cx.q.convert_path(path)?; - let parameters = generics_parameters(cx, &named)?; - let meta = cx.lookup_meta(path, named.item, parameters)?; + binding + }); - let Some((st, kind)) = struct_match_for(cx, &meta) else { - return Err(compile::Error::expected_meta( - path, - meta.info(cx.q.pool), - "type that can be used in a struct pattern", - )); - }; + let kind = match &ast.ident { + ast::ObjectIdent::Named(path) => { + let named = cx.q.convert_path(path)?; + let parameters = generics_parameters(cx, &named)?; + let meta = cx.lookup_meta(path, named.item, parameters)?; - let mut fields = st.fields.clone(); - - for binding in bindings.iter() { - if !fields.remove(binding.key()) { - return Err(compile::Error::new( - ast, - ErrorKind::LitObjectNotField { - field: binding.key().into(), - item: cx.q.pool.item(meta.item_meta.item).to_owned(), - }, - )); - } - } + let Some((st, kind)) = struct_match_for(cx, &meta) else { + return Err(compile::Error::expected_meta( + path, + meta.info(cx.q.pool), + "type that can be used in a struct pattern", + )); + }; - if !is_open && !fields.is_empty() { - let mut fields = fields - .into_iter() - .map(Box::::from) - .collect::>(); - fields.sort(); + let mut fields = st.fields.clone(); + for binding in bindings.iter() { + if !fields.remove(binding.key()) { return Err(compile::Error::new( ast, - ErrorKind::PatternMissingFields { + ErrorKind::LitObjectNotField { + field: binding.key().into(), item: cx.q.pool.item(meta.item_meta.item).to_owned(), - fields, }, )); } - - kind } - ast::ObjectIdent::Anonymous(..) => { - hir::PatItemsKind::Anonymous { count, is_open } + + if !is_open && !fields.is_empty() { + let mut fields = fields + .into_iter() + .map(Box::::from) + .collect::>(); + fields.sort(); + + return Err(compile::Error::new( + ast, + ErrorKind::PatternMissingFields { + item: cx.q.pool.item(meta.item_meta.item).to_owned(), + fields, + }, + )); } - }; - hir::PatKind::Object(alloc!(hir::PatItems { - kind, - items, - is_open, - count, - bindings, - })) - } - ast::Pat::Binding(..) => hir::PatKind::Binding, - }, + kind + } + ast::ObjectIdent::Anonymous(..) => hir::PatItemsKind::Anonymous { count, is_open }, + }; + + hir::PatKind::Object(alloc!(hir::PatItems { + kind, + items, + is_open, + count, + bindings, + })) + } + _ => { + return Err(compile::Error::new(ast, ErrorKind::UnsupportedPatternExpr)); + } + }; + + Ok(hir::Pat { + span: ast.span(), + kind, }) } @@ -1248,13 +1341,6 @@ fn expr_path_meta<'hir>( } } -fn label(_: &mut Ctxt<'_, '_, '_>, ast: &ast::Label) -> compile::Result { - Ok(ast::Label { - span: ast.span, - source: ast.source, - }) -} - fn condition<'hir>( cx: &mut Ctxt<'hir, '_, '_>, ast: &ast::Condition, @@ -1271,18 +1357,18 @@ fn condition<'hir>( } /// Test if the given pattern is open or not. -fn pat_items_count(items: &[hir::Pat<'_>]) -> compile::Result<(bool, usize)> { +fn pat_items_count(items: &[(ast::Pat, Option)]) -> compile::Result<(bool, usize)> { let mut it = items.iter(); let (is_open, mut count) = match it.next_back() { - Some(pat) => matches!(pat.kind, hir::PatKind::Rest) + Some((pat, _)) => matches!(pat, ast::Pat::Rest { .. }) .then(|| (true, 0)) .unwrap_or((false, 1)), None => return Ok((false, 0)), }; - for pat in it { - if let hir::PatKind::Rest = pat.kind { + for (pat, _) in it { + if let ast::Pat::Rest { .. } = pat { return Err(compile::Error::new(pat, ErrorKind::UnsupportedPatternRest)); } diff --git a/crates/rune/src/hir/scopes.rs b/crates/rune/src/hir/scopes.rs index bdef5da02..98ee1b771 100644 --- a/crates/rune/src/hir/scopes.rs +++ b/crates/rune/src/hir/scopes.rs @@ -22,6 +22,7 @@ pub(crate) struct Scope(usize); enum LayerKind { #[default] Default, + Loop, Captures, } @@ -39,6 +40,8 @@ pub(crate) struct Layer<'hir> { order: Vec>, /// Captures inside of this layer. captures: BTreeSet>, + /// An optional layer label. + label: Option<&'hir str>, } impl<'hir> Layer<'hir> { @@ -70,23 +73,20 @@ impl<'hir> Scopes<'hir> { /// Push a scope. pub(crate) fn push(&mut self) { - let scope = Scope(self.scopes.vacant_key()); - - let layer = Layer { - scope, - parent: Some(NonZeroUsize::new(self.scope.0.wrapping_add(1)).expect("ran out of ids")), - variables: HashSet::new(), - order: Vec::new(), - kind: LayerKind::Default, - captures: BTreeSet::new(), - }; - - self.scopes.insert(layer); - self.scope = scope; + self.push_kind(LayerKind::Default, None) } /// Push an async block. pub(crate) fn push_captures(&mut self) { + self.push_kind(LayerKind::Captures, None) + } + + /// Push a loop. + pub(crate) fn push_loop(&mut self, label: Option<&'hir str>) { + self.push_kind(LayerKind::Loop, label) + } + + fn push_kind(&mut self, kind: LayerKind, label: Option<&'hir str>) { let scope = Scope(self.scopes.vacant_key()); let layer = Layer { @@ -94,8 +94,9 @@ impl<'hir> Scopes<'hir> { parent: Some(NonZeroUsize::new(self.scope.0.wrapping_add(1)).expect("ran out of ids")), variables: HashSet::new(), order: Vec::new(), - kind: LayerKind::Captures, + kind, captures: BTreeSet::new(), + label, }; self.scopes.insert(layer); @@ -171,6 +172,29 @@ impl<'hir> Scopes<'hir> { Some((name, scope)) } + + /// Walk the loop and construct captures for it. + #[tracing::instrument(skip_all, fields(?self.scope, ?label))] + pub(crate) fn loop_drop(&self, label: Option<&str>) -> Option>> { + let mut captures = Vec::new(); + let mut scope = self.scopes.get(self.scope.0); + + while let Some(layer) = scope.take() { + if let Some(label) = label { + if layer.label == Some(label) { + return Some(captures); + } + } else if matches!(layer.kind, LayerKind::Loop) { + return Some(captures); + } + + captures.extend(layer.order.iter().rev().copied()); + tracing::trace!(parent = ?layer.parent()); + scope = self.scopes.get(layer.parent()?); + } + + None + } } impl<'hir> Default for Scopes<'hir> { diff --git a/crates/rune/src/indexing.rs b/crates/rune/src/indexing.rs index 4b4b86230..52cb34906 100644 --- a/crates/rune/src/indexing.rs +++ b/crates/rune/src/indexing.rs @@ -7,7 +7,6 @@ use crate::no_std::prelude::*; use crate::ast; use crate::compile::meta; use crate::compile::{ItemId, ItemMeta}; -use crate::hash::Hash; use crate::parse::NonZeroId; use crate::runtime::Call; @@ -112,8 +111,6 @@ pub(crate) struct Closure { pub(crate) ast: Box, /// Calling convention used for closure. pub(crate) call: Call, - /// Captures. - pub(crate) captures: Hash, } #[derive(Debug, Clone)] @@ -122,8 +119,6 @@ pub(crate) struct AsyncBlock { pub(crate) ast: ast::Block, /// Calling convention used for async block. pub(crate) call: Call, - /// Captured variables. - pub(crate) captures: Hash, } #[derive(Debug, Clone)] diff --git a/crates/rune/src/indexing/index.rs b/crates/rune/src/indexing/index.rs index 93fc247f3..e7dc97688 100644 --- a/crates/rune/src/indexing/index.rs +++ b/crates/rune/src/indexing/index.rs @@ -1721,7 +1721,7 @@ fn expr_index(idx: &mut Indexer<'_, '_>, ast: &mut ast::ExprIndex) -> compile::R #[instrument(span = ast)] fn expr_break(idx: &mut Indexer<'_, '_>, ast: &mut ast::ExprBreak) -> compile::Result<()> { - if let Some(ast::ExprBreakValue::Expr(e)) = ast.expr.as_deref_mut() { + if let Some(e) = ast.expr.as_deref_mut() { expr(idx, e)?; } diff --git a/crates/rune/src/tests/compiler_general.rs b/crates/rune/src/tests/compiler_general.rs index 114919d51..13b6ca1d7 100644 --- a/crates/rune/src/tests/compiler_general.rs +++ b/crates/rune/src/tests/compiler_general.rs @@ -20,6 +20,14 @@ fn break_outside_of_loop() { }; } +#[test] +fn continue_outside_of_loop() { + assert_errors! { + r#"pub fn main() { continue; }"#, + span!(16, 24), ContinueOutsideOfLoop + }; +} + #[test] fn test_pointers() { assert_errors! { diff --git a/crates/rune/src/tests/continue_.rs b/crates/rune/src/tests/continue_.rs index 62f1d7c86..29b59c2dc 100644 --- a/crates/rune/src/tests/continue_.rs +++ b/crates/rune/src/tests/continue_.rs @@ -118,7 +118,7 @@ fn test_continue_not_in_loop() { fn test_continue_missing_label() { assert_errors! { r#"pub fn main() { 'existing: loop { loop { continue 'missing; } } }"#, - span!(50, 58), MissingLoopLabel { label } => { + span!(41, 58), MissingLoopLabel { label } => { assert_eq!(&*label, "missing"); } };