From d95ececd7b3f2344eae046bc983420081d156fc1 Mon Sep 17 00:00:00 2001 From: chloe caruso Date: Thu, 1 Feb 2024 19:59:47 -0800 Subject: [PATCH] fix various `return` and `pub` related issues --- crates/ast/ast.rs | 2 +- .../js_ast/convert/hoist_block_to_js_block.rs | 20 +++++----- crates/js_ast/convert/hoist_expr.rs | 7 +++- crates/js_ast/convert/process_function.rs | 16 ++++---- crates/js_ast/convert/process_statement.rs | 39 ++++++++++++------- crates/js_ast/js_ast.rs | 2 +- crates/js_printer/lib.rs | 9 ++++- crates/parser/parse_block.rs | 8 ++++ crates/parser/parse_expr_basic.rs | 27 ++++++++----- crates/parser/parse_fn_declaration.rs | 6 +-- crates/parser/parse_statement.rs | 5 +++ crates/parser/parser.rs | 13 ++++--- 12 files changed, 101 insertions(+), 53 deletions(-) diff --git a/crates/ast/ast.rs b/crates/ast/ast.rs index 680c579aa79c8a3a0dd1cc19cc010ea8cd46cf7e..d8c2939268152f08a1e1253f5150076564eaa608 100644 --- a/crates/ast/ast.rs +++ b/crates/ast/ast.rs @@ -162,7 +162,7 @@ pub enum ExprData { arguments: Vec, }, If(If), - Return(Box), + Return(Option>), Literal(Literal), Identifier(String), } diff --git a/crates/js_ast/convert/hoist_block_to_js_block.rs b/crates/js_ast/convert/hoist_block_to_js_block.rs index 2dd10483b209d1d999b593d2763cb4b3b9109c2c..22fcd4a5ea5a1d4d7bf80a2e169f1c8c1d7257d6 100644 --- a/crates/js_ast/convert/hoist_block_to_js_block.rs +++ b/crates/js_ast/convert/hoist_block_to_js_block.rs @@ -52,16 +52,18 @@ pub fn hoist_block_to_js_block( // If the last statement is an expression, we need // to assign to the temporary variable. - match child.take_last_statement_if_expr() { - Some(expr) => { - *did_assign = true; - child.add_statement(new_js_assignment_statement( - expr, - target.get(ctx), - loc.clone(), - )); + if block.has_implicit_return { + match child.take_last_statement_if_expr() { + Some(expr) => { + *did_assign = true; + child.add_statement(new_js_assignment_statement( + expr, + target.get(ctx), + loc.clone(), + )); + } + _ => {} } - _ => {} } child.into() diff --git a/crates/js_ast/convert/hoist_expr.rs b/crates/js_ast/convert/hoist_expr.rs index a79cb5a9928c6a95e0e49e9e9a18ff44ffd77646..3affb5c8e36e4105059bf1ff98989739bf4b5f88 100644 --- a/crates/js_ast/convert/hoist_expr.rs +++ b/crates/js_ast/convert/hoist_expr.rs @@ -94,9 +94,12 @@ pub fn hoist_expr_contents(ctx: &mut BlockConvertContext, expr: Expr) -> Option< ExprData::If(_) => hoist_if_expr(ctx, expr), - ExprData::Return(expr) => { + ExprData::Return(maybe_expr) => { let loc = expr.loc.clone(); - let return_value = hoist_expr_if_needed(ctx, *expr)?; + let return_value = match maybe_expr { + Some(expr) => Some(hoist_expr_if_needed(ctx, *expr)?), + None => None, + }; ctx.add_statement(JsStatement::new( JsStatementData::Return(return_value), loc, diff --git a/crates/js_ast/convert/process_function.rs b/crates/js_ast/convert/process_function.rs index d45090b7889d25c279447ce1dfc3bd7f3297b19e..b451c3ac2881b7b28f3e2d63f5cbdef052b161d7 100644 --- a/crates/js_ast/convert/process_function.rs +++ b/crates/js_ast/convert/process_function.rs @@ -30,18 +30,20 @@ pub fn process_function_to_arrow( }; } - if let Some(expr) = child.take_last_statement_if_expr() { - child.add_statement(JsStatement::new( - JsStatementData::Return(expr), - loc.clone(), - None, - )); + if block.has_implicit_return { + if let Some(expr) = child.take_last_statement_if_expr() { + child.add_statement(JsStatement::new( + JsStatementData::Return(Some(expr)), + loc.clone(), + None, + )); + } } } _ => { if let Some(expr) = hoist_expr_if_needed(&mut child, body) { child.add_statement(JsStatement::new( - JsStatementData::Return(expr), + JsStatementData::Return(Some(expr)), loc.clone(), None, )); diff --git a/crates/js_ast/convert/process_statement.rs b/crates/js_ast/convert/process_statement.rs index f2cdaabb1636d75a56592e38f641a90d48b3aa58..6dbba248ca9167cc3e149c12069116aa2149ba85 100644 --- a/crates/js_ast/convert/process_statement.rs +++ b/crates/js_ast/convert/process_statement.rs @@ -22,18 +22,24 @@ pub fn process_statement(ctx: &mut BlockConvertContext, statement: Statement) -> // and `a` will be looked up before it is re-declared. // we will also return in the noreturn case let init = hoist_expr_contents(ctx, decl.init)?; - let name = ctx.declare_identifier(decl.name); - ctx.add_statement(JsStatement::new( - JsStatementData::Declaration(JSDeclaration { - name: name.get(&ctx), - init: Some(init), - is_constant: decl.is_constant, - }), + let decl_data = JSDeclaration { + name: name.get(&ctx), + init: Some(init), + is_constant: decl.is_constant, + }; + let stmt = JsStatement::new( + if decl.is_pub { + JsStatementData::Export(JsExport::Declaration(Box::new(decl_data))) + } else { + JsStatementData::Declaration(decl_data) + }, decl.loc, decl.comment.map(|x| x.into()), - )); + ); + + ctx.add_statement(stmt); Some(()) } @@ -56,6 +62,7 @@ pub fn process_statement(ctx: &mut BlockConvertContext, statement: Statement) -> body, loc, comment, + is_pub, .. }) => { let name = ctx.declare_identifier(name); @@ -68,12 +75,18 @@ pub fn process_statement(ctx: &mut BlockConvertContext, statement: Statement) -> None, )); + let decl_data = JSDeclaration { + name: name.get(&ctx), + init: processed, + is_constant: true, + }; + ctx.add_statement(JsStatement::new( - JsStatementData::Declaration(JSDeclaration { - name: name.get(&ctx), - init: processed, - is_constant: true, - }), + if is_pub { + JsStatementData::Export(JsExport::Declaration(Box::new(decl_data))) + } else { + JsStatementData::Declaration(decl_data) + }, loc, comment.map(|x| x.into()), )); diff --git a/crates/js_ast/js_ast.rs b/crates/js_ast/js_ast.rs index a580ac8502ce38df702844a319c825cd030a03ad..29bdcdfe339551d713456dd52330e5b24166bf50 100644 --- a/crates/js_ast/js_ast.rs +++ b/crates/js_ast/js_ast.rs @@ -109,7 +109,7 @@ pub enum JsElse { #[derive(Debug, PartialEq, Clone)] pub enum JsStatementData { Expression(JsExpr), - Return(JsExpr), + Return(Option), Export(JsExport), Declaration(JSDeclaration), Function(JSFunctionDeclaration), diff --git a/crates/js_printer/lib.rs b/crates/js_printer/lib.rs index b90a4a2850196ea232831af66d36017ead017334..82678a2bbc537625748357d1f623957181a99c1f 100644 --- a/crates/js_printer/lib.rs +++ b/crates/js_printer/lib.rs @@ -53,8 +53,13 @@ impl Print for JsStatement { write!(w, ";") } js_ast::JsStatementData::Return(expr) => { - write!(w, "return ")?; - expr.print(w) + write!(w, "return")?; + if let Some(expr) = expr { + write!(w, " ")?; + expr.print(w)?; + } + write!(w, ";")?; + Ok(()) } js_ast::JsStatementData::Export(export) => export.print(w), js_ast::JsStatementData::Declaration(decl) => decl.print(w), diff --git a/crates/parser/parse_block.rs b/crates/parser/parse_block.rs index 4ae805a759d0dd7b58c6642c94dc83e5c821c9e4..fc64e74aacce9ebf988d6b7a6c196f2901fd3a56 100644 --- a/crates/parser/parse_block.rs +++ b/crates/parser/parse_block.rs @@ -12,6 +12,9 @@ pub fn parse_block(p: &mut Parser) -> Result { let mut statements = Vec::new(); + let was_top_level = p.is_top_level; + p.is_top_level = false; + 'outer: loop { let is_statement_like_expression = p .peek() @@ -20,6 +23,7 @@ pub fn parse_block(p: &mut Parser) -> Result { let statement = match parse_statement(p) { Ok(statement) => statement, Err(err) => { + p.is_top_level = was_top_level; return Err(err); } }; @@ -31,6 +35,7 @@ pub fn parse_block(p: &mut Parser) -> Result { match p.peek() { Some(Token::ClosedBrace) => { p.next(); + p.is_top_level = was_top_level; return Ok(Block { statements, has_implicit_return: false, @@ -41,6 +46,8 @@ pub fn parse_block(p: &mut Parser) -> Result { }, Some(Token::ClosedBrace) => { p.next(); + p.is_top_level = was_top_level; + if !statement.is_expression() { return Err(p .error("Expected semicolon or '}' after statement".to_string()) @@ -54,6 +61,7 @@ pub fn parse_block(p: &mut Parser) -> Result { } _ => { if statement.require_semicolon(is_statement_like_expression) { + p.is_top_level = was_top_level; return Err(p .error("Expected semicolon or '}' after statement".to_string()) .into()); diff --git a/crates/parser/parse_expr_basic.rs b/crates/parser/parse_expr_basic.rs index efcf48fb65ecc9b0f8c8512e682ddfe7aab3d96b..ad484d31b2941b035d86aee60b2dacd64ced832f 100644 --- a/crates/parser/parse_expr_basic.rs +++ b/crates/parser/parse_expr_basic.rs @@ -7,7 +7,7 @@ use crate::{ parse_expr_if::parse_expr_if, Parser, }; -pub fn parse_expr_basic(p: &mut Parser) -> Result { +pub fn maybe_parse_expr_basic(p: &mut Parser) -> Result> { let loc = p.loc(); let comments = p.take_comments(); let mut expr = match p.peek() { @@ -63,17 +63,15 @@ pub fn parse_expr_basic(p: &mut Parser) -> Result { } Some(Token::Return) => { - if p.allow_return { + if p.is_top_level { return Err(p .error("'return' cannot be used at top level".to_string()) .into()); } else { _ = p.next(); - return Ok(Expr::new( - ExprData::Return(Box::new(parse_expr(p)?)), - loc, - comments, - )); + let inner = maybe_parse_expr_basic(p)?.map(Box::new); + + return Ok(Some(Expr::new(ExprData::Return(inner), loc, comments))); } } @@ -111,8 +109,7 @@ pub fn parse_expr_basic(p: &mut Parser) -> Result { ) } - Some(t) => return Err(p.error(format!("Unexpected '{}'", t)).into()), - None => return Err(p.error("Unexpected end of file".to_string()).into()), + Some(_) | None => return Ok(None), }; // Parse function calls and field accesses @@ -176,5 +173,15 @@ pub fn parse_expr_basic(p: &mut Parser) -> Result { } } - Ok(expr) + Ok(Some(expr)) +} + +pub fn parse_expr_basic(p: &mut Parser) -> Result { + match maybe_parse_expr_basic(p)? { + Some(expr) => Ok(expr), + None => match p.peek() { + Some(token) => Err(p.error(format!("Unexpected '{}'", token)).into()), + None => Err(p.error("Unexpected end of file".to_string()).into()), + }, + } } diff --git a/crates/parser/parse_fn_declaration.rs b/crates/parser/parse_fn_declaration.rs index c1b576cec8709710e413f4f1ae9726d6ba80635f..e52179524e2b97e25d29d1e08f60d7d1ee9e65d9 100644 --- a/crates/parser/parse_fn_declaration.rs +++ b/crates/parser/parse_fn_declaration.rs @@ -84,8 +84,8 @@ pub fn parse_fn_declaration( _ => Type::Infer, }; - let was_allow_return = p.allow_return; - p.allow_return = true; + let was_in_fn = p.is_in_fn; + p.is_in_fn = true; let body = match p.peek() { Some(Token::OpenBrace) => { @@ -108,7 +108,7 @@ pub fn parse_fn_declaration( } }; - p.allow_return = was_allow_return; + p.is_in_fn = was_in_fn; Ok(FnDecl { name, diff --git a/crates/parser/parse_statement.rs b/crates/parser/parse_statement.rs index 537f9512571cf182854ce26e549ded5c65d729be..0e806628140dfb10ce687edc6c95a67a15e0e944 100644 --- a/crates/parser/parse_statement.rs +++ b/crates/parser/parse_statement.rs @@ -26,6 +26,11 @@ pub fn parse_statement(p: &mut Parser) -> Result { match p.peek() { Some(Token::DocComment(_)) => unreachable!(), Some(Token::Pub) => { + if !p.is_top_level { + return Err(p + .error("Public declarations are not allowed inside of functions".to_string()) + .into()); + } _ = p.next(); match p.peek() { Some(Token::Var) | Some(Token::Let) => Ok(Statement::VarDecl(parse_declaration( diff --git a/crates/parser/parser.rs b/crates/parser/parser.rs index 465ce180e9bdd2c388361825407ee0d14bedbf05..8b7d0aa24f1982aae893c71236d0cd6dce03863f 100644 --- a/crates/parser/parser.rs +++ b/crates/parser/parser.rs @@ -52,8 +52,12 @@ pub struct Parser { tokens: Vec, cursor: usize, - /// Whether or not the parser is at the top level - pub allow_return: bool, + /// Whether or not the parser is at the top level. + /// Currently controls the validity of `pub` keywords. + pub is_top_level: bool, + /// Whether or not the parser is inside a function. + /// Controls the validity of the `return` keyword. + pub is_in_fn: bool, comments: Vec, } @@ -63,7 +67,8 @@ impl Parser { Parser { tokens, cursor: 0, - allow_return: false, + is_top_level: true, + is_in_fn: false, comments: vec![], } } @@ -72,8 +77,6 @@ impl Parser { self.cursor == self.tokens.len() } - // TODO(@paperdave): fix parser token stuff. its very messy rn - pub fn next(&mut self) -> Option { if self.cursor == self.tokens.len() { return None; -- 2.54.0