From 605ec9853960a407261481d57fcf7e31fd85597f Mon Sep 17 00:00:00 2001 From: Teakowa Date: Mon, 31 Aug 2026 19:37:24 +0800 Subject: [PATCH 1/2] feat(opy): complete audited statement grammar surface Add source HIR nodes and diagnostics for del, min/max assignments, continue, goto, and labels while keeping canonical WIR gaps explicit. Fixes #141 --- compatibility/support-matrix.json | 20 ++- crates/opy-rs/src/compiler/mod.rs | 53 ++++++- crates/opy-rs/src/cst.rs | 20 +++ crates/opy-rs/src/hir/dump.rs | 34 +++++ crates/opy-rs/src/hir/types.rs | 26 ++++ crates/opy-rs/src/hir/validate.rs | 54 +++++++- crates/opy-rs/src/lexer.rs | 2 + crates/opy-rs/src/lower.rs | 68 +++++++-- crates/opy-rs/src/parser.rs | 154 +++++++++++++++++++++ crates/opy-rs/src/support.rs | 2 +- crates/opy-rs/src/tooling.rs | 7 + crates/opy-rs/support-matrix.json | 20 ++- crates/opy-rs/tests/issue_141_syntax.rs | 135 ++++++++++++++++++ docs/hir/opy-hir-v2.md | 16 +++ docs/overpy-support/syntax-and-projects.md | 8 +- 15 files changed, 596 insertions(+), 23 deletions(-) create mode 100644 crates/opy-rs/tests/issue_141_syntax.rs diff --git a/compatibility/support-matrix.json b/compatibility/support-matrix.json index 487f96b..dc85abd 100644 --- a/compatibility/support-matrix.json +++ b/compatibility/support-matrix.json @@ -116,6 +116,22 @@ "evidence": ["fixtures:synthetic/issue-33-switch-break", "upstream:src/tests/switches.opy", "upstream:src/tests/loops.opy"], "notes": "Issue #33: break is a real HIR statement, validates its enclosing switch/loop context, and is retained without implicit arm exits." }, + { + "id": "syntax/statement-surface", + "name": "del, continue, goto, labels, and dynamic loc+ targets", + "category": "syntax", + "state": "source-supported", + "evidence": ["tests:crates/opy-rs/tests/issue_141_syntax.rs", "upstream:src/tests/loops.opy", "upstream:src/tests/gotos.opy"], + "notes": "Issue #141: audited statements lower to source HIR nodes with spans; continue context and malformed targets produce structured diagnostics. Canonical WIR lowering remains an explicit integration boundary." + }, + { + "id": "syntax/augmented-min-max", + "name": "min= and max= modification forms", + "category": "syntax", + "state": "source-supported", + "evidence": ["tests:crates/opy-rs/tests/issue_141_syntax.rs", "upstream:src/tests/operators.opy"], + "notes": "Issue #141: min=/max= are retained as source-semantic binary modifications with provenance; Workshop support is not claimed." + }, { "id": "syntax/do-while", "name": "do … while", @@ -759,13 +775,13 @@ "summary": { "byState": { "planned": 0, - "source-supported": 23, + "source-supported": 25, "semantic-supported": 13, "lowering-dependent": 12, "end-to-end-supported": 8 }, "byCategory": { - "syntax": 14, + "syntax": 16, "semantics": 14, "preprocessing": 4, "macros": 3, diff --git a/crates/opy-rs/src/compiler/mod.rs b/crates/opy-rs/src/compiler/mod.rs index 2a2b72b..3c65700 100644 --- a/crates/opy-rs/src/compiler/mod.rs +++ b/crates/opy-rs/src/compiler/mod.rs @@ -819,9 +819,26 @@ impl MacroExpander { .collect::, IntegrationError>>()?, span: *span, }, + Stmt::Delete { target, span } => Stmt::Delete { + target: Box::new(self.expand_expr(target, bindings)?), + span: *span, + }, + Stmt::Goto { + label, + offset, + span, + } => Stmt::Goto { + label: label.clone(), + offset: offset + .as_ref() + .map(|offset| self.expand_expr(offset, bindings).map(Box::new)) + .transpose()?, + span: *span, + }, Stmt::Break { .. } | Stmt::CallSubroutine { .. } | Stmt::Pass { .. } => { statement.clone() } + Stmt::Continue { .. } | Stmt::Label { .. } => statement.clone(), }) } @@ -2005,6 +2022,22 @@ impl<'a> Lowering<'a> { arms, span, } => self.lower_switch(value, arms, *span).map(|action| vec![action]), + Stmt::Delete { span, .. } => Err(self.unsupported( + "delete statements are not representable in canonical WIR", + *span, + )), + Stmt::Continue { span } => Err(self.unsupported( + "continue statements are not representable in canonical WIR", + *span, + )), + Stmt::Goto { span, .. } => Err(self.unsupported( + "goto statements are not representable in canonical WIR", + *span, + )), + Stmt::Label { span, .. } => Err(self.unsupported( + "labels are not representable in canonical WIR", + *span, + )), Stmt::Break { span } => match break_target { Some(BreakTarget::Loop) => Ok(vec![self.wir.actions.push(Action::Call { name: "break".to_string(), @@ -3830,6 +3863,9 @@ fn collect_implicit_stmts( collect_implicit_expr(target, declared_globals, declared_players, globals, players); collect_implicit_expr(value, declared_globals, declared_players, globals, players); } + Stmt::Delete { target, .. } => { + collect_implicit_expr(target, declared_globals, declared_players, globals, players); + } Stmt::If { branches, r#else, .. } => { @@ -3928,7 +3964,22 @@ fn collect_implicit_stmts( } } } - Stmt::Break { .. } | Stmt::CallSubroutine { .. } | Stmt::Pass { .. } => {} + Stmt::Goto { offset, .. } => { + if let Some(offset) = offset { + collect_implicit_expr( + offset, + declared_globals, + declared_players, + globals, + players, + ); + } + } + Stmt::Break { .. } + | Stmt::Continue { .. } + | Stmt::Label { .. } + | Stmt::CallSubroutine { .. } + | Stmt::Pass { .. } => {} } } } diff --git a/crates/opy-rs/src/cst.rs b/crates/opy-rs/src/cst.rs index bbed747..e175e79 100644 --- a/crates/opy-rs/src/cst.rs +++ b/crates/opy-rs/src/cst.rs @@ -197,9 +197,25 @@ pub enum Stmt { arms: Vec, span: Span, }, + Delete { + target: Expr, + span: Span, + }, Break { span: Span, }, + Continue { + span: Span, + }, + Goto { + label: Option, + offset: Option, + span: Span, + }, + Label { + name: String, + span: Span, + }, Pass { span: Span, }, @@ -393,7 +409,11 @@ impl Stmt { | Stmt::While { span, .. } | Stmt::DoWhile { span, .. } | Stmt::Switch { span, .. } + | Stmt::Delete { span, .. } | Stmt::Break { span } + | Stmt::Continue { span } + | Stmt::Goto { span, .. } + | Stmt::Label { span, .. } | Stmt::Pass { span } => *span, } } diff --git a/crates/opy-rs/src/hir/dump.rs b/crates/opy-rs/src/hir/dump.rs index 00cbe1e..23531f2 100644 --- a/crates/opy-rs/src/hir/dump.rs +++ b/crates/opy-rs/src/hir/dump.rs @@ -311,6 +311,11 @@ fn dump_stmt(statement: &Stmt, out: &mut String, level: usize) { } } } + Stmt::Delete { target, span } => { + out.push_str(&format!("{}delete ", indent(level))); + render_expr(target, out); + out.push_str(&format!("{}\n", span_suffix(span.as_ref()))); + } Stmt::Break { span } => { out.push_str(&format!( "{}break{}\n", @@ -318,6 +323,35 @@ fn dump_stmt(statement: &Stmt, out: &mut String, level: usize) { span_suffix(span.as_ref()) )); } + Stmt::Continue { span } => { + out.push_str(&format!( + "{}continue{}\n", + indent(level), + span_suffix(span.as_ref()) + )); + } + Stmt::Goto { + label, + offset, + span, + } => { + out.push_str(&format!("{}goto ", indent(level))); + if let Some(label) = label { + out.push_str(&format!("label {label}")); + } else if let Some(offset) = offset { + out.push_str("loc+"); + render_expr(offset, out); + } + out.push_str(&format!("{}\n", span_suffix(span.as_ref()))); + } + Stmt::Label { name, span } => { + out.push_str(&format!( + "{}label {}{}\n", + indent(level), + name, + span_suffix(span.as_ref()) + )); + } Stmt::CallSubroutine { name, span } => { out.push_str(&format!( "{}callSubroutine {}{}\n", diff --git a/crates/opy-rs/src/hir/types.rs b/crates/opy-rs/src/hir/types.rs index 946e7a0..6922f69 100644 --- a/crates/opy-rs/src/hir/types.rs +++ b/crates/opy-rs/src/hir/types.rs @@ -457,10 +457,32 @@ pub enum Stmt { #[serde(skip_serializing_if = "Option::is_none")] span: Option, }, + Delete { + target: Box, + #[serde(skip_serializing_if = "Option::is_none")] + span: Option, + }, Break { #[serde(skip_serializing_if = "Option::is_none")] span: Option, }, + Continue { + #[serde(skip_serializing_if = "Option::is_none")] + span: Option, + }, + Goto { + #[serde(default, skip_serializing_if = "Option::is_none")] + label: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + offset: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + span: Option, + }, + Label { + name: String, + #[serde(skip_serializing_if = "Option::is_none")] + span: Option, + }, CallSubroutine { name: String, #[serde(skip_serializing_if = "Option::is_none")] @@ -491,7 +513,11 @@ impl Stmt { | Stmt::While { span, .. } | Stmt::DoWhile { span, .. } | Stmt::Switch { span, .. } + | Stmt::Delete { span, .. } | Stmt::Break { span } + | Stmt::Continue { span } + | Stmt::Goto { span, .. } + | Stmt::Label { span, .. } | Stmt::CallSubroutine { span, .. } | Stmt::Pass { span } => span.as_ref(), } diff --git a/crates/opy-rs/src/hir/validate.rs b/crates/opy-rs/src/hir/validate.rs index 65ba038..0a200a6 100644 --- a/crates/opy-rs/src/hir/validate.rs +++ b/crates/opy-rs/src/hir/validate.rs @@ -41,7 +41,11 @@ const STMT_KINDS: &[&str] = &[ "while", "doWhile", "switch", + "delete", "break", + "continue", + "goto", + "label", "callSubroutine", "pass", ]; @@ -467,6 +471,39 @@ fn validate_stmts( } } } + Stmt::Delete { target, span } => { + if !matches!(target.as_ref(), Expr::Index { .. }) { + errors.push(invalid( + "invalid-structure", + "a delete statement must target an array index", + *span, + )); + } + } + Stmt::Goto { + label, + offset, + span, + .. + } => { + if label.is_some() == offset.is_some() { + errors.push(invalid( + "invalid-structure", + "a goto must contain exactly one label or offset", + *span, + )); + } + if let Some(label) = label { + if let Err(error) = check_name(label, "label", *span) { + errors.push(error); + } + } + } + Stmt::Label { name, span } => { + if let Err(error) = check_name(name, "label", *span) { + errors.push(error); + } + } _ => {} } }); @@ -542,6 +579,7 @@ fn statement_exprs(statements: &[Stmt]) -> Vec<&Expr> { exprs.push(target.as_ref()); exprs.push(value.as_ref()); } + Stmt::Delete { target, .. } => exprs.push(target.as_ref()), Stmt::If { branches, r#else, .. } => { @@ -589,7 +627,16 @@ fn statement_exprs(statements: &[Stmt]) -> Vec<&Expr> { } } } - Stmt::Break { .. } | Stmt::CallSubroutine { .. } | Stmt::Pass { .. } => {} + Stmt::Goto { offset, .. } => { + if let Some(offset) = offset { + exprs.push(offset.as_ref()); + } + } + Stmt::Break { .. } + | Stmt::Continue { .. } + | Stmt::Label { .. } + | Stmt::CallSubroutine { .. } + | Stmt::Pass { .. } => {} } } exprs @@ -685,7 +732,11 @@ fn for_each_stmt<'a>(statements: &'a [Stmt], f: &mut impl FnMut(&'a Stmt)) { } Stmt::Expr { .. } | Stmt::Assign { .. } + | Stmt::Delete { .. } | Stmt::Break { .. } + | Stmt::Continue { .. } + | Stmt::Goto { .. } + | Stmt::Label { .. } | Stmt::CallSubroutine { .. } | Stmt::Pass { .. } => {} } @@ -851,6 +902,7 @@ fn check_stmt(value: &Value) -> Result<(), HirError> { "condition", "variable", "iterable", + "offset", ] { if let Some(child) = object.get(field) { check_expr(child)?; diff --git a/crates/opy-rs/src/lexer.rs b/crates/opy-rs/src/lexer.rs index 2591c80..99a6c9b 100644 --- a/crates/opy-rs/src/lexer.rs +++ b/crates/opy-rs/src/lexer.rs @@ -37,6 +37,7 @@ pub enum TokenKind { RBrace, Comma, Colon, + Semicolon, Dot, Assign, Plus, @@ -157,6 +158,7 @@ impl Lexer { '}' => self.single(TokenKind::RBrace), ',' => self.single(TokenKind::Comma), ':' => self.single(TokenKind::Colon), + ';' => self.single(TokenKind::Semicolon), '.' => self.single(TokenKind::Dot), '@' => self.single(TokenKind::At), '=' => self.two(TokenKind::Assign, TokenKind::Eq, '='), diff --git a/crates/opy-rs/src/lower.rs b/crates/opy-rs/src/lower.rs index a09e73c..8e354b1 100644 --- a/crates/opy-rs/src/lower.rs +++ b/crates/opy-rs/src/lower.rs @@ -223,7 +223,7 @@ pub fn lower_with_preprocessing( source_name: name.clone(), span: Some(span.into()), name_span: Some(name_span.into()), - body: lowerer.lower_block(body, &[], false, true), + body: lowerer.lower_block(body, &[], false, true, false), annotations: lower_annotations(annotations), }); } @@ -631,7 +631,7 @@ impl Lowerer { .iter() .map(|condition| self.lower_expr(condition, &[], CallPosition::Value)) .collect(); - let actions = self.lower_block(&rule.actions, &[], false, true); + let actions = self.lower_block(&rule.actions, &[], false, true, false); Ok(Rule { name: render_rule_name( &rule.name, @@ -669,6 +669,7 @@ impl Lowerer { macro_params: &[String], breakable: bool, allow_do_while: bool, + loopable: bool, ) -> Vec { stmts .iter() @@ -686,12 +687,18 @@ impl Lowerer { stmt.span(), ); } - self.lower_stmt(stmt, macro_params, breakable) + self.lower_stmt(stmt, macro_params, breakable, loopable) }) .collect() } - fn lower_stmt(&mut self, stmt: &Stmt, macro_params: &[String], breakable: bool) -> HirStmt { + fn lower_stmt( + &mut self, + stmt: &Stmt, + macro_params: &[String], + breakable: bool, + loopable: bool, + ) -> HirStmt { match stmt { Stmt::Expr { expr, span } => { // A bare call of a declared subroutine becomes @@ -733,12 +740,18 @@ impl Lowerer { macro_params, CallPosition::Value, )), - body: self.lower_block(&branch.body, macro_params, breakable, false), + body: self.lower_block( + &branch.body, + macro_params, + breakable, + false, + loopable, + ), }) .collect(), r#else: r#else .as_ref() - .map(|body| self.lower_block(body, macro_params, breakable, false)), + .map(|body| self.lower_block(body, macro_params, breakable, false, loopable)), span: Some(span.into()), }, Stmt::For { @@ -764,7 +777,7 @@ impl Lowerer { HirStmt::For { variable: Box::new(self.lower_for_binder(variable, macro_params)), iterable: Box::new(self.lower_expr(iterable, macro_params, iterable_position)), - body: self.lower_block(body, macro_params, true, false), + body: self.lower_block(body, macro_params, true, false, true), span: Some(span.into()), } } @@ -774,7 +787,7 @@ impl Lowerer { span, } => HirStmt::While { condition: Box::new(self.lower_expr(condition, macro_params, CallPosition::Value)), - body: self.lower_block(body, macro_params, true, false), + body: self.lower_block(body, macro_params, true, false, true), span: Some(span.into()), }, Stmt::DoWhile { @@ -783,7 +796,7 @@ impl Lowerer { span, } => HirStmt::DoWhile { condition: Box::new(self.lower_expr(condition, macro_params, CallPosition::Value)), - body: self.lower_block(body, macro_params, true, true), + body: self.lower_block(body, macro_params, true, true, true), span: Some(span.into()), }, Stmt::Switch { value, arms, span } => HirStmt::Switch { @@ -797,17 +810,21 @@ impl Lowerer { macro_params, CallPosition::Value, )), - body: self.lower_block(body, macro_params, true, false), + body: self.lower_block(body, macro_params, true, false, loopable), span: Some((*span).into()), }, cst::SwitchArm::Default { body, span } => HirSwitchArm::Default { - body: self.lower_block(body, macro_params, true, false), + body: self.lower_block(body, macro_params, true, false, loopable), span: Some((*span).into()), }, }) .collect(), span: Some(span.into()), }, + Stmt::Delete { target, span } => HirStmt::Delete { + target: Box::new(self.lower_expr(target, macro_params, CallPosition::Value)), + span: Some(span.into()), + }, Stmt::Break { span } => { if !breakable { self.error_at( @@ -820,6 +837,33 @@ impl Lowerer { span: Some(span.into()), } } + Stmt::Continue { span } => { + if !loopable { + self.error_at( + "continue-context", + "continue is only valid inside a loop".to_string(), + *span, + ); + } + HirStmt::Continue { + span: Some(span.into()), + } + } + Stmt::Goto { + label, + offset, + span, + } => HirStmt::Goto { + label: label.clone(), + offset: offset.as_ref().map(|offset| { + Box::new(self.lower_expr(offset, macro_params, CallPosition::Value)) + }), + span: Some(span.into()), + }, + Stmt::Label { name, span } => HirStmt::Label { + name: name.clone(), + span: Some(span.into()), + }, Stmt::Pass { span } => HirStmt::Pass { span: Some(span.into()), }, @@ -845,7 +889,7 @@ impl Lowerer { } fn lower_macro_body(&mut self, body: &[Stmt], params: &[String]) -> Vec { - self.lower_block(body, params, false, false) + self.lower_block(body, params, false, false, false) } fn lower_expr( diff --git a/crates/opy-rs/src/parser.rs b/crates/opy-rs/src/parser.rs index 3963034..413e9c9 100644 --- a/crates/opy-rs/src/parser.rs +++ b/crates/opy-rs/src/parser.rs @@ -189,6 +189,15 @@ impl Parser<'_> { } } + fn expect_statement_end(&mut self, what: &str) -> Result<(), ()> { + if matches!(self.peek_kind(), TokenKind::Newline | TokenKind::Eof) { + Ok(()) + } else { + self.error_at_current(format!("expected the end of {what}")); + Err(()) + } + } + // ---- declarations ---- fn parse_variable(&mut self, declarations: &mut Vec, global: bool) -> bool { @@ -890,6 +899,13 @@ impl Parser<'_> { "while" => return self.parse_while(), "do" => return self.parse_do_while(), "switch" => return self.parse_switch(), + "del" => return self.parse_delete(), + "continue" => { + let token = self.advance(); + self.expect_statement_end("the continue statement")?; + return Ok(Stmt::Continue { span: token.span }); + } + "goto" => return self.parse_goto(), "break" => { let token = self.advance(); return Ok(Stmt::Break { span: token.span }); @@ -900,10 +916,68 @@ impl Parser<'_> { } _ => {} } + if self.peek_at(1).kind == TokenKind::Colon { + return self.parse_label(); + } } self.parse_expr_statement() } + fn parse_delete(&mut self) -> Result { + let start = self.advance(); + let target = self.parse_postfix()?; + if !matches!(target, Expr::Index { .. }) { + self.errors.push(OpyError::at( + "parse-error", + "the del statement requires an array index target".to_string(), + target.span(), + )); + return Err(()); + } + self.expect_statement_end("the del statement")?; + Ok(Stmt::Delete { + span: Span::new(start.span.file, start.span.start, target.span().end), + target, + }) + } + + fn parse_goto(&mut self) -> Result { + let start = self.advance(); + if self.is_ident("loc") { + self.advance(); + self.expect(TokenKind::Plus, "'+' after 'goto loc'")?; + let offset = self.parse_expr()?; + self.expect_statement_end("the goto target")?; + return Ok(Stmt::Goto { + label: None, + span: Span::new(start.span.file, start.span.start, offset.span().end), + offset: Some(offset), + }); + } + + let label = self.expect_ident("a label or 'loc+...' after 'goto'")?; + self.expect_statement_end("the goto target")?; + Ok(Stmt::Goto { + label: Some(label), + offset: None, + span: Span::new( + start.span.file, + start.span.start, + self.tokens[self.pos - 1].span.end, + ), + }) + } + + fn parse_label(&mut self) -> Result { + let name = self.advance(); + let colon = self.expect(TokenKind::Colon, "':' after a label")?; + self.expect_statement_end("the label")?; + Ok(Stmt::Label { + name: name.text, + span: Span::new(name.span.file, name.span.start, colon.span.end), + }) + } + fn parse_expr_statement(&mut self) -> Result { let start = self.peek().span; let expr = self.parse_expr()?; @@ -949,6 +1023,26 @@ impl Parser<'_> { span: Span::new(start.file, start.start, end), }) } + TokenKind::Ident + if matches!(self.peek().text.as_str(), "min" | "max") + && self.peek_at(1).kind == TokenKind::Assign => + { + let op = self.advance().text; + self.advance(); + let rhs = self.parse_expr()?; + let end = self.peek().span.start; + let value = Expr::Binary { + op, + left: Box::new(expr.clone()), + right: Box::new(rhs), + span: Span::new(start.file, start.start, end), + }; + Ok(Stmt::Assign { + target: expr, + value, + span: Span::new(start.file, start.start, end), + }) + } TokenKind::Increment | TokenKind::Decrement => { let operator = self.advance(); if !matches!(self.peek_kind(), TokenKind::Newline | TokenKind::Eof) { @@ -2159,6 +2253,66 @@ mod tests { assert_eq!(body.len(), 2); } + #[test] + fn parses_issue_141_statement_surface() { + let program = parse_ok(concat!( + "globalvar value\n", + "rule \"r\":\n", + " @Event global\n", + " del value[1]\n", + " value min= 2\n", + " value max= 3\n", + " while value < 4:\n", + " continue\n", + " goto target\n", + " goto loc + value\n", + " target:\n", + )); + let RuleEntry::Rule(rule) = &program.rules[0] else { + panic!("expected rule"); + }; + assert!(matches!(rule.actions[0], Stmt::Delete { .. })); + for (statement, expected) in [(&rule.actions[1], "min"), (&rule.actions[2], "max")] { + let Stmt::Assign { value, .. } = statement else { + panic!("expected augmented assignment"); + }; + assert!(matches!(value, Expr::Binary { op, .. } if op == expected)); + } + let Stmt::While { body, .. } = &rule.actions[3] else { + panic!("expected while"); + }; + assert!(matches!(body.as_slice(), [Stmt::Continue { .. }])); + assert!( + matches!(&rule.actions[4], Stmt::Goto { label: Some(label), offset: None, .. } if label == "target") + ); + assert!(matches!( + &rule.actions[5], + Stmt::Goto { + label: None, + offset: Some(_), + .. + } + )); + assert!(matches!(&rule.actions[6], Stmt::Label { name, .. } if name == "target")); + } + + #[test] + fn rejects_invalid_issue_141_statement_forms() { + for source in [ + "rule \"r\":\n @Event global\n del value\n", + "rule \"r\":\n @Event global\n goto\n", + "rule \"r\":\n @Event global\n goto loc\n", + "rule \"r\":\n @Event global\n goto target extra\n", + "rule \"r\":\n @Event global\n continue now\n", + "rule \"r\":\n @Event global\n A = 1; A = 2\n", + ] { + let errors = parse_err(source); + assert!(!errors.is_empty(), "invalid form parsed: {source}"); + assert!(errors.iter().all(|error| error.code == "parse-error")); + assert!(errors.iter().all(|error| error.span.is_some())); + } + } + #[test] fn parses_issue_28_constructs() { let program = parse_ok( diff --git a/crates/opy-rs/src/support.rs b/crates/opy-rs/src/support.rs index 774e288..8490b14 100644 --- a/crates/opy-rs/src/support.rs +++ b/crates/opy-rs/src/support.rs @@ -214,7 +214,7 @@ mod tests { fn category_and_state_filters_match_the_summary() { let matrix = SupportMatrix::builtin().unwrap(); let syntax = matrix.features_by_category("syntax"); - assert_eq!(syntax.len(), 14); + assert_eq!(syntax.len(), 16); assert!(syntax.iter().all(|feature| feature.category == "syntax")); let lowering = matrix.features_by_state("lowering-dependent"); assert_eq!( diff --git a/crates/opy-rs/src/tooling.rs b/crates/opy-rs/src/tooling.rs index 34f39a4..22a60f2 100644 --- a/crates/opy-rs/src/tooling.rs +++ b/crates/opy-rs/src/tooling.rs @@ -633,6 +633,7 @@ impl SemanticModel { Self::collect_expr(target, sites); Self::collect_expr(value, sites); } + HirStmt::Delete { target, .. } => Self::collect_expr(target, sites), HirStmt::If { branches, r#else, .. } => { @@ -695,6 +696,12 @@ impl SemanticModel { } } HirStmt::Break { .. } => {} + HirStmt::Continue { .. } | HirStmt::Label { .. } => {} + HirStmt::Goto { offset, .. } => { + if let Some(offset) = offset { + Self::collect_expr(offset, sites); + } + } HirStmt::CallSubroutine { name, span } => { if let Some(span) = span { sites.push(( diff --git a/crates/opy-rs/support-matrix.json b/crates/opy-rs/support-matrix.json index 487f96b..dc85abd 100644 --- a/crates/opy-rs/support-matrix.json +++ b/crates/opy-rs/support-matrix.json @@ -116,6 +116,22 @@ "evidence": ["fixtures:synthetic/issue-33-switch-break", "upstream:src/tests/switches.opy", "upstream:src/tests/loops.opy"], "notes": "Issue #33: break is a real HIR statement, validates its enclosing switch/loop context, and is retained without implicit arm exits." }, + { + "id": "syntax/statement-surface", + "name": "del, continue, goto, labels, and dynamic loc+ targets", + "category": "syntax", + "state": "source-supported", + "evidence": ["tests:crates/opy-rs/tests/issue_141_syntax.rs", "upstream:src/tests/loops.opy", "upstream:src/tests/gotos.opy"], + "notes": "Issue #141: audited statements lower to source HIR nodes with spans; continue context and malformed targets produce structured diagnostics. Canonical WIR lowering remains an explicit integration boundary." + }, + { + "id": "syntax/augmented-min-max", + "name": "min= and max= modification forms", + "category": "syntax", + "state": "source-supported", + "evidence": ["tests:crates/opy-rs/tests/issue_141_syntax.rs", "upstream:src/tests/operators.opy"], + "notes": "Issue #141: min=/max= are retained as source-semantic binary modifications with provenance; Workshop support is not claimed." + }, { "id": "syntax/do-while", "name": "do … while", @@ -759,13 +775,13 @@ "summary": { "byState": { "planned": 0, - "source-supported": 23, + "source-supported": 25, "semantic-supported": 13, "lowering-dependent": 12, "end-to-end-supported": 8 }, "byCategory": { - "syntax": 14, + "syntax": 16, "semantics": 14, "preprocessing": 4, "macros": 3, diff --git a/crates/opy-rs/tests/issue_141_syntax.rs b/crates/opy-rs/tests/issue_141_syntax.rs new file mode 100644 index 0000000..a016f8f --- /dev/null +++ b/crates/opy-rs/tests/issue_141_syntax.rs @@ -0,0 +1,135 @@ +use std::path::Path; + +use opy_rs::hir::types::{Expr, Stmt}; + +#[test] +fn issue_141_surface_reaches_validated_hir_with_spans() { + let source = concat!( + "globalvar value\n", + "rule \"syntax surface\":\n", + " @Event global\n", + " del value[1]\n", + " value min= 2\n", + " value max= 3\n", + " while value < 4:\n", + " continue\n", + " goto target\n", + " goto loc + value\n", + " target:\n", + ); + let program = opy_rs::compile(source, "issue-141.opy", Path::new("")) + .expect("the issue-141 source surface must lower to HIR"); + program.validate().expect("the generated HIR must validate"); + + let rule = match &program.rules[0] { + opy_rs::hir::types::RuleEntry::Rule(rule) => rule, + other => panic!("expected rule, got {other:?}"), + }; + assert!(matches!( + rule.actions[0], + Stmt::Delete { span: Some(_), .. } + )); + assert!(matches!( + &rule.actions[1], + Stmt::Assign { + value, + span: Some(_), + .. + } if matches!(value.as_ref(), Expr::Binary { op, .. } if op == "min") + )); + assert!(matches!( + &rule.actions[2], + Stmt::Assign { + value, + span: Some(_), + .. + } if matches!(value.as_ref(), Expr::Binary { op, .. } if op == "max") + )); + let Stmt::While { + body, + span: Some(_), + .. + } = &rule.actions[3] + else { + panic!("expected while with a source span"); + }; + assert!(matches!( + body.as_slice(), + [Stmt::Continue { span: Some(_) }] + )); + assert!(matches!( + &rule.actions[4], + Stmt::Goto { + label: Some(_), + offset: None, + span: Some(_) + } + )); + assert!(matches!( + &rule.actions[5], + Stmt::Goto { + label: None, + offset: Some(_), + span: Some(_) + } + )); + assert!(matches!(&rule.actions[6], Stmt::Label { name, span: Some(_) } if name == "target")); +} + +#[test] +fn issue_141_invalid_statement_contexts_are_source_diagnostics() { + let cases = [ + ( + "rule \"invalid delete\":\n @Event global\n del value\n", + "parse-error", + ), + ( + "rule \"invalid goto\":\n @Event global\n goto loc\n", + "parse-error", + ), + ( + "rule \"outside continue\":\n @Event global\n continue\n", + "continue-context", + ), + ]; + for (source, expected_code) in cases { + let error = opy_rs::compile(source, "issue-141-invalid.opy", Path::new("")) + .expect_err("invalid issue-141 form unexpectedly compiled"); + assert_eq!(error.code, expected_code, "source: {source}"); + assert!( + error.span.is_some(), + "source diagnostic lost its span: {source}" + ); + } +} + +#[test] +fn issue_141_backend_boundary_is_explicit_for_source_only_statements() { + let cases = [ + ( + "rule \"delete\":\n @Event global\n del A[1]\n", + "delete statements", + ), + ( + "rule \"continue\":\n @Event global\n while A < 1:\n continue\n", + "continue statements", + ), + ( + "rule \"goto\":\n @Event global\n goto target\n", + "goto statements", + ), + ( + "rule \"label\":\n @Event global\n target:\n", + "labels", + ), + ]; + let compiler = opy_rs::Compiler::new().expect("the compiler contract loads"); + for (source, expected_text) in cases { + let error = compiler + .compile_source(source, "issue-141-backend.opy", Path::new("")) + .expect_err("source-only syntax must not be silently discarded"); + assert_eq!(error.diagnostic.code, "unsupported-integration-surface"); + assert!(error.diagnostic.message.contains(expected_text)); + assert!(error.diagnostic.span.is_some()); + } +} diff --git a/docs/hir/opy-hir-v2.md b/docs/hir/opy-hir-v2.md index 1105277..109c50a 100644 --- a/docs/hir/opy-hir-v2.md +++ b/docs/hir/opy-hir-v2.md @@ -51,6 +51,22 @@ The optional `member_span` field preserves the exact source span of the player variable member identifier (for example, `I` in `hostPlayer.I`); `span` continues to cover the complete member expression. +## Additive statement nodes + +The source frontend also retains the audited control-flow statements that are +not yet representable in canonical Workshop WIR: + +| Kind | Fields | Meaning | +| --- | --- | --- | +| `delete` | `target`, `span` | Delete an element addressed by an array index. | +| `continue` | `span` | Continue the innermost loop; source lowering rejects it outside a loop. | +| `goto` | `label`, `offset`, `span` | Jump to a named label or to a relative `loc+` offset; exactly one target field is present. | +| `label` | `name`, `span` | A named jump target. | + +These nodes are source-semantic and preserve provenance. The bounded compiler +reports an explicit integration diagnostic for them until canonical WIR owns +the corresponding Workshop control-flow semantics. + ## Consumer migration Every external `wright/opy-hir` consumer must migrate its protocol gate and diff --git a/docs/overpy-support/syntax-and-projects.md b/docs/overpy-support/syntax-and-projects.md index 48ad04a..315979a 100644 --- a/docs/overpy-support/syntax-and-projects.md +++ b/docs/overpy-support/syntax-and-projects.md @@ -21,7 +21,7 @@ Source: pinned OverPy `9.7.10`, content commit | List comprehensions | ✅ Supported | Mapping and filtering are separate behaviors. | | `lambda` with element/index binders | ✅ Supported | Valid positions are contextual. | | Member access, calls and postfix expressions | ✅ Supported | Receiver and dispatch checks are contract-sensitive. | -| `del` array element statement | 🚧 Coming soon | Audited upstream keyword; compilation support is incomplete. | +| `del` array element statement | 🚧 Coming soon | Source syntax and HIR are supported; canonical compilation remains incomplete. | | Conditional value `a if condition else b` | ✅ Supported | Chained forms are right-associative; distinct from statement `if`. | | `in` and `not in` membership | ✅ Supported | String containment uses `strContains`. | | Arithmetic, comparison, boolean and unary operators | ✅ Supported | Augmented forms are separate rows below. | @@ -35,7 +35,7 @@ Source: pinned OverPy `9.7.10`, content commit | Simple assignment `=` | ✅ Supported | Global, player and indexed forms differ at lowering. | | `+=`, `-=`, `*=`, `/=`, `%=` | ✅ Supported | Each spelling is independently audited; postfix `++`/`--` forms lower to canonical Add/Subtract modifications. | | `**=` augmented assignment | ✅ Supported | Separate from `**`; uses Raise To Power. | -| `min=` and `max=` modification forms | 🚧 Coming soon | Recognized by the audit; Workshop support is not claimed. | +| `min=` and `max=` modification forms | 🚧 Coming soon | Source syntax and HIR are supported; Workshop support is not claimed. | | `globalvar name [index]` | ✅ Supported | Explicit and implicit index forms are distinct. | | `playervar name [index]` | ✅ Supported | Explicit and implicit index forms are distinct. | | Variable initializer `globalvar/playervar name = value` | ✅ Supported | Constant-zero behavior is observable. | @@ -55,8 +55,8 @@ Source: pinned OverPy `9.7.10`, content commit | `while` and `do ... while` loops | ✅ Supported | Distinct entry-condition behavior. | | `switch` / `case` / `default` | ✅ Supported | Fall-through and `break` are separate. | | `break` in loops and switch arms | ✅ Supported | | -| `continue` in loops | 🚧 Coming soon | Upstream keyword exists; end-to-end support is incomplete. | -| `goto`, labels and dynamic `loc+` targets | 🚧 Coming soon | Audited from keyword registry and `src/tests/gotos.opy`. | +| `continue` in loops | 🚧 Coming soon | Source syntax and HIR are supported with loop-context diagnostics; canonical lowering is incomplete. | +| `goto`, labels and dynamic `loc+` targets | 🚧 Coming soon | Source syntax and HIR are supported; canonical lowering is incomplete. | | `pass` and `return` statements | ✅ Supported | Context restrictions remain conformance work. | | `#!include` root-relative composition | ✅ Supported | Missing files and cycles have distinct failures. | | Nested include closure and main-file selection | ✅ Supported | Project behavior is not inferred from one-file tests. | From b62ee8709652b9f5a26d9fed1a799d0460a5b195 Mon Sep 17 00:00:00 2001 From: Teakowa Date: Mon, 31 Aug 2026 20:51:49 +0800 Subject: [PATCH 2/2] fix(opy): preserve RULE_START goto semantics Keep OverPy's RULE_START form distinct from named labels in source HIR and make the support filter test follow its dynamic result. Refs #141 --- crates/opy-cli/tests/cli.rs | 9 +++++++- crates/opy-rs/src/compiler/mod.rs | 2 ++ crates/opy-rs/src/cst.rs | 1 + crates/opy-rs/src/hir/dump.rs | 5 ++++- crates/opy-rs/src/hir/types.rs | 2 ++ crates/opy-rs/src/hir/validate.rs | 7 ++++-- crates/opy-rs/src/lower.rs | 2 ++ crates/opy-rs/src/parser.rs | 30 ++++++++++++++++++++----- crates/opy-rs/tests/issue_141_syntax.rs | 24 ++++++++++++++++---- docs/hir/opy-hir-v2.md | 2 +- 10 files changed, 70 insertions(+), 14 deletions(-) diff --git a/crates/opy-cli/tests/cli.rs b/crates/opy-cli/tests/cli.rs index 27c90ab..4154206 100644 --- a/crates/opy-cli/tests/cli.rs +++ b/crates/opy-cli/tests/cli.rs @@ -267,7 +267,14 @@ fn support_filters_by_feature_id_and_category() { let slice: serde_json::Value = serde_json::from_slice(&by_category.stdout).expect("category JSON"); assert_eq!(slice["category"], "syntax"); - assert_eq!(slice["count"], 14); + let features = slice["features"].as_array().expect("filtered features"); + assert_eq!(slice["count"], features.len()); + assert!(!features.is_empty()); + assert!( + features + .iter() + .all(|feature| feature["category"] == "syntax") + ); let unknown = run(&["support", "nope/nothing"]); assert_eq!(unknown.status.code(), Some(2)); diff --git a/crates/opy-rs/src/compiler/mod.rs b/crates/opy-rs/src/compiler/mod.rs index 3c65700..ca5e8f7 100644 --- a/crates/opy-rs/src/compiler/mod.rs +++ b/crates/opy-rs/src/compiler/mod.rs @@ -826,6 +826,7 @@ impl MacroExpander { Stmt::Goto { label, offset, + rule_start, span, } => Stmt::Goto { label: label.clone(), @@ -833,6 +834,7 @@ impl MacroExpander { .as_ref() .map(|offset| self.expand_expr(offset, bindings).map(Box::new)) .transpose()?, + rule_start: *rule_start, span: *span, }, Stmt::Break { .. } | Stmt::CallSubroutine { .. } | Stmt::Pass { .. } => { diff --git a/crates/opy-rs/src/cst.rs b/crates/opy-rs/src/cst.rs index e175e79..a4c4f2d 100644 --- a/crates/opy-rs/src/cst.rs +++ b/crates/opy-rs/src/cst.rs @@ -210,6 +210,7 @@ pub enum Stmt { Goto { label: Option, offset: Option, + rule_start: bool, span: Span, }, Label { diff --git a/crates/opy-rs/src/hir/dump.rs b/crates/opy-rs/src/hir/dump.rs index 23531f2..60d508e 100644 --- a/crates/opy-rs/src/hir/dump.rs +++ b/crates/opy-rs/src/hir/dump.rs @@ -333,10 +333,13 @@ fn dump_stmt(statement: &Stmt, out: &mut String, level: usize) { Stmt::Goto { label, offset, + rule_start, span, } => { out.push_str(&format!("{}goto ", indent(level))); - if let Some(label) = label { + if *rule_start { + out.push_str("RULE_START"); + } else if let Some(label) = label { out.push_str(&format!("label {label}")); } else if let Some(offset) = offset { out.push_str("loc+"); diff --git a/crates/opy-rs/src/hir/types.rs b/crates/opy-rs/src/hir/types.rs index 6922f69..91e5b6c 100644 --- a/crates/opy-rs/src/hir/types.rs +++ b/crates/opy-rs/src/hir/types.rs @@ -475,6 +475,8 @@ pub enum Stmt { label: Option, #[serde(default, skip_serializing_if = "Option::is_none")] offset: Option>, + #[serde(default)] + rule_start: bool, #[serde(skip_serializing_if = "Option::is_none")] span: Option, }, diff --git a/crates/opy-rs/src/hir/validate.rs b/crates/opy-rs/src/hir/validate.rs index 0a200a6..761ba46 100644 --- a/crates/opy-rs/src/hir/validate.rs +++ b/crates/opy-rs/src/hir/validate.rs @@ -483,13 +483,16 @@ fn validate_stmts( Stmt::Goto { label, offset, + rule_start, span, .. } => { - if label.is_some() == offset.is_some() { + let target_count = + u8::from(label.is_some()) + u8::from(offset.is_some()) + u8::from(*rule_start); + if target_count != 1 { errors.push(invalid( "invalid-structure", - "a goto must contain exactly one label or offset", + "a goto must contain exactly one label, offset, or RULE_START target", *span, )); } diff --git a/crates/opy-rs/src/lower.rs b/crates/opy-rs/src/lower.rs index 8e354b1..5dd0739 100644 --- a/crates/opy-rs/src/lower.rs +++ b/crates/opy-rs/src/lower.rs @@ -852,12 +852,14 @@ impl Lowerer { Stmt::Goto { label, offset, + rule_start, span, } => HirStmt::Goto { label: label.clone(), offset: offset.as_ref().map(|offset| { Box::new(self.lower_expr(offset, macro_params, CallPosition::Value)) }), + rule_start: *rule_start, span: Some(span.into()), }, Stmt::Label { name, span } => HirStmt::Label { diff --git a/crates/opy-rs/src/parser.rs b/crates/opy-rs/src/parser.rs index 413e9c9..ad79912 100644 --- a/crates/opy-rs/src/parser.rs +++ b/crates/opy-rs/src/parser.rs @@ -952,14 +952,17 @@ impl Parser<'_> { label: None, span: Span::new(start.span.file, start.span.start, offset.span().end), offset: Some(offset), + rule_start: false, }); } let label = self.expect_ident("a label or 'loc+...' after 'goto'")?; self.expect_statement_end("the goto target")?; + let rule_start = label == "RULE_START"; Ok(Stmt::Goto { - label: Some(label), + label: (!rule_start).then_some(label), offset: None, + rule_start, span: Span::new( start.span.file, start.span.start, @@ -2264,6 +2267,7 @@ mod tests { " value max= 3\n", " while value < 4:\n", " continue\n", + " goto RULE_START\n", " goto target\n", " goto loc + value\n", " target:\n", @@ -2282,18 +2286,34 @@ mod tests { panic!("expected while"); }; assert!(matches!(body.as_slice(), [Stmt::Continue { .. }])); - assert!( - matches!(&rule.actions[4], Stmt::Goto { label: Some(label), offset: None, .. } if label == "target") - ); + assert!(matches!( + &rule.actions[4], + Stmt::Goto { + label: None, + offset: None, + rule_start: true, + .. + } + )); assert!(matches!( &rule.actions[5], + Stmt::Goto { + label: Some(label), + offset: None, + rule_start: false, + .. + } if label == "target" + )); + assert!(matches!( + &rule.actions[6], Stmt::Goto { label: None, offset: Some(_), + rule_start: false, .. } )); - assert!(matches!(&rule.actions[6], Stmt::Label { name, .. } if name == "target")); + assert!(matches!(&rule.actions[7], Stmt::Label { name, .. } if name == "target")); } #[test] diff --git a/crates/opy-rs/tests/issue_141_syntax.rs b/crates/opy-rs/tests/issue_141_syntax.rs index a016f8f..2633fdc 100644 --- a/crates/opy-rs/tests/issue_141_syntax.rs +++ b/crates/opy-rs/tests/issue_141_syntax.rs @@ -13,6 +13,7 @@ fn issue_141_surface_reaches_validated_hir_with_spans() { " value max= 3\n", " while value < 4:\n", " continue\n", + " goto RULE_START\n", " goto target\n", " goto loc + value\n", " target:\n", @@ -60,20 +61,31 @@ fn issue_141_surface_reaches_validated_hir_with_spans() { assert!(matches!( &rule.actions[4], Stmt::Goto { - label: Some(_), + label: None, offset: None, - span: Some(_) + rule_start: true, + span: Some(_), } )); assert!(matches!( &rule.actions[5], + Stmt::Goto { + label: Some(_), + offset: None, + rule_start: false, + span: Some(_), + } + )); + assert!(matches!( + &rule.actions[6], Stmt::Goto { label: None, offset: Some(_), - span: Some(_) + rule_start: false, + span: Some(_), } )); - assert!(matches!(&rule.actions[6], Stmt::Label { name, span: Some(_) } if name == "target")); + assert!(matches!(&rule.actions[7], Stmt::Label { name, span: Some(_) } if name == "target")); } #[test] @@ -118,6 +130,10 @@ fn issue_141_backend_boundary_is_explicit_for_source_only_statements() { "rule \"goto\":\n @Event global\n goto target\n", "goto statements", ), + ( + "rule \"goto rule start\":\n @Event global\n goto RULE_START\n", + "goto statements", + ), ( "rule \"label\":\n @Event global\n target:\n", "labels", diff --git a/docs/hir/opy-hir-v2.md b/docs/hir/opy-hir-v2.md index 109c50a..f7d36f2 100644 --- a/docs/hir/opy-hir-v2.md +++ b/docs/hir/opy-hir-v2.md @@ -60,7 +60,7 @@ not yet representable in canonical Workshop WIR: | --- | --- | --- | | `delete` | `target`, `span` | Delete an element addressed by an array index. | | `continue` | `span` | Continue the innermost loop; source lowering rejects it outside a loop. | -| `goto` | `label`, `offset`, `span` | Jump to a named label or to a relative `loc+` offset; exactly one target field is present. | +| `goto` | `label`, `offset`, `ruleStart`, `span` | Jump to a named label, to a relative `loc+` offset, or to the enclosing rule loop with `RULE_START`; exactly one target field is present. | | `label` | `name`, `span` | A named jump target. | These nodes are source-semantic and preserve provenance. The bounded compiler