diff --git a/src/protocol/ast.rs b/src/protocol/ast.rs index 5ba51755ee94a3ca7ac27b1ef375c73608ea19f8..da5ba6db6c6a05dde8bafa165679eda350dd7256 100644 --- a/src/protocol/ast.rs +++ b/src/protocol/ast.rs @@ -699,21 +699,32 @@ impl<'a> Iterator for ConcreteTypeIter<'a> { pub enum Scope { Definition(DefinitionId), Regular(BlockStatementId), - Synchronous((SynchronousStatementId, BlockStatementId)), + Synchronous(SynchronousStatementId, BlockStatementId), } impl Scope { + pub(crate) fn new_invalid() -> Scope { + return Scope::Definition(DefinitionId::new_invalid()); + } + + pub(crate) fn is_invalid(&self) -> bool { + match self { + Scope::Definition(id) => id.is_invalid(), + _ => false, + } + } + pub fn is_block(&self) -> bool { match &self { Scope::Definition(_) => false, Scope::Regular(_) => true, - Scope::Synchronous(_) => true, + Scope::Synchronous(_, _) => true, } } pub fn to_block(&self) -> BlockStatementId { match &self { Scope::Regular(id) => *id, - Scope::Synchronous((_, id)) => *id, + Scope::Synchronous(_, id) => *id, _ => panic!("unable to get BlockStatement from Scope") } } @@ -726,13 +737,15 @@ impl Scope { pub struct ScopeNode { pub parent: Scope, pub nested: Vec, + pub relative_pos_in_parent: i32, } impl ScopeNode { pub(crate) fn new_invalid() -> Self { ScopeNode{ - parent: Scope::Definition(DefinitionId::new_invalid()), + parent: Scope::new_invalid(), nested: Vec::new(), + relative_pos_in_parent: -1, } } } @@ -1170,7 +1183,6 @@ pub struct BlockStatement { pub scope_node: ScopeNode, pub first_unique_id_in_scope: i32, // Temporary fix until proper bytecode/asm is generated pub next_unique_id_in_scope: i32, // Temporary fix until proper bytecode/asm is generated - pub relative_pos_in_parent: i32, pub locals: Vec, pub labels: Vec, pub next: StatementId, diff --git a/src/protocol/ast_printer.rs b/src/protocol/ast_printer.rs index b0e2d899195d33eedfd977ee874103a9eb8329c7..5529cd3dde91d3b9834ab3cc4ecce3d8641eef9d 100644 --- a/src/protocol/ast_printer.rs +++ b/src/protocol/ast_printer.rs @@ -403,7 +403,7 @@ impl ASTWriter { self.kv(indent2).with_s_key("EndBlockID").with_disp_val(&stmt.end_block.0.index); self.kv(indent2).with_s_key("FirstUniqueScopeID").with_disp_val(&stmt.first_unique_id_in_scope); self.kv(indent2).with_s_key("NextUniqueScopeID").with_disp_val(&stmt.next_unique_id_in_scope); - self.kv(indent2).with_s_key("RelativePos").with_disp_val(&stmt.relative_pos_in_parent); + self.kv(indent2).with_s_key("RelativePos").with_disp_val(&stmt.scope_node.relative_pos_in_parent); self.kv(indent2).with_s_key("Statements"); for stmt_id in &stmt.statements { diff --git a/src/protocol/parser/pass_definitions.rs b/src/protocol/parser/pass_definitions.rs index 8e2d188044654999bd3044fc3e9e20228e528ec5..66ed638cc16da38f731d12225efd5d7c0a7da217 100644 --- a/src/protocol/parser/pass_definitions.rs +++ b/src/protocol/parser/pass_definitions.rs @@ -367,7 +367,6 @@ impl PassDefinitions { scope_node: ScopeNode::new_invalid(), first_unique_id_in_scope: -1, next_unique_id_in_scope: -1, - relative_pos_in_parent: 0, locals: Vec::new(), labels: Vec::new(), next: StatementId::new_invalid(), @@ -547,7 +546,6 @@ impl PassDefinitions { scope_node: ScopeNode::new_invalid(), first_unique_id_in_scope: -1, next_unique_id_in_scope: -1, - relative_pos_in_parent: 0, locals: Vec::new(), labels: Vec::new(), next: StatementId::new_invalid(), diff --git a/src/protocol/parser/pass_validation_linking.rs b/src/protocol/parser/pass_validation_linking.rs index c0dd23295de7feac26b8dcd385a504623a5b7d5c..7c4ee025797c50873fbdc82094227c4d82931caf 100644 --- a/src/protocol/parser/pass_validation_linking.rs +++ b/src/protocol/parser/pass_validation_linking.rs @@ -134,7 +134,7 @@ impl PassValidationLinking { in_test_expr: StatementId::new_invalid(), in_binding_expr: BindingExpressionId::new_invalid(), in_binding_expr_lhs: false, - cur_scope: Scope::Definition(DefinitionId::new_invalid()), + cur_scope: Scope::new_invalid(), prev_stmt: StatementId::new_invalid(), expr_parent: ExpressionParent::None, def_type: DefinitionType::Function(FunctionDefinitionId::new_invalid()), @@ -156,7 +156,7 @@ impl PassValidationLinking { self.in_test_expr = StatementId::new_invalid(); self.in_binding_expr = BindingExpressionId::new_invalid(); self.in_binding_expr_lhs = false; - self.cur_scope = Scope::Definition(DefinitionId::new_invalid()); + self.cur_scope = Scope::new_invalid(); self.def_type = DefinitionType::Function(FunctionDefinitionId::new_invalid()); self.prev_stmt = StatementId::new_invalid(); self.expr_parent = ExpressionParent::None; @@ -272,7 +272,28 @@ impl Visitor for PassValidationLinking { //-------------------------------------------------------------------------- fn visit_block_stmt(&mut self, ctx: &mut Ctx, id: BlockStatementId) -> VisitorResult { - self.visit_block_stmt_with_hint(ctx, id, None) + let old_scope = self.push_statement_scope(ctx, Scope::Regular(id)); + + // Set end of block + let block_stmt = &ctx.heap[id]; + let end_block_id = block_stmt.end_block; + + // Copy statement IDs into buffer + + // Traverse statements in block + let statement_section = self.statement_buffer.start_section_initialized(&block_stmt.statements); + assign_and_replace_next_stmt!(self, ctx, id.upcast()); + + for stmt_idx in 0..statement_section.len() { + self.relative_pos_in_block = stmt_idx as i32; + self.visit_stmt(ctx, statement_section[stmt_idx])?; + } + + statement_section.forget(); + assign_and_replace_next_stmt!(self, ctx, end_block_id.upcast()); + + self.pop_statement_scope(old_scope); + Ok(()) } fn visit_local_memory_stmt(&mut self, ctx: &mut Ctx, id: MemoryStatementId) -> VisitorResult { @@ -429,10 +450,15 @@ impl Visitor for PassValidationLinking { // Synchronous statement implicitly moves to its block assign_then_erase_next_stmt!(self, ctx, id.upcast()); + // Visit block statement. Note that we explicitly push the scope here + // (and the `visit_block_stmt` will also push, but without effect) to + // ensure the scope contains the sync ID. let sync_body = ctx.heap[id].body; debug_assert!(self.in_sync.is_invalid()); self.in_sync = id; - self.visit_block_stmt_with_hint(ctx, sync_body, Some(id))?; + let old_scope = self.push_statement_scope(ctx, Scope::Synchronous(id, sync_body)); + self.visit_block_stmt(ctx, sync_body)?; + self.pop_statement_scope(old_scope); assign_and_replace_next_stmt!(self, ctx, end_sync_id.upcast()); self.in_sync = SynchronousStatementId::new_invalid(); @@ -501,13 +527,17 @@ impl Visitor for PassValidationLinking { assign_then_erase_next_stmt!(self, ctx, id.upcast()); - // Link up the "Select" with the "EndSelect". If there are no cases then - // runtime should pick the "EndSelect" immediately. - for idx in 0..num_cases { let base_idx = 2 * idx; let guard_id = case_stmt_ids[base_idx ]; let arm_block_id = case_stmt_ids[base_idx + 1]; + debug_assert_eq!(ctx.heap[arm_block_id].as_block().this.upcast(), arm_block_id); // backwards way of saying arm_block_id is a BlockStatementId + let arm_block_id = BlockStatementId(arm_block_id); + + // The guard statement ends up belonging to the block statement + // following the arm. The reason we parse it separately is to + // extract all of the "get" calls. + let old_scope = self.push_statement_scope(ctx, Scope::Regular(arm_block_id)); // Visit the guard of this arm debug_assert!(self.in_select_guard.is_invalid()); @@ -516,23 +546,11 @@ impl Visitor for PassValidationLinking { self.visit_stmt(ctx, guard_id)?; self.in_select_guard = SelectStatementId::new_invalid(); - // If the arm declares a variable, then add it to the variables of - // this arm's code block - match &ctx.heap[guard_id] { - Statement::Local(LocalStatement::Memory(stmt)) => { - debug_assert_eq!(ctx.heap[arm_block_id].as_block().this.upcast(), arm_block_id); // backwards way of saying arm_block_id is a BlockStatementId - let variable_id = stmt.variable; - let block_stmt = BlockStatementId(arm_block_id); - let block_scope = Scope::Regular(block_stmt); - self.checked_at_single_scope_add_local(ctx, block_scope, -1, variable_id)?; - }, - Statement::Expression(_) => {}, - _ => unreachable!(), // just to be sure the parser produced the expected AST - } - // Visit the code associated with the guard - self.visit_stmt(ctx, arm_block_id)?; + self.visit_block_stmt(ctx, arm_block_id)?; + self.pop_statement_scope(old_scope); + // Link up last statement in block to EndSelect assign_then_erase_next_stmt!(self, ctx, end_select_id.upcast()); } @@ -563,24 +581,6 @@ impl Visitor for PassValidationLinking { } fn visit_goto_stmt(&mut self, ctx: &mut Ctx, id: GotoStatementId) -> VisitorResult { - // TODO: Remove this - // let target_id = self.find_label(ctx, &ctx.heap[id].label)?; - // ctx.heap[id].target = Some(target_id); - // - // let target = &ctx.heap[target_id]; - // if self.in_sync != target.in_sync { - // // We can only goto the current scope or outer scopes. Because - // // nested sync statements are not allowed we must be inside a sync - // // statement. - // debug_assert!(!self.in_sync.is_invalid()); - // let goto_stmt = &ctx.heap[id]; - // let sync_stmt = &ctx.heap[self.in_sync]; - // return Err( - // ParseError::new_error_str_at_span(&ctx.module().source, goto_stmt.span, "goto may not escape the surrounding synchronous block") - // .with_info_str_at_span(&ctx.module().source, target.label.span, "this is the target of the goto statement") - // .with_info_str_at_span(&ctx.module().source, sync_stmt.span, "which will jump past this statement") - // ); - // } self.control_flow_stmts.push(ControlFlowStatement{ in_sync: self.in_sync, in_while: self.in_while, @@ -1433,59 +1433,48 @@ impl PassValidationLinking { // Special traversal //-------------------------------------------------------------------------- - fn visit_block_stmt_with_hint(&mut self, ctx: &mut Ctx, id: BlockStatementId, hint: Option) -> VisitorResult { - // Set parent scope and relative position in the parent scope. Remember - // these values to set them back to the old values when we're done with - // the traversal of the block's statements. + /// Pushes a new scope associated with a particular statement. If that + /// statement already has an associated scope (i.e. scope associated with + /// sync statement or select statement's arm) then we won't do anything. + /// In all cases the caller must call `pop_statement_scope` with the scope + /// and relative scope position returned by this function. + fn push_statement_scope(&mut self, ctx: &mut Ctx, new_scope: Scope) -> (Scope, i32) { let old_scope = self.cur_scope.clone(); - let new_scope = match hint { - Some(sync_id) => Scope::Synchronous((sync_id, id)), - None => Scope::Regular(id), + debug_assert!(new_scope.is_block()); // never call for Definition scope + let is_new_block = if old_scope.is_block() { + old_scope.to_block() != new_scope.to_block() + } else { + true }; - match old_scope { - Scope::Definition(_def_id) => { - // Don't do anything. Block is implicitly a child of a - // definition scope. - if cfg!(debug_assertions) { - match &ctx.heap[_def_id] { - Definition::Function(proc_def) => debug_assert_eq!(proc_def.body, id), - Definition::Component(proc_def) => debug_assert_eq!(proc_def.body, id), - _ => unreachable!(), - } - } - }, - Scope::Regular(block_id) | Scope::Synchronous((_, block_id)) => { - let parent_block = &mut ctx.heap[block_id]; - parent_block.scope_node.nested.push(new_scope); - } + if !is_new_block { + // No need to push, but still return old scope, we pretend like we + // replaced it. + debug_assert!(!ctx.heap[new_scope.to_block()].scope_node.parent.is_invalid()); + return (old_scope, self.relative_pos_in_block); + } + + // This is a new block, so link it up + if old_scope.is_block() { + let parent_block = &mut ctx.heap[old_scope.to_block()]; + parent_block.scope_node.nested.push(new_scope); } self.cur_scope = new_scope; - let body = &mut ctx.heap[id]; - body.scope_node.parent = old_scope; - body.relative_pos_in_parent = self.relative_pos_in_block; - let end_block_id = body.end_block; + let cur_block = &mut ctx.heap[new_scope.to_block()]; + cur_block.scope_node.parent = old_scope; + cur_block.scope_node.relative_pos_in_parent = self.relative_pos_in_block; let old_relative_pos = self.relative_pos_in_block; + self.relative_pos_in_block = -1; - // Copy statement IDs into buffer - let statement_section = self.statement_buffer.start_section_initialized(&body.statements); - - // Perform the depth-first traversal - assign_and_replace_next_stmt!(self, ctx, id.upcast()); - for stmt_idx in 0..statement_section.len() { - self.relative_pos_in_block = stmt_idx as i32; - self.visit_stmt(ctx, statement_section[stmt_idx])?; - } - assign_and_replace_next_stmt!(self, ctx, end_block_id.upcast()); - - self.cur_scope = old_scope; - self.relative_pos_in_block = old_relative_pos; - statement_section.forget(); + return (old_scope, old_relative_pos) + } - Ok(()) + fn pop_statement_scope(&mut self, scope_to_restore: (Scope, i32)) { + self.cur_scope = scope_to_restore.0; + self.relative_pos_in_block = scope_to_restore.1; } fn visit_definition_and_assign_local_ids(&mut self, ctx: &mut Ctx, definition_id: DefinitionId) { @@ -1537,7 +1526,7 @@ impl PassValidationLinking { }; let relative_scope_pos = if scope_idx < scope_section.len() { - ctx.heap[scope_section[scope_idx]].as_block().relative_pos_in_parent + ctx.heap[scope_section[scope_idx]].as_block().scope_node.relative_pos_in_parent } else { i32::MAX }; @@ -1627,7 +1616,7 @@ impl PassValidationLinking { fn checked_add_local(&mut self, ctx: &mut Ctx, target_scope: Scope, relative_pos: i32, id: VariableId) -> Result<(), ParseError> { debug_assert!(target_scope.is_block()); let local = &ctx.heap[id]; - println!("DEBUG: Adding local '{}' at relative_pos {} in scope {:?}", local.identifier.value.as_str(), relative_pos, self.cur_scope); + println!("DEBUG: Adding local '{}' at relative_pos {} in scope {:?}", local.identifier.value.as_str(), relative_pos, target_scope); let mut scope = target_scope; loop { @@ -1656,7 +1645,7 @@ impl PassValidationLinking { } // If here then the parent scope is a block scope - let local_relative_pos = ctx.heap[scope.to_block()].relative_pos_in_parent; + let local_relative_pos = ctx.heap[scope.to_block()].scope_node.relative_pos_in_parent; for other_local_id in &block.locals { let other_local = &ctx.heap[*other_local_id]; @@ -1775,7 +1764,7 @@ impl PassValidationLinking { // Variable could not be found return None } else { - relative_pos = block.relative_pos_in_parent; + relative_pos = block.scope_node.relative_pos_in_parent; } } } @@ -1830,7 +1819,7 @@ impl PassValidationLinking { loop { debug_assert!(scope.is_block(), "scope is not a block"); - let relative_scope_pos = ctx.heap[scope.to_block()].relative_pos_in_parent; + let relative_scope_pos = ctx.heap[scope.to_block()].scope_node.relative_pos_in_parent; let block = &ctx.heap[scope.to_block()]; for label_id in &block.labels { diff --git a/src/protocol/tests/parser_validation.rs b/src/protocol/tests/parser_validation.rs index 6cc4e99196c67d163d5265831256da171defe96e..a6c63495b33f7bace49de11de94829744a70b7f1 100644 --- a/src/protocol/tests/parser_validation.rs +++ b/src/protocol/tests/parser_validation.rs @@ -592,8 +592,31 @@ fn test_variable_introduction_in_scope() { #[test] fn test_correct_select_statement() { + + Tester::new_single_source_expect_ok( + "guard variable decl", + " + primitive f() { + channel unused -> input; + + u32 outer_value = 0; + sync select { + auto in_same_guard = get(input) -> {} // decl A1 + auto in_same_gaurd = get(input) -> {} // decl A2 + auto in_guard_and_block = get(input) -> {} // decl B1 + outer_value = get(input) -> { auto in_guard_and_block = outer_value; } // decl B2 + } + } + " + ); + + Tester::new_single_source_expect_ok( + "empty select", + "primitive f() { sync select {} }" + ); + Tester::new_single_source_expect_ok( - "correct single-use", " + "mixed uses", " primitive f() { channel unused_output -> input; u32 outer_value = 0;