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

Changeset - 699bec97a862

Parent rev.

Child rev.

[Not reviewed]

0 2 0

mh - 4 years ago 2021-12-21 15:39:08
contact@maxhenger.nl

Fix longstanding variable scoping bug, add select statement test

2 files changed with 32 insertions and 14 deletions:

src/protocol/parser/pass_validation_linking.rs

src/protocol/tests/parser_validation.rs

0 comments (0 inline, 0 general)

src/protocol/parser/pass_validation_linking.rs

➞

Show inline comments

@@ @@ -1613,18 +1613,18 @@ impl PassValidationLinking { @@
     /// Adds a local variable to the current scope. It will also annotate the
     /// `Local` in the AST with its relative position in the block.
-    fn checked_add_local(&mut self, ctx: &mut Ctx, target_scope: Scope, relative_pos: i32, id: VariableId) -> Result<(), ParseError> {
+    fn checked_add_local(&mut self, ctx: &mut Ctx, target_scope: Scope, target_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, target_scope);
         let mut scope = target_scope;
         println!("DEBUG: Adding local '{}' at relative_pos {} in scope {:?}", local.identifier.value.as_str(), target_relative_pos, target_scope);
         // We immediately go to the parent scope. We check the target scope
         // in the call at the end. That is also where we check for collisions
         // with symbols.
         let block = &ctx.heap[target_scope.to_block()];
         let mut scope = block.scope_node.parent;
         let mut cur_relative_pos = block.scope_node.relative_pos_in_parent;
         loop {
             // We immediately go to the parent scope. We check the current scope
             // in the call at the end. Likewise for checking the symbol table.
             let block = &ctx.heap[scope.to_block()];
             scope = block.scope_node.parent;
             if let Scope::Definition(definition_id) = scope {
                 // At outer scope, check parameters of function/component
                 for parameter_id in ctx.heap[definition_id].parameters() {
@@ @@ -1645,7 +1645,7 @@ impl PassValidationLinking { @@
+            }
             // If here then the parent scope is a block scope
-            let local_relative_pos = ctx.heap[scope.to_block()].scope_node.relative_pos_in_parent;
+            let block = &ctx.heap[scope.to_block()];
             for other_local_id in &block.locals {
                 let other_local = &ctx.heap[*other_local_id];
@@ @@ -1653,7 +1653,7 @@ impl PassValidationLinking { @@
                 // is defined in a higher-level scope, but later than the scope
                 // in which the current variable resides.
                 if local.this != *other_local_id &&
-                    local_relative_pos >= other_local.relative_pos_in_block &&
+                    cur_relative_pos >= other_local.relative_pos_in_block &&
                     local.identifier == other_local.identifier {
                     // Collision within this scope
                     return Err(
@@ @@ -1665,10 +1665,13 @@ impl PassValidationLinking { @@
                     );
+                }
+            }
             scope = block.scope_node.parent;
             cur_relative_pos = block.scope_node.relative_pos_in_parent;
+        }
         // No collisions in any of the parent scope, attempt to add to scope
-        self.checked_at_single_scope_add_local(ctx, target_scope, relative_pos, id)
+        self.checked_at_single_scope_add_local(ctx, target_scope, target_relative_pos, id)
+    }
     /// Adds a local variable to the specified scope. Will check the specified

src/protocol/tests/parser_validation.rs

➞

Show inline comments

@@ @@ -651,14 +651,29 @@ fn test_incorrect_select_statement() { @@
         .assert_msg_has(0, "inside sync blocks");
     });
-    Tester::new_single_source_expect_ok(
+    Tester::new_single_source_expect_err(
         "variable in previous block",
         "primitive f() {
             channel<u32> tx -> rx;
             u32 a = 0; // this one will be shadowed
             sync select { auto a = get(rx) -> {} }
         }"
     );
     ).error(|e| { e
         .assert_num(2)
         .assert_occurs_at(0, "a = get").assert_msg_has(0, "variable name conflicts")
         .assert_occurs_at(1, "a = 0").assert_msg_has(1, "Previous variable");
     });
     Tester::new_single_source_expect_err(
         "put inside arm",
         "primitive f() {
             channel<u32> a -> b;
             sync select { put(a) -> {} }
         }"
     ).error(|e| { e
         .assert_occurs_at(0, "put")
         .assert_msg_has(0, "may not occur");
     });
+}
 #[test]

0 comments (0 inline, 0 general)