diff --git a/compiler/forget/Cargo.lock b/compiler/forget/Cargo.lock index 2a132d093c..55f707deca 100644 --- a/compiler/forget/Cargo.lock +++ b/compiler/forget/Cargo.lock @@ -153,6 +153,15 @@ dependencies = [ "rustc-demangle", ] +[[package]] +name = "backtrace-ext" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "537beee3be4a18fb023b570f80e3ae28003db9167a751266b259926e25539d50" +dependencies = [ + "backtrace", +] + [[package]] name = "base64" version = "0.13.1" @@ -232,6 +241,8 @@ dependencies = [ "estree", "hir", "indexmap 2.0.0", + "miette 5.9.0", + "thiserror", ] [[package]] @@ -276,7 +287,7 @@ dependencies = [ "encode_unicode", "lazy_static", "libc", - "windows-sys", + "windows-sys 0.45.0", ] [[package]] @@ -423,6 +434,27 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "88bffebc5d80432c9b140ee17875ff173a8ab62faad5b257da912bd2f6c1c0a1" +[[package]] +name = "errno" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4bcfec3a70f97c962c307b2d2c56e358cf1d00b558d74262b5f929ee8cc7e73a" +dependencies = [ + "errno-dragonfly", + "libc", + "windows-sys 0.48.0", +] + +[[package]] +name = "errno-dragonfly" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aa68f1b12764fab894d2755d2518754e71b4fd80ecfb822714a1206c2aab39bf" +dependencies = [ + "cc", + "libc", +] + [[package]] name = "estree" version = "0.1.0" @@ -460,6 +492,7 @@ dependencies = [ "estree-swc", "hir", "insta", + "miette 5.9.0", ] [[package]] @@ -667,6 +700,17 @@ dependencies = [ "yaml-rust", ] +[[package]] +name = "io-lifetimes" +version = "1.0.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eae7b9aee968036d54dce06cebaefd919e4472e753296daccd6d344e3e2df0c2" +dependencies = [ + "hermit-abi 0.3.2", + "libc", + "windows-sys 0.48.0", +] + [[package]] name = "is-macro" version = "0.3.0" @@ -680,6 +724,18 @@ dependencies = [ "syn 2.0.23", ] +[[package]] +name = "is-terminal" +version = "0.4.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "adcf93614601c8129ddf72e2d5633df827ba6551541c6d8c59520a371475be1f" +dependencies = [ + "hermit-abi 0.3.2", + "io-lifetimes", + "rustix", + "windows-sys 0.48.0", +] + [[package]] name = "is_ci" version = "1.1.1" @@ -810,6 +866,12 @@ version = "0.5.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0717cef1bc8b636c6e1c1bbdefc09e6322da8a9321966e8928ef80d20f7f770f" +[[package]] +name = "linux-raw-sys" +version = "0.3.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ef53942eb7bf7ff43a617b3e2c1c4a5ecf5944a7c1bc12d7ee39bbb15e5c1519" + [[package]] name = "lock_api" version = "0.4.10" @@ -858,12 +920,33 @@ checksum = "1c90329e44f9208b55f45711f9558cec15d7ef8295cc65ecd6d4188ae8edc58c" dependencies = [ "atty", "backtrace", - "miette-derive", + "miette-derive 4.7.1", "once_cell", "owo-colors", - "supports-color", - "supports-hyperlinks", - "supports-unicode", + "supports-color 1.3.1", + "supports-hyperlinks 1.2.0", + "supports-unicode 1.0.2", + "terminal_size", + "textwrap", + "thiserror", + "unicode-width", +] + +[[package]] +name = "miette" +version = "5.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a236ff270093b0b67451bc50a509bd1bad302cb1d3c7d37d5efe931238581fa9" +dependencies = [ + "backtrace", + "backtrace-ext", + "is-terminal", + "miette-derive 5.9.0", + "once_cell", + "owo-colors", + "supports-color 2.0.0", + "supports-hyperlinks 2.1.0", + "supports-unicode 2.0.0", "terminal_size", "textwrap", "thiserror", @@ -881,6 +964,17 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "miette-derive" +version = "5.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4901771e1d44ddb37964565c654a3223ba41a594d02b8da471cc4464912b5cfa" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.23", +] + [[package]] name = "minimal-lexical" version = "0.2.1" @@ -1287,6 +1381,20 @@ dependencies = [ "semver 0.9.0", ] +[[package]] +name = "rustix" +version = "0.37.22" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8818fa822adcc98b18fedbb3632a6a33213c070556b5aa7c4c8cc21cff565c4c" +dependencies = [ + "bitflags 1.3.2", + "errno", + "io-lifetimes", + "libc", + "linux-raw-sys", + "windows-sys 0.48.0", +] + [[package]] name = "rustversion" version = "1.0.13" @@ -1549,6 +1657,16 @@ dependencies = [ "is_ci", ] +[[package]] +name = "supports-color" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4950e7174bffabe99455511c39707310e7e9b440364a2fcb1cc21521be57b354" +dependencies = [ + "is-terminal", + "is_ci", +] + [[package]] name = "supports-hyperlinks" version = "1.2.0" @@ -1558,6 +1676,15 @@ dependencies = [ "atty", ] +[[package]] +name = "supports-hyperlinks" +version = "2.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f84231692eb0d4d41e4cdd0cabfdd2e6cd9e255e65f80c9aa7c98dd502b4233d" +dependencies = [ + "is-terminal", +] + [[package]] name = "supports-unicode" version = "1.0.2" @@ -1567,6 +1694,15 @@ dependencies = [ "atty", ] +[[package]] +name = "supports-unicode" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4b6c2cb240ab5dd21ed4906895ee23fe5a48acdbd15a3ce388e7b62a9b66baf7" +dependencies = [ + "is-terminal", +] + [[package]] name = "swc" version = "0.264.8" @@ -2293,7 +2429,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "108322b719696e8c368c39dc6d8748494ea2aa870e7d80ea5956078aa6b4dd4d" dependencies = [ "anyhow", - "miette", + "miette 4.7.1", "once_cell", "parking_lot", "swc_common", @@ -2758,6 +2894,15 @@ dependencies = [ "windows-targets 0.42.2", ] +[[package]] +name = "windows-sys" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "677d2418bec65e3338edb076e806bc1ec15693c5d0104683f2efe857f61056a9" +dependencies = [ + "windows-targets 0.48.1", +] + [[package]] name = "windows-targets" version = "0.42.2" diff --git a/compiler/forget/Cargo.toml b/compiler/forget/Cargo.toml index b135c5e09a..b20b6328ab 100644 --- a/compiler/forget/Cargo.toml +++ b/compiler/forget/Cargo.toml @@ -1,5 +1,5 @@ [workspace] - +resolver = "2" members = [ "crates/build-hir", "crates/fixtures", @@ -7,4 +7,14 @@ members = [ "crates/swc-demo", "crates/estree", "crates/estree-swc", -] \ No newline at end of file +] + +# Make insta run faster by compiling with release mode optimizations +# https://docs.rs/insta/latest/insta/#optional-faster-runs +[profile.dev.package.insta] +opt-level = 3 + +# Make insta diffing libary faster by compiling with release mode optimizations +# https://docs.rs/insta/latest/insta/#optional-faster-runs +[profile.dev.package.similar] +opt-level = 3 \ No newline at end of file diff --git a/compiler/forget/crates/build-hir/Cargo.toml b/compiler/forget/crates/build-hir/Cargo.toml index 5e108f3706..b5fbd2a4eb 100644 --- a/compiler/forget/crates/build-hir/Cargo.toml +++ b/compiler/forget/crates/build-hir/Cargo.toml @@ -10,3 +10,5 @@ hir = { path = "../hir" } estree = { path = "../estree" } indexmap = "2.0.0" bumpalo = "3.13.0" +miette = { version = "5.9.0" } +thiserror = "1.0.41" diff --git a/compiler/forget/crates/build-hir/src/build.rs b/compiler/forget/crates/build-hir/src/build.rs index 8ca67ec204..a4241d7ee2 100644 --- a/compiler/forget/crates/build-hir/src/build.rs +++ b/compiler/forget/crates/build-hir/src/build.rs @@ -1,4 +1,4 @@ -use bumpalo::collections::{CollectIn, String}; +use bumpalo::collections::{String, Vec}; use estree::{ AssignmentTarget, BinaryExpression, ExpressionLike, ForInit, ForStatement, FunctionDeclaration, IfStatement, Literal, LiteralValue, Pattern, Statement, VariableDeclarationKind, @@ -9,7 +9,11 @@ use hir::{ PrimitiveValue, TerminalValue, }; -use crate::builder::{Binding, Builder, LoopScope}; +use crate::{ + builder::{Binding, Builder, LoopScope}, + error::DiagnosticError, + BuildDiagnostic, ErrorSeverity, +}; /// Converts a React function in ESTree format into HIR. Returns the HIR /// if it was constructed sucessfully, otherwise a list of diagnostics @@ -20,7 +24,7 @@ use crate::builder::{Binding, Builder, LoopScope}; pub fn build<'a>( environment: &'a Environment<'a>, fun: FunctionDeclaration, -) -> Result, Diagnostic> { +) -> Result, BuildDiagnostic> { let mut builder = Builder::new(environment); lower_statement(environment, &mut builder, fun.body.unwrap(), None)?; @@ -57,7 +61,7 @@ fn lower_statement<'a>( builder: &mut Builder<'a>, stmt: Statement, label: Option>, -) -> Result<(), Diagnostic> { +) -> Result<(), BuildDiagnostic> { match stmt { Statement::BlockStatement(stmt) => { for stmt in stmt.body { @@ -86,7 +90,7 @@ fn lower_statement<'a>( } Statement::ReturnStatement(stmt) => { let value = match stmt.argument { - Some(argument) => lower_expression_to_temporary(env, builder, argument), + Some(argument) => lower_expression_to_temporary(env, builder, argument)?, None => lower_value_to_temporary( env, builder, @@ -101,7 +105,7 @@ fn lower_statement<'a>( ); } Statement::ExpressionStatement(stmt) => { - lower_expression_to_temporary(env, builder, stmt.expression); + lower_expression_to_temporary(env, builder, stmt.expression)?; } Statement::EmptyStatement(_) => { // no-op @@ -110,25 +114,37 @@ fn lower_statement<'a>( let kind = match stmt.kind { VariableDeclarationKind::Const => InstructionKind::Const, VariableDeclarationKind::Let => InstructionKind::Let, - VariableDeclarationKind::Var => panic!("`var` declarations are not supported"), + VariableDeclarationKind::Var => { + return Err(BuildDiagnostic::new( + DiagnosticError::VariableDeclarationKindIsVar, + ErrorSeverity::Unsupported, + stmt.range, + )); + } }; for declaration in stmt.declarations { if let Some(init) = declaration.init { - let value = lower_expression_to_temporary(env, builder, init); + let value = lower_expression_to_temporary(env, builder, init)?; lower_assignment( env, builder, kind, AssignmentTarget::Pattern(declaration.id.into()), value, - ); + )?; } else { if let Pattern::Identifier(id) = declaration.id { // TODO: handle unbound variables - let binding = builder.resolve_binding(&id).unwrap(); + let binding = builder.resolve_binding(&id)?; let identifier = match binding { Binding::Local(identifier) => identifier, - _ => panic!("Expected variable declaration to be a local binding"), + _ => { + return Err(BuildDiagnostic::new( + DiagnosticError::VariableDeclarationBindingIsNonLocal, + ErrorSeverity::Invariant, + id.range, + )); + } }; let place = Place { effect: None, @@ -158,24 +174,24 @@ fn lower_statement<'a>( } = *stmt; let consequent_block = builder.enter(BlockKind::Block, |builder| { - lower_statement(env, builder, consequent, None).unwrap(); - TerminalValue::Goto(hir::GotoTerminal { + lower_statement(env, builder, consequent, None)?; + Ok(TerminalValue::Goto(hir::GotoTerminal { block: fallthrough_block.id, kind: GotoKind::Break, - }) - }); + })) + })?; let alternate_block = builder.enter(BlockKind::Block, |builder| { if let Some(alternate) = alternate { - lower_statement(env, builder, alternate, None).unwrap(); + lower_statement(env, builder, alternate, None)?; } - TerminalValue::Goto(hir::GotoTerminal { + Ok(TerminalValue::Goto(hir::GotoTerminal { block: fallthrough_block.id, kind: GotoKind::Break, - }) - }); + })) + })?; - let test = lower_expression_to_temporary(env, builder, test); + let test = lower_expression_to_temporary(env, builder, test)?; let terminal = TerminalValue::If(hir::IfTerminal { test, consequent: consequent_block, @@ -201,26 +217,31 @@ fn lower_statement<'a>( let init_block = builder.enter(BlockKind::Loop, |builder| { if let Some(ForInit::VariableDeclaration(decl)) = init { - lower_statement(env, builder, Statement::VariableDeclaration(decl), None) - .unwrap(); - TerminalValue::Goto(hir::GotoTerminal { + lower_statement(env, builder, Statement::VariableDeclaration(decl), None)?; + Ok(TerminalValue::Goto(hir::GotoTerminal { block: test_block.id, kind: GotoKind::Break, - }) + })) } else { - panic!("Expected for statement to have a variable declaration initializer") + Err(BuildDiagnostic::new( + DiagnosticError::ForStatementIsMissingInitializer, + ErrorSeverity::Todo, + None, + )) } - }); + })?; - let update_block = update.map(|update| { - builder.enter(BlockKind::Loop, |builder| { - lower_expression_to_temporary(env, builder, update); - TerminalValue::Goto(hir::GotoTerminal { - block: test_block.id, - kind: GotoKind::Break, + let update_block = update + .map(|update| { + builder.enter(BlockKind::Loop, |builder| { + lower_expression_to_temporary(env, builder, update)?; + Ok(TerminalValue::Goto(hir::GotoTerminal { + block: test_block.id, + kind: GotoKind::Break, + })) }) }) - }); + .transpose()?; let body_block = builder.enter(BlockKind::Block, |builder| { let loop_ = LoopScope { @@ -229,13 +250,13 @@ fn lower_statement<'a>( break_block: fallthrough_block.id, }; builder.enter_loop(loop_, |builder| { - lower_statement(env, builder, body, None).unwrap(); - TerminalValue::Goto(hir::GotoTerminal { + lower_statement(env, builder, body, None)?; + Ok(TerminalValue::Goto(hir::GotoTerminal { block: update_block.unwrap_or(test_block.id), kind: GotoKind::Continue, - }) + })) }) - }); + })?; let terminal = TerminalValue::For(ForTerminal { body: body_block, @@ -247,7 +268,7 @@ fn lower_statement<'a>( builder.terminate_with_fallthrough(terminal, test_block); if let Some(test) = test { - let test_value = lower_expression_to_temporary(env, builder, test); + let test_value = lower_expression_to_temporary(env, builder, test)?; let terminal = TerminalValue::Branch(BranchTerminal { test: test_value, consequent: body_block, @@ -255,7 +276,11 @@ fn lower_statement<'a>( }); builder.terminate_with_fallthrough(terminal, fallthrough_block); } else { - panic!("Expected for statement to have a tesst block"); + return Err(BuildDiagnostic::new( + DiagnosticError::ForStatementIsMissingTest, + ErrorSeverity::Todo, + stmt.range, + )); } } _ => todo!("Lower {stmt:#?}"), @@ -268,9 +293,9 @@ fn lower_expression_to_temporary<'a>( env: &'a Environment<'a>, builder: &mut Builder<'a>, expr: ExpressionLike, -) -> Place<'a> { - let value = lower_expression(env, builder, expr); - lower_value_to_temporary(env, builder, value) +) -> Result, BuildDiagnostic> { + let value = lower_expression(env, builder, expr)?; + Ok(lower_value_to_temporary(env, builder, value)) } /// Converts an ESTree Expression into an HIR InstructionValue. Note that while only a single @@ -281,11 +306,11 @@ fn lower_expression<'a>( env: &'a Environment<'a>, builder: &mut Builder<'a>, expr: ExpressionLike, -) -> InstructionValue<'a> { - match expr { +) -> Result, BuildDiagnostic> { + Ok(match expr { ExpressionLike::Identifier(expr) => { // TODO: handle unbound variables - let binding = builder.resolve_binding(&expr).unwrap(); + let binding = builder.resolve_binding(&expr)?; match binding { Binding::Local(identifier) => { let place = Place { @@ -303,23 +328,23 @@ fn lower_expression<'a>( value: lower_primitive(env, builder, *expr), }), ExpressionLike::ArrayExpression(expr) => { - let elements = expr - .elements - .into_iter() - .map(|expr| match expr { + let mut elements = Vec::with_capacity_in(expr.elements.len(), &env.allocator); + for expr in expr.elements { + let element = match expr { ExpressionLike::SpreadElement(expr) => ArrayElement::Spread( - lower_expression_to_temporary(env, builder, expr.argument), + lower_expression_to_temporary(env, builder, expr.argument)?, ), - _ => ArrayElement::Place(lower_expression_to_temporary(env, builder, expr)), - }) - .collect_in(env.allocator); + _ => ArrayElement::Place(lower_expression_to_temporary(env, builder, expr)?), + }; + elements.push(element); + } InstructionValue::Array(hir::Array { elements }) } ExpressionLike::AssignmentExpression(expr) => match expr.operator { estree::AssignmentOperator::Equals => { - let right = lower_expression_to_temporary(env, builder, expr.right); - lower_assignment(env, builder, InstructionKind::Reassign, expr.left, right) + let right = lower_expression_to_temporary(env, builder, expr.right)?; + lower_assignment(env, builder, InstructionKind::Reassign, expr.left, right)? } _ => todo!("lower assignment expr {:#?}", expr), }, @@ -331,8 +356,8 @@ fn lower_expression<'a>( right, .. } = *expr; - let left = lower_expression_to_temporary(env, builder, left); - let right = lower_expression_to_temporary(env, builder, right); + let left = lower_expression_to_temporary(env, builder, left)?; + let right = lower_expression_to_temporary(env, builder, right)?; InstructionValue::Binary(hir::Binary { left, operator, @@ -342,11 +367,15 @@ fn lower_expression<'a>( // Cases that cannot appear in expression position but which are included in ExpressionLike // to make serialization easier - ExpressionLike::SpreadElement(_) => { - panic!("SpreadElement may not appear in normal expression position") + ExpressionLike::SpreadElement(expr) => { + return Err(BuildDiagnostic::new( + DiagnosticError::NonExpressionInExpressionPosition, + ErrorSeverity::Invariant, + expr.range, + )); } _ => todo!("Lower expr {expr:#?}"), - } + }) } fn lower_assignment<'a>( @@ -355,11 +384,11 @@ fn lower_assignment<'a>( kind: InstructionKind, lvalue: AssignmentTarget, value: Place<'a>, -) -> InstructionValue<'a> { - match lvalue { +) -> Result, BuildDiagnostic> { + Ok(match lvalue { AssignmentTarget::Pattern(lvalue) => match *lvalue { Pattern::Identifier(lvalue) => { - let place = lower_identifier_for_assignment(env, builder, kind, *lvalue).unwrap(); + let place = lower_identifier_for_assignment(env, builder, kind, *lvalue)?; let temporary = lower_value_to_temporary( env, builder, @@ -373,7 +402,7 @@ fn lower_assignment<'a>( _ => todo!("lower assignment pattern for {:#?}", lvalue), }, _ => todo!("lower assignment for {:#?}", lvalue), - } + }) } fn lower_identifier_for_assignment<'a>( @@ -381,11 +410,15 @@ fn lower_identifier_for_assignment<'a>( builder: &mut Builder<'a>, _kind: InstructionKind, identifier: estree::Identifier, -) -> Option> { +) -> Result, BuildDiagnostic> { let binding = builder.resolve_binding(&identifier)?; match binding { - Binding::Module(..) | Binding::Global => panic!("Cannot reassign a global"), - Binding::Local(id) => Some(Place { + Binding::Module(..) | Binding::Global => Err(BuildDiagnostic::new( + DiagnosticError::ReassignedGlobal, + ErrorSeverity::InvalidReact, + identifier.range, + )), + Binding::Local(id) => Ok(Place { identifier: id, effect: None, }), @@ -440,5 +473,3 @@ fn lower_primitive<'a>( _ => todo!("Lower literal {literal:#?}"), } } - -type Diagnostic = (); diff --git a/compiler/forget/crates/build-hir/src/builder.rs b/compiler/forget/crates/build-hir/src/builder.rs index 3dc358cbd5..519b18e505 100644 --- a/compiler/forget/crates/build-hir/src/builder.rs +++ b/compiler/forget/crates/build-hir/src/builder.rs @@ -7,6 +7,8 @@ use hir::{ }; use indexmap::IndexMap; +use crate::{invariant, BuildDiagnostic, DiagnosticError, ErrorSeverity}; + /// Helper struct used when converting from ESTree to HIR. Includes: /// - Variable resolution /// - Label resolution (for labeled statements and break/continue) @@ -46,7 +48,9 @@ pub(crate) enum Binding<'a> { #[derive(Clone, PartialEq, Eq, Debug)] enum ControlFlowScope<'a> { Loop(LoopScope<'a>), + // Switch(SwitchScope<'a>), + #[allow(dead_code)] Label(LabelScope<'a>), } @@ -102,7 +106,7 @@ impl<'a> Builder<'a> { /// /// TODO: refine the type, only invariants should be possible here, /// not other types of errors - pub(crate) fn build(self) -> Result, Diagnostic> { + pub(crate) fn build(self) -> Result, BuildDiagnostic> { let mut hir = HIR { entry: self.entry, blocks: self.completed, @@ -168,22 +172,34 @@ impl<'a> Builder<'a> { } } - pub(crate) fn enter(&mut self, kind: BlockKind, f: F) -> BlockId + pub(crate) fn enter(&mut self, kind: BlockKind, f: F) -> Result where - F: FnOnce(&mut Self) -> TerminalValue<'a>, + F: FnOnce(&mut Self) -> Result, BuildDiagnostic>, { let wip = self.reserve(kind); let id = wip.id; - self.enter_reserved(wip, f); - id + self.enter_reserved(wip, f)?; + Ok(id) } - fn enter_reserved(&mut self, wip: WipBlock<'a>, f: F) + fn enter_reserved(&mut self, wip: WipBlock<'a>, f: F) -> Result<(), BuildDiagnostic> where - F: FnOnce(&mut Self) -> TerminalValue<'a>, + F: FnOnce(&mut Self) -> Result, BuildDiagnostic>, { let current = std::mem::replace(&mut self.wip, wip); - let terminal = f(self); + + let (result, terminal) = match f(self) { + Ok(terminal) => (Ok(()), terminal), + Err(error) => ( + Err(error), + // TODO: add a `Terminal::Error` variant + TerminalValue::Goto(hir::GotoTerminal { + block: current.id, + kind: GotoKind::Break, + }), + ), + }; + let completed = std::mem::replace(&mut self.wip, current); self.completed.insert( completed.id, @@ -198,11 +214,16 @@ impl<'a> Builder<'a> { predecessors: Default::default(), }, ); + result } - pub(crate) fn enter_loop(&mut self, scope: LoopScope<'a>, f: F) -> TerminalValue<'a> + pub(crate) fn enter_loop( + &mut self, + scope: LoopScope<'a>, + f: F, + ) -> Result, BuildDiagnostic> where - F: FnOnce(&mut Self) -> TerminalValue<'a>, + F: FnOnce(&mut Self) -> Result, BuildDiagnostic>, { self.scopes.push(ControlFlowScope::Loop(scope.clone())); let terminal = f(self); @@ -230,7 +251,7 @@ impl<'a> Builder<'a> { pub(crate) fn resolve_break( &self, label: Option<&estree::Identifier>, - ) -> Result { + ) -> Result { for scope in self.scopes.iter().rev() { match (label, scope.label()) { // If this is an unlabeled break, return the most recent break target @@ -243,7 +264,11 @@ impl<'a> Builder<'a> { _ => continue, } } - Err(()) + Err(BuildDiagnostic::new( + DiagnosticError::UnresolvedBreakTarget, + ErrorSeverity::InvalidSyntax, + None, + )) } /// Resolves the target for the given continue label (if present), or returns the default @@ -252,7 +277,7 @@ impl<'a> Builder<'a> { pub(crate) fn resolve_continue( &self, label: Option<&estree::Identifier>, - ) -> Result { + ) -> Result { for scope in self.scopes.iter().rev() { match scope { ControlFlowScope::Loop(scope) => { @@ -273,31 +298,46 @@ impl<'a> Builder<'a> { match (label, scope.label()) { (Some(label), Some(scope_label)) if label.name.as_str() == scope_label => { // Error, the continue referred to a label that is not a loop - return Err(()); + return Err(BuildDiagnostic::new( + DiagnosticError::ContinueTargetIsNotALoop, + ErrorSeverity::InvalidSyntax, + None, + )); } _ => continue, } } } } - Err(()) + Err(BuildDiagnostic::new( + DiagnosticError::UnresolvedContinueTarget, + ErrorSeverity::InvalidSyntax, + None, + )) } pub(crate) fn resolve_binding( &mut self, identifier: &estree::Identifier, - ) -> Option> { - identifier.binding.as_ref().map(|binding| match binding { - estree::Binding::Global => Binding::Global, - estree::Binding::Local(id) => Binding::Local( - self.environment - .resolve_binding_identifier(&identifier.name, *id), - ), - estree::Binding::Module(id) => Binding::Module( - self.environment - .resolve_binding_identifier(&identifier.name, *id), - ), - }) + ) -> Result, BuildDiagnostic> { + match &identifier.binding { + Some(binding) => Ok(match binding { + estree::Binding::Global => Binding::Global, + estree::Binding::Local(id) => Binding::Local( + self.environment + .resolve_binding_identifier(&identifier.name, *id), + ), + estree::Binding::Module(id) => Binding::Module( + self.environment + .resolve_binding_identifier(&identifier.name, *id), + ), + }), + _ => Err(BuildDiagnostic::new( + DiagnosticError::UnknownIdentifier, + ErrorSeverity::Invariant, + identifier.range.clone(), + )), + } } } @@ -403,13 +443,17 @@ fn remove_unreachable_do_while_statements<'a>(hir: &mut HIR<'a>) { /// Updates the instruction ids for all instructions and blocks /// Relies on the blocks being in reverse postorder to ensure that id ordering is correct -fn mark_instruction_ids<'a>(hir: &mut HIR<'a>) -> Result<(), Diagnostic> { +fn mark_instruction_ids<'a>(hir: &mut HIR<'a>) -> Result<(), BuildDiagnostic> { let mut id_gen = InstructionIdGenerator::new(); let mut visited = HashSet::<(usize, usize)>::new(); for (block_ix, block) in hir.blocks.values_mut().enumerate() { for (instr_ix, instr) in block.instructions.iter_mut().enumerate() { invariant(visited.insert((block_ix, instr_ix)), || { - format!("Expected bb{block_ix} i{instr_ix} not to have been visited yet") + BuildDiagnostic::new( + DiagnosticError::BlockVisitedTwice { block: block.id }, + ErrorSeverity::Invariant, + None, + ) })?; instr.id = id_gen.next(); } @@ -443,16 +487,3 @@ fn mark_predecessors<'a>(hir: &mut HIR<'a>) { } visit(hir.entry, None, hir, &mut visited); } - -fn invariant(cond: bool, f: F) -> Result<(), Diagnostic> -where - F: FnOnce() -> std::string::String, -{ - if !cond { - let msg = f(); - panic!("Invariant: {msg}"); - } - Ok(()) -} - -type Diagnostic = (); diff --git a/compiler/forget/crates/build-hir/src/error.rs b/compiler/forget/crates/build-hir/src/error.rs new file mode 100644 index 0000000000..7bd64ef61c --- /dev/null +++ b/compiler/forget/crates/build-hir/src/error.rs @@ -0,0 +1,125 @@ +use estree::SourceRange; +use hir::BlockId; +use miette::{ByteOffset, Diagnostic, SourceSpan}; +use thiserror::Error; + +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Error)] +pub enum ErrorSeverity { + /// A feature that is intended to work but not yet implemented + #[error("Not implemented")] + Todo, + + /// Syntax that is valid but inentionally not supported + #[error("Unsupported")] + Unsupported, + + /// Invalid syntax + #[error("Invalid JavaScript")] + InvalidSyntax, + + /// Valid syntax, but invalid React + #[error("Invalid React")] + InvalidReact, + + /// Internal compiler error (ICE) + #[error("Internal error")] + Invariant, +} + +/// Errors which can occur during HIR construction +#[derive(Error, Diagnostic, Debug)] +pub enum DiagnosticError { + /// ErrorSeverity::Unsupported + #[error( + "Variable declarations must be `let` or `const`, `var` declarations are not supported" + )] + VariableDeclarationKindIsVar, + + /// ErrorSeverity::Invariant + #[error("Invariant: Expected variable declaration to declare a fresh binding")] + VariableDeclarationBindingIsNonLocal, + + /// ErrorSeverity::Todo + #[error("`for` statements must have an initializer, eg `for (**let i = 0**; ...)`")] + ForStatementIsMissingInitializer, + + /// ErrorSeverity::Todo + #[error( + "`for` statements must have a test condition, eg `for (let i = 0; **i < count**; ...)`" + )] + ForStatementIsMissingTest, + + /// ErrorSeverity::Invariant + #[error("Invariant: Expected an expression node")] + NonExpressionInExpressionPosition, + + /// ErrorSeverity::InvalidReact + #[error("React functions may not reassign variables defined outside of the component or hook")] + ReassignedGlobal, + + /// ErrorSeverity::Invariant + #[error("Invariant: Expected block {block} not to have been visited yet")] + BlockVisitedTwice { block: BlockId }, + + /// ErrorSeverity::InvalidSyntax + #[error("Could not resolve a target for `break` statement")] + UnresolvedBreakTarget, + + /// ErrorSeverity::InvalidSyntax + #[error("Could not resolve a target for `continue` statement")] + UnresolvedContinueTarget, + + /// ErrorSeverity::InvalidSyntax + #[error("Labeled `continue` statements must use the label of a loop statement")] + ContinueTargetIsNotALoop, + + /// ErrorSeverity::Invariant + #[error("Invariant: Identifier was not resolved (did name resolution run successfully?)")] + UnknownIdentifier, +} + +#[derive(Error, Diagnostic, Debug)] +#[error("{error}")] +pub struct BuildDiagnostic { + /// The actual error + pub error: DiagnosticError, + + /// Error severity + pub severity: ErrorSeverity, + + /// Source of the error + #[label] + pub range: Option, +} + +impl BuildDiagnostic { + pub fn new( + error: DiagnosticError, + severity: ErrorSeverity, + range: Option, + ) -> Self { + Self { + error, + severity, + range: range.map(|range| { + SourceSpan::new( + ByteOffset::from(range.start as usize - 1).into(), + ByteOffset::from((u32::from(range.end) - range.start) as usize).into(), + ) + }), + } + } +} + +/// Returns Ok(()) if the condition is true, otherwise returns Err() +/// with the diagnostic produced by the provided callback +pub fn invariant(cond: bool, f: F) -> Result<(), BuildDiagnostic> +where + F: FnOnce() -> BuildDiagnostic, +{ + if cond { + Ok(()) + } else { + Err(f()) + } +} diff --git a/compiler/forget/crates/build-hir/src/lib.rs b/compiler/forget/crates/build-hir/src/lib.rs index 8cb58b94e5..15fd7765c5 100644 --- a/compiler/forget/crates/build-hir/src/lib.rs +++ b/compiler/forget/crates/build-hir/src/lib.rs @@ -1,4 +1,6 @@ mod build; mod builder; +mod error; pub use build::build; +pub use error::*; diff --git a/compiler/forget/crates/estree/Cargo.toml b/compiler/forget/crates/estree/Cargo.toml index e5ceb2cbd5..08ede92540 100644 --- a/compiler/forget/crates/estree/Cargo.toml +++ b/compiler/forget/crates/estree/Cargo.toml @@ -12,14 +12,3 @@ insta = { version = "1.30.0", features = ["glob"] } serde = { version = "1.0.164", features = ["derive"] } serde_json = "1.0.99" static_assertions = "1.1.0" - - -# Make insta run faster by compiling with release mode optimizations -# https://docs.rs/insta/latest/insta/#optional-faster-runs -[profile.dev.package.insta] -opt-level = 3 - -# Make insta diffing libary faster by compiling with release mode optimizations -# https://docs.rs/insta/latest/insta/#optional-faster-runs -[profile.dev.package.similar] -opt-level = 3 \ No newline at end of file diff --git a/compiler/forget/crates/estree/src/lib.rs b/compiler/forget/crates/estree/src/lib.rs index a467714bab..89b6b789d0 100644 --- a/compiler/forget/crates/estree/src/lib.rs +++ b/compiler/forget/crates/estree/src/lib.rs @@ -11,7 +11,7 @@ pub struct SourceLocation { pub end: Position, } -#[derive(Serialize, Deserialize, Debug)] +#[derive(Serialize, Deserialize, Debug, Clone)] pub struct Position { /// >= 1 pub line: NonZeroU32, @@ -20,7 +20,7 @@ pub struct Position { } assert_eq_size!(Option, u64); -#[derive(Serialize, Deserialize, Debug)] +#[derive(Serialize, Deserialize, Debug, Clone)] pub struct SourceRange { pub start: u32, // end is exclusive so it can always be non-zero. This allows diff --git a/compiler/forget/crates/fixtures/Cargo.toml b/compiler/forget/crates/fixtures/Cargo.toml index 0587df6654..2c5f4a0e0d 100644 --- a/compiler/forget/crates/fixtures/Cargo.toml +++ b/compiler/forget/crates/fixtures/Cargo.toml @@ -13,3 +13,4 @@ estree-swc = { path = "../estree-swc" } hir = { path = "../hir" } build-hir = { path = "../build-hir" } bumpalo = { version = "3.13.0", features = ["collections"] } +miette = { version = "5.9.0", features = ["backtrace", "fancy"] } diff --git a/compiler/forget/crates/fixtures/tests/fixtures/error.assign-to-global.js b/compiler/forget/crates/fixtures/tests/fixtures/error.assign-to-global.js new file mode 100644 index 0000000000..0434032d6f --- /dev/null +++ b/compiler/forget/crates/fixtures/tests/fixtures/error.assign-to-global.js @@ -0,0 +1,3 @@ +function foo() { + x = true; +} diff --git a/compiler/forget/crates/fixtures/tests/fixtures_test.rs b/compiler/forget/crates/fixtures/tests/fixtures_test.rs index 68b1b4dd17..2b532a66e0 100644 --- a/compiler/forget/crates/fixtures/tests/fixtures_test.rs +++ b/compiler/forget/crates/fixtures/tests/fixtures_test.rs @@ -1,9 +1,12 @@ +use std::fmt::Write; + use build_hir::build; use bumpalo::Bump; use estree::{ModuleItem, Statement}; use estree_swc::parse; use hir::{Environment, Print, Registry}; use insta::{assert_snapshot, glob}; +use miette::{NamedSource, Report}; #[test] fn fixtures() { @@ -24,12 +27,25 @@ fn fixtures() { }, Registry, )); - let hir = build(&environment, *fun).unwrap(); - if ix != 0 { output.push_str("\n\n"); } - hir.print(&mut output).unwrap(); + match build(&environment, *fun) { + Ok(hir) => { + hir.print(&mut output).unwrap(); + } + Err(error) => { + write!(&mut output, "{}", error,).unwrap(); + eprintln!( + "{:?}", + Report::new(error).with_source_code(NamedSource::new( + path.to_string_lossy(), + input.clone(), + )) + ); + continue; + } + }; } } } diff --git a/compiler/forget/crates/fixtures/tests/snapshots/fixtures_test__fixtures@error.assign-to-global.js.snap b/compiler/forget/crates/fixtures/tests/snapshots/fixtures_test__fixtures@error.assign-to-global.js.snap new file mode 100644 index 0000000000..4e02559c6d --- /dev/null +++ b/compiler/forget/crates/fixtures/tests/snapshots/fixtures_test__fixtures@error.assign-to-global.js.snap @@ -0,0 +1,13 @@ +--- +source: crates/fixtures/tests/fixtures_test.rs +expression: "format!(\"Input:\\n{input}\\n\\nOutput:\\n{output}\")" +input_file: crates/fixtures/tests/fixtures/error.assign-to-global.js +--- +Input: +function foo() { + x = true; +} + + +Output: +React functions may not reassign variables defined outside of the component or hook