CSY/reowolf Changeset - f6771292dc6d · Centrum Wiskunde & Informatica (CWI)

Changeset - f6771292dc6d

Parent rev.

Child rev.

[Not reviewed]

0 5 0

mh - 4 years ago 2021-12-21 15:22:02
contact@maxhenger.nl

Refactor code related to scoping variables and labels

5 files changed with 118 insertions and 96 deletions:

src/protocol/ast.rs

src/protocol/ast_printer.rs

src/protocol/parser/pass_definitions.rs

src/protocol/parser/pass_validation_linking.rs

src/protocol/tests/parser_validation.rs

0 comments (0 inline, 0 general)

src/protocol/ast.rs

➞

Show inline comments

@@ @@ -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<Scope>,
     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<VariableId>,
     pub labels: Vec<LabeledStatementId>,
     pub next: StatementId,

src/protocol/ast_printer.rs

➞

Show inline comments

@@ @@ -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 {

src/protocol/parser/pass_definitions.rs

➞

Show inline comments

@@ @@ -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(),

src/protocol/parser/pass_validation_linking.rs

➞

Show inline comments

@@ @@ -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<SynchronousStatementId>) -> 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 {

src/protocol/tests/parser_validation.rs

➞

Show inline comments

@@ @@ -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<u32> 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;

0 comments (0 inline, 0 general)