From 9b1676655bec40abe8050c7f4e3ba5c6fb6981b9 Mon Sep 17 00:00:00 2001 From: Teakowa Date: Mon, 31 Aug 2026 23:37:22 +0800 Subject: [PATCH 1/2] feat(opy): complete semantic HIR subroutine resolution Materialize implicit subroutine declarations for def definitions, enforce source-order visibility, and expose stable diagnostics and tooling provenance. Fixes #143 --- compatibility/compiler-expectations.json | 11 +- crates/opy-rs/src/lower.rs | 201 +++++++++++++++--- crates/opy-rs/tests/issue_143_semantic_hir.rs | 80 +++++++ crates/opy-rs/tests/tooling.rs | 20 +- docs/opy/tooling-api.md | 9 +- 5 files changed, 275 insertions(+), 46 deletions(-) create mode 100644 crates/opy-rs/tests/issue_143_semantic_hir.rs diff --git a/compatibility/compiler-expectations.json b/compatibility/compiler-expectations.json index 63259a9..26f9f37 100644 --- a/compatibility/compiler-expectations.json +++ b/compatibility/compiler-expectations.json @@ -766,17 +766,16 @@ }, { "fixture": "synthetic/issue-31-positive", - "nativeStatus": "failure", + "nativeStatus": "success", "classification": "known-gap", - "comparison": "diagnostic-code", + "comparison": "semantic-wir", "evidence": [ "oracle:synthetic/issue-31-positive/oracle.json", "provenance:synthetic/issue-31-positive/fixture.json" ], - "owner": "opy-rs#88", - "note": "The positive optimizer/directive fixture exceeds the independently evidenced compiler baseline.", - "failureClass": "integration", - "diagnosticCode": "unsupported-integration-surface" + "owner": "opy-rs#145", + "note": "The semantic HIR now materializes the implicit def subroutine declaration; optimizer/directive compiler parity remains a separate lowering gap.", + "semanticEquivalent": false }, { "fixture": "synthetic/issue-33-f-string", diff --git a/crates/opy-rs/src/lower.rs b/crates/opy-rs/src/lower.rs index 5dd0739..5f1c3eb 100644 --- a/crates/opy-rs/src/lower.rs +++ b/crates/opy-rs/src/lower.rs @@ -61,12 +61,15 @@ enum CallPosition { /// The lowerer's symbol context, built from the CST declarations. struct Lowerer { - globals: HashSet, - players: HashSet, - subroutines: HashSet, - macros: HashSet, + global_declarations: HashMap, + player_declarations: HashMap, + subroutine_declarations: HashMap, + subroutine_definitions: Vec<(String, usize)>, + macro_declarations: HashMap, enums: HashMap>, + enum_declarations: HashMap, locals: Vec, + current_order: usize, allow_dict_literal: bool, /// The authoritative builtin semantic table (issue #109). manifest: &'static Manifest, @@ -109,12 +112,15 @@ pub fn lower_with_preprocessing( } }; let mut lowerer = Lowerer { - globals: HashSet::new(), - players: HashSet::new(), - subroutines: HashSet::new(), - macros: HashSet::new(), + global_declarations: HashMap::new(), + player_declarations: HashMap::new(), + subroutine_declarations: HashMap::new(), + subroutine_definitions: Vec::new(), + macro_declarations: HashMap::new(), enums: HashMap::new(), + enum_declarations: HashMap::new(), locals: Vec::new(), + current_order: 0, allow_dict_literal: false, manifest, catalog, @@ -123,7 +129,8 @@ pub fn lower_with_preprocessing( lowerer.collect_symbols(program); let mut declarations = Vec::new(); - for decl in &program.declarations { + for (order, decl) in program.declarations.iter().enumerate() { + lowerer.current_order = order; match decl { Decl::GlobalVariable { name, @@ -188,8 +195,31 @@ pub fn lower_with_preprocessing( } } - let mut rules = Vec::new(); + let mut implicit_subroutines = HashSet::new(); for entry in &program.rules { + let CstRuleEntry::SubroutineDef { + name, + span, + name_span, + .. + } = entry + else { + continue; + }; + if !lowerer.subroutine_declarations.contains_key(name) + && implicit_subroutines.insert(name.clone()) + { + declarations.push(Declaration::Subroutine { + name: name.clone(), + index: None, + span: Some(span.into()), + name_span: Some(name_span.into()), + }); + } + } + let mut rules = Vec::new(); + for (rule_order, entry) in program.rules.iter().enumerate() { + lowerer.current_order = program.declarations.len() + rule_order; match entry { CstRuleEntry::Rule(rule) => rules.push(RuleEntry::Rule(lowerer.lower_rule( rule, @@ -584,28 +614,110 @@ fn lower_settings_node(node: &cst::SettingsNode) -> HirSettingsNode { impl Lowerer { fn collect_symbols(&mut self, program: &cst::Program) { - for decl in &program.declarations { + for (order, decl) in program.declarations.iter().enumerate() { match decl { - Decl::GlobalVariable { name, .. } => { - self.globals.insert(name.clone()); + Decl::GlobalVariable { name, span, .. } => { + if self + .global_declarations + .insert(name.clone(), order) + .is_some() + { + self.error_at( + "duplicate-declaration", + format!("duplicate global variable '{name}'"), + *span, + ); + } } - Decl::PlayerVariable { name, .. } => { - self.players.insert(name.clone()); + Decl::PlayerVariable { name, span, .. } => { + if self + .player_declarations + .insert(name.clone(), order) + .is_some() + { + self.error_at( + "duplicate-declaration", + format!("duplicate player variable '{name}'"), + *span, + ); + } } - Decl::Subroutine { name, .. } => { - self.subroutines.insert(name.clone()); + Decl::Subroutine { name, span, .. } => { + if self + .subroutine_declarations + .insert(name.clone(), order) + .is_some() + { + self.error_at( + "duplicate-declaration", + format!("duplicate subroutine '{name}'"), + *span, + ); + } } Decl::Enum { name, members, .. } => { - self.enums.insert( - name.clone(), - members.iter().map(|(member, _)| member.clone()).collect(), - ); + self.enum_declarations.entry(name.clone()).or_insert(order); + self.enums.entry(name.clone()).or_insert_with(|| { + members.iter().map(|(member, _)| member.clone()).collect() + }); } Decl::Macro { name, .. } => { - self.macros.insert(name.clone()); + self.macro_declarations.entry(name.clone()).or_insert(order); } } } + for (rule_order, entry) in program.rules.iter().enumerate() { + let CstRuleEntry::SubroutineDef { name, span, .. } = entry else { + continue; + }; + if self + .subroutine_definitions + .iter() + .any(|(defined, _)| defined == name) + { + self.error_at( + "duplicate-definition", + format!("duplicate subroutine definition '{name}'"), + *span, + ); + } + self.subroutine_definitions + .push((name.clone(), program.declarations.len() + rule_order)); + } + } + + fn subroutine_visible(&self, name: &str) -> bool { + self.subroutine_declarations + .get(name) + .is_some_and(|order| *order <= self.current_order) + || self + .subroutine_definitions + .iter() + .any(|(definition, order)| definition == name && *order <= self.current_order) + } + + fn global_visible(&self, name: &str) -> bool { + self.global_declarations + .get(name) + .is_some_and(|order| *order <= self.current_order) + } + + fn player_visible(&self, name: &str) -> bool { + self.player_declarations + .get(name) + .is_some_and(|order| *order <= self.current_order) + } + + fn macro_visible(&self, name: &str) -> bool { + self.macro_declarations + .get(name) + .is_some_and(|order| *order <= self.current_order) + } + + fn enum_visible(&self, name: &str) -> bool { + self.enum_declarations + .get(name) + .is_some_and(|order| *order <= self.current_order) } /// A declaration initializer: integer-`0` literal initializers are @@ -704,7 +816,7 @@ impl Lowerer { // A bare call of a declared subroutine becomes // `CallSubroutine` (reference behavior). if let Expr::Call { name, args, .. } = expr { - if self.subroutines.contains(name) && args.is_empty() { + if self.subroutine_visible(name) && args.is_empty() { return HirStmt::CallSubroutine { name: name.clone(), span: Some(span.into()), @@ -880,6 +992,14 @@ impl Lowerer { span, } = variable { + if !default_var_index(member).is_some() && !self.player_visible(member) { + self.error_at( + "unknown-identifier", + format!("unknown player variable '{member}'"), + *member_span, + ); + return HirExpr::Null { span: None }; + } return HirExpr::PlayerVar { player: Box::new(self.lower_expr(receiver, macro_params, CallPosition::Value)), name: member.clone(), @@ -1134,17 +1254,17 @@ impl Lowerer { "hostPlayer" => HirExpr::HostPlayer { span: Some(span.into()), }, - _ if self.globals.contains(name) => HirExpr::GlobalVar { + _ if self.global_visible(name) => HirExpr::GlobalVar { name: name.to_string(), span: Some(span.into()), }, - _ if self.players.contains(name) => HirExpr::PlayerVar { + _ if self.player_visible(name) => HirExpr::PlayerVar { player: Box::new(HirExpr::EventPlayer { span: None }), name: name.to_string(), member_span: None, span: Some(span.into()), }, - _ if self.enums.contains_key(name) => { + _ if self.enum_visible(name) => { self.error_at( "enum-type-without-member", format!("enum type '{name}' must be used with a member (e.g. {name}.MEMBER)"), @@ -1183,7 +1303,8 @@ impl Lowerer { ) -> HirExpr { if let Expr::Name { name, .. } = receiver { // Custom enum member: folds to its numeric constant. - if let Some(members) = self.enums.get(name) { + if self.enum_visible(name) { + let members = self.enums.get(name).expect("enum span and members agree"); return match members.iter().position(|candidate| candidate == member) { Some(index) => HirExpr::Number { value: index as f64, @@ -1246,6 +1367,14 @@ impl Lowerer { } // Event-player member: a player-variable reference. if name == "eventPlayer" { + if !default_var_index(member).is_some() && !self.player_visible(member) { + self.error_at( + "unknown-identifier", + format!("unknown player variable '{member}'"), + member_span, + ); + return HirExpr::Null { span: None }; + } return HirExpr::PlayerVar { player: Box::new(HirExpr::EventPlayer { span: None }), name: member.to_string(), @@ -1254,6 +1383,14 @@ impl Lowerer { }; } if name == "hostPlayer" { + if !default_var_index(member).is_some() && !self.player_visible(member) { + self.error_at( + "unknown-identifier", + format!("unknown player variable '{member}'"), + member_span, + ); + return HirExpr::Null { span: None }; + } return HirExpr::PlayerVar { player: Box::new(HirExpr::HostPlayer { span: None }), name: member.to_string(), @@ -1274,8 +1411,8 @@ impl Lowerer { // when canonical member existence is deferred to Workshop. Keep // both the resolved variable receiver and the source member // identity in HIR instead of treating it as an unknown member. - if self.globals.contains(name) - || self.players.contains(name) + if self.global_visible(name) + || self.player_visible(name) || default_var_index(name).is_some() { let receiver = if default_var_index(name).is_some() { @@ -1315,7 +1452,7 @@ impl Lowerer { } // Builtin identity and position checks run before the special forms // so that a misplaced `wait`/`vect` still diagnoses its position. - if !self.macros.contains(name) && !self.subroutines.contains(name) && name != "sorted" { + if !self.macro_visible(name) && !self.subroutine_visible(name) && name != "sorted" { match self.manifest.resolve_function(name) { Some(entry) => self.check_call_position(name, entry, position, span), None => { @@ -1372,7 +1509,7 @@ impl Lowerer { } } _ => { - if self.macros.contains(name) { + if self.macro_visible(name) { // A declared `macro` invocation is recorded as a macroCall // (positional-only; keyword arguments are an explicit // diagnostic). @@ -1399,7 +1536,7 @@ impl Lowerer { // Declared subroutines with arguments stay generic // calls; builtins get keyword binding, arity, and // domain/default handling. - if self.subroutines.contains(name) { + if self.subroutine_visible(name) { return HirExpr::Call { name: name.to_string(), args: self.lower_arg_values(args, macro_params), diff --git a/crates/opy-rs/tests/issue_143_semantic_hir.rs b/crates/opy-rs/tests/issue_143_semantic_hir.rs new file mode 100644 index 0000000..f778a69 --- /dev/null +++ b/crates/opy-rs/tests/issue_143_semantic_hir.rs @@ -0,0 +1,80 @@ +use std::path::Path; + +use opy_rs::hir::types::{Declaration, RuleEntry, Stmt}; +use opy_rs::tooling::{SymbolKind, check}; + +#[test] +fn def_materializes_a_tooling_usable_subroutine_declaration() { + let source = + "def worker():\n pass\n\nrule \"call worker\":\n @Event global\n worker()\n"; + let outcome = check(source, "main.opy", Path::new("")); + assert!( + outcome.is_clean(), + "def-only call must resolve: {:?}", + outcome.diagnostics + ); + let model = outcome.model.expect("clean semantic model"); + + assert!(model.declarations().iter().any(|declaration| matches!( + declaration, + Declaration::Subroutine { name, name_span, .. } + if name == "worker" && name_span.is_some() + ))); + assert!(model.rules().iter().any(|entry| matches!( + entry, + RuleEntry::Rule(rule) + if rule.actions.iter().any(|statement| matches!( + statement, + Stmt::CallSubroutine { name, .. } if name == "worker" + )) + ))); + + let worker_symbols: Vec<_> = model + .symbols() + .iter() + .filter(|symbol| symbol.name == "worker") + .collect(); + assert_eq!(worker_symbols.len(), 2); + assert!( + worker_symbols + .iter() + .all(|symbol| !symbol.references.is_empty()) + ); + assert!( + worker_symbols + .iter() + .any(|symbol| symbol.kind == SymbolKind::Subroutine) + ); + assert!( + worker_symbols + .iter() + .any(|symbol| symbol.kind == SymbolKind::Def) + ); +} + +#[test] +fn subroutine_visibility_follows_source_order_and_rejects_duplicate_defs() { + let forward_call = check( + "rule \"call worker\":\n @Event global\n worker()\n\ndef worker():\n pass\n", + "main.opy", + Path::new(""), + ); + let diagnostic = forward_call + .diagnostics + .first() + .expect("forward call diagnostic"); + assert_eq!(diagnostic.code, "unknown-action"); + assert_eq!(diagnostic.span.as_ref().expect("source span").start.line, 3); + + let duplicate = check( + "def worker():\n pass\n\ndef worker():\n pass\n", + "main.opy", + Path::new(""), + ); + let diagnostic = duplicate + .diagnostics + .first() + .expect("duplicate definition diagnostic"); + assert_eq!(diagnostic.code, "duplicate-definition"); + assert_eq!(diagnostic.span.as_ref().expect("source span").start.line, 4); +} diff --git a/crates/opy-rs/tests/tooling.rs b/crates/opy-rs/tests/tooling.rs index be0e97a..14909a3 100644 --- a/crates/opy-rs/tests/tooling.rs +++ b/crates/opy-rs/tests/tooling.rs @@ -38,7 +38,7 @@ fn multi_file_project_checks_and_resolves_end_to_end() { // Declarations across both files: globalvar/subroutine/macro from the // include, playervar from the main file (enums are not retained in the // Opy HIR; they are queried separately). - assert_eq!(model.declarations().len(), 4); + assert_eq!(model.declarations().len(), 5); assert!(model .declarations() .iter() @@ -90,9 +90,21 @@ fn multi_file_project_checks_and_resolves_end_to_end() { .kind, SymbolKind::Macro ); - assert_eq!( - model.symbol("finish").expect("def binding").kind, - SymbolKind::Def + let finish_symbols: Vec<_> = model + .symbols() + .iter() + .filter(|symbol| symbol.name == "finish") + .collect(); + assert_eq!(finish_symbols.len(), 2); + assert!( + finish_symbols + .iter() + .any(|symbol| symbol.kind == SymbolKind::Subroutine) + ); + assert!( + finish_symbols + .iter() + .any(|symbol| symbol.kind == SymbolKind::Def) ); // References: uses in the main file and in the def body resolve to the diff --git a/docs/opy/tooling-api.md b/docs/opy/tooling-api.md index 978ab6d..343c978 100644 --- a/docs/opy/tooling-api.md +++ b/docs/opy/tooling-api.md @@ -128,6 +128,7 @@ Span layout: `file_id` indexes the registry, positions are 1-based | `manifest-error` | resolve | Semantic manifest load failure | | `unknown-identifier` / `enum-type-without-member` | resolve | Unresolved names | | `unknown-action` / `unknown-value` / `unknown-member` | resolve | Unknown builtins | +| `duplicate-declaration` / `duplicate-definition` | resolve | Duplicate OPY declarations or `def` definitions | | `unsupported-member` | resolve | Member access outside the declared surface | | `unknown-enum-member` | resolve | Custom (user-declared) enum member validation | | `invalid-arity` / `missing-argument` / `invalid-argument` | resolve | Signature validation | @@ -210,10 +211,10 @@ stdout. ## Known limitations -* `def NAME():` bodies resolve, but calls resolve only against `subroutine - NAME` declarations; a def-only subroutine call is an `unknown-action` - diagnostic (existing source implementation resolution contract; tracked as source implementation - follow-up). +* `def NAME():` materializes an implicit `subroutine NAME` declaration in the + HIR. The declaration and definition are separate tooling symbols, and calls + resolve according to source order; a call before the declaration or + definition receives an `unknown-action` diagnostic. * Custom enums fold to constants in the HIR (reference behavior); enum declarations are queryable through `SemanticModel::enums`, not the HIR declaration list. From c113d6fa4643f30c414a1495b018ecb3535dff9f Mon Sep 17 00:00:00 2001 From: Teakowa Date: Tue, 1 Sep 2026 00:22:02 +0800 Subject: [PATCH 2/2] fix(opy): preserve top-level semantic order Keep declaration visibility aligned with interleaved source forms and retain first declaration order when reporting duplicates. --- crates/opy-rs/src/cst.rs | 11 + crates/opy-rs/src/lower.rs | 189 ++++++++---------- crates/opy-rs/src/parser.rs | 20 +- crates/opy-rs/tests/issue_143_semantic_hir.rs | 39 ++++ 4 files changed, 153 insertions(+), 106 deletions(-) diff --git a/crates/opy-rs/src/cst.rs b/crates/opy-rs/src/cst.rs index a4c4f2d..4ebc037 100644 --- a/crates/opy-rs/src/cst.rs +++ b/crates/opy-rs/src/cst.rs @@ -13,10 +13,21 @@ use crate::diag::Span; pub struct Program { pub declarations: Vec, pub rules: Vec, + /// All top-level forms in source order. The category-specific vectors are + /// retained for the HIR-shaped parser API, while lowering uses this list + /// for scope and visibility decisions. + pub top_level: Vec, /// The parsed top-of-file `settings { ... }` block, when present (#86). pub settings: Option, } +/// A top-level declaration or rule entry in source order. +#[derive(Debug, Clone)] +pub enum TopLevel { + Declaration(Decl), + Rule(RuleEntry), +} + /// A parsed `settings { ... }` block (JSONC, #86). #[derive(Debug, Clone)] pub struct Settings { diff --git a/crates/opy-rs/src/lower.rs b/crates/opy-rs/src/lower.rs index 5f1c3eb..901f8a7 100644 --- a/crates/opy-rs/src/lower.rs +++ b/crates/opy-rs/src/lower.rs @@ -35,7 +35,7 @@ use crate::hir::types::{ Stmt as HirStmt, SwitchArm as HirSwitchArm, default_var_index, }; -use crate::cst::{self, CallArg, Decl, Expr, RuleEntry as CstRuleEntry, Stmt}; +use crate::cst::{self, CallArg, Decl, Expr, RuleEntry as CstRuleEntry, Stmt, TopLevel}; use crate::diag::{OpyError, OpyResult, Span}; use crate::manifest::{ Function, FunctionContext, FunctionKind, Manifest, Param, ParamDefault, ReceiverCategory, @@ -129,104 +129,71 @@ pub fn lower_with_preprocessing( lowerer.collect_symbols(program); let mut declarations = Vec::new(); - for (order, decl) in program.declarations.iter().enumerate() { + let mut rules = Vec::new(); + let mut implicit_subroutines = HashSet::new(); + for (order, item) in program.top_level.iter().enumerate() { lowerer.current_order = order; - match decl { - Decl::GlobalVariable { - name, - index, - span, - name_span, - initializer, - } => { - declarations.push(Declaration::GlobalVariable { + match item { + TopLevel::Declaration(decl) => match decl { + Decl::GlobalVariable { + name, + index, + span, + name_span, + initializer, + } => declarations.push(Declaration::GlobalVariable { name: name.clone(), index: *index, span: Some(span.into()), name_span: Some(name_span.into()), initializer: lowerer.initializer(initializer.as_ref()), - }); - } - Decl::PlayerVariable { - name, - index, - span, - name_span, - initializer, - } => { - declarations.push(Declaration::PlayerVariable { + }), + Decl::PlayerVariable { + name, + index, + span, + name_span, + initializer, + } => declarations.push(Declaration::PlayerVariable { name: name.clone(), index: *index, span: Some(span.into()), name_span: Some(name_span.into()), initializer: lowerer.initializer(initializer.as_ref()), - }); - } - Decl::Subroutine { - name, - span, - name_span, - } => { - declarations.push(Declaration::Subroutine { + }), + Decl::Subroutine { + name, + span, + name_span, + } => declarations.push(Declaration::Subroutine { name: name.clone(), index: None, span: Some(span.into()), name_span: Some(name_span.into()), - }); - } - Decl::Enum { .. } => { - // Custom enums fold to numeric constants at use sites and - // produce no HIR declaration (reference behavior). - } - Decl::Macro { - name, - args, - body, - span, - } => { - let lowered_body = lowerer.lower_macro_body(body, args); - declarations.push(Declaration::Macro { - name: name.clone(), - args: args.clone(), - span: Some(span.into()), - body: lowered_body, - }); - } - } - } - - let mut implicit_subroutines = HashSet::new(); - for entry in &program.rules { - let CstRuleEntry::SubroutineDef { - name, - span, - name_span, - .. - } = entry - else { - continue; - }; - if !lowerer.subroutine_declarations.contains_key(name) - && implicit_subroutines.insert(name.clone()) - { - declarations.push(Declaration::Subroutine { - name: name.clone(), - index: None, - span: Some(span.into()), - name_span: Some(name_span.into()), - }); - } - } - let mut rules = Vec::new(); - for (rule_order, entry) in program.rules.iter().enumerate() { - lowerer.current_order = program.declarations.len() + rule_order; - match entry { - CstRuleEntry::Rule(rule) => rules.push(RuleEntry::Rule(lowerer.lower_rule( - rule, - files.as_slice(), - preprocessing, - )?)), - CstRuleEntry::SubroutineDef { + }), + Decl::Enum { .. } => { + // Custom enums fold to numeric constants at use sites and + // produce no HIR declaration (reference behavior). + } + Decl::Macro { + name, + args, + body, + span, + } => { + let lowered_body = lowerer.lower_macro_body(body, args); + declarations.push(Declaration::Macro { + name: name.clone(), + args: args.clone(), + span: Some(span.into()), + body: lowered_body, + }); + } + }, + TopLevel::Rule(CstRuleEntry::Rule(rule)) => rules.push(RuleEntry::Rule( + lowerer.lower_rule(rule, files.as_slice(), preprocessing)?, + )), + TopLevel::Rule(CstRuleEntry::SubroutineDef { name, presentation_name, span, @@ -234,7 +201,17 @@ pub fn lower_with_preprocessing( body, annotations, rule_prefix, - } => { + }) => { + if !lowerer.subroutine_declarations.contains_key(name) + && implicit_subroutines.insert(name.clone()) + { + declarations.push(Declaration::Subroutine { + name: name.clone(), + index: None, + span: Some(span.into()), + name_span: Some(name_span.into()), + }); + } let base_name = presentation_name .as_deref() .map(str::to_string) @@ -614,14 +591,17 @@ fn lower_settings_node(node: &cst::SettingsNode) -> HirSettingsNode { impl Lowerer { fn collect_symbols(&mut self, program: &cst::Program) { - for (order, decl) in program.declarations.iter().enumerate() { + for (order, item) in program.top_level.iter().enumerate() { + let TopLevel::Declaration(decl) = item else { + continue; + }; match decl { Decl::GlobalVariable { name, span, .. } => { - if self - .global_declarations - .insert(name.clone(), order) - .is_some() - { + let duplicate = self.global_declarations.contains_key(name); + self.global_declarations + .entry(name.clone()) + .or_insert(order); + if duplicate { self.error_at( "duplicate-declaration", format!("duplicate global variable '{name}'"), @@ -630,11 +610,11 @@ impl Lowerer { } } Decl::PlayerVariable { name, span, .. } => { - if self - .player_declarations - .insert(name.clone(), order) - .is_some() - { + let duplicate = self.player_declarations.contains_key(name); + self.player_declarations + .entry(name.clone()) + .or_insert(order); + if duplicate { self.error_at( "duplicate-declaration", format!("duplicate player variable '{name}'"), @@ -643,11 +623,11 @@ impl Lowerer { } } Decl::Subroutine { name, span, .. } => { - if self - .subroutine_declarations - .insert(name.clone(), order) - .is_some() - { + let duplicate = self.subroutine_declarations.contains_key(name); + self.subroutine_declarations + .entry(name.clone()) + .or_insert(order); + if duplicate { self.error_at( "duplicate-declaration", format!("duplicate subroutine '{name}'"), @@ -666,8 +646,8 @@ impl Lowerer { } } } - for (rule_order, entry) in program.rules.iter().enumerate() { - let CstRuleEntry::SubroutineDef { name, span, .. } = entry else { + for (order, item) in program.top_level.iter().enumerate() { + let TopLevel::Rule(CstRuleEntry::SubroutineDef { name, span, .. }) = item else { continue; }; if self @@ -681,8 +661,7 @@ impl Lowerer { *span, ); } - self.subroutine_definitions - .push((name.clone(), program.declarations.len() + rule_order)); + self.subroutine_definitions.push((name.clone(), order)); } } diff --git a/crates/opy-rs/src/parser.rs b/crates/opy-rs/src/parser.rs index ad79912..64148e7 100644 --- a/crates/opy-rs/src/parser.rs +++ b/crates/opy-rs/src/parser.rs @@ -9,7 +9,7 @@ use crate::cst::{ Annotation, AnnotationArg, CallArg, Decl, DictEntry, Event, Expr, IfBranch, Program, Rule, - RuleEntry, Stmt, SwitchArm, + RuleEntry, Stmt, SwitchArm, TopLevel, }; use crate::diag::{OpyError, Position, Span}; use crate::lexer::{Token, TokenKind}; @@ -134,6 +134,7 @@ impl Parser<'_> { fn parse_program(&mut self) -> Program { let mut declarations = Vec::new(); let mut rules = Vec::new(); + let mut top_level = Vec::new(); loop { self.skip_newlines(); if self.peek_kind() == TokenKind::Eof { @@ -144,7 +145,23 @@ impl Parser<'_> { } else { None }; + let declaration_count = declarations.len(); + let rule_count = rules.len(); let ok = self.parse_top_level(&mut declarations, &mut rules, rule_prefix); + if ok { + if declarations.len() > declaration_count { + top_level.push(TopLevel::Declaration( + declarations + .last() + .expect("declaration was appended") + .clone(), + )); + } else if rules.len() > rule_count { + top_level.push(TopLevel::Rule( + rules.last().expect("rule was appended").clone(), + )); + } + } if !ok { self.recover_line(); } @@ -152,6 +169,7 @@ impl Parser<'_> { Program { declarations, rules, + top_level, settings: None, } } diff --git a/crates/opy-rs/tests/issue_143_semantic_hir.rs b/crates/opy-rs/tests/issue_143_semantic_hir.rs index f778a69..d503495 100644 --- a/crates/opy-rs/tests/issue_143_semantic_hir.rs +++ b/crates/opy-rs/tests/issue_143_semantic_hir.rs @@ -78,3 +78,42 @@ fn subroutine_visibility_follows_source_order_and_rejects_duplicate_defs() { assert_eq!(diagnostic.code, "duplicate-definition"); assert_eq!(diagnostic.span.as_ref().expect("source span").start.line, 4); } + +#[test] +fn visibility_uses_interleaved_top_level_source_order() { + let source = "def worker():\n pass\n\nmacro call_worker():\n worker()\n\nrule \"call macro\":\n @Event global\n call_worker()\n"; + let outcome = check(source, "main.opy", Path::new("")); + assert!( + outcome.is_clean(), + "a macro must see an earlier def: {:?}", + outcome.diagnostics + ); + + let later_macro = check( + "rule \"call macro\":\n @Event global\n later_macro()\n\nmacro later_macro():\n pass\n", + "main.opy", + Path::new(""), + ); + assert_eq!( + later_macro + .diagnostics + .first() + .expect("forward macro diagnostic") + .code, + "unknown-action" + ); + + let later_global = check( + "rule \"use global\":\n @Event global\n later = 1\n\nglobalvar later\n", + "main.opy", + Path::new(""), + ); + assert_eq!( + later_global + .diagnostics + .first() + .expect("forward global diagnostic") + .code, + "unknown-identifier" + ); +}