Ensure that scopes are cleaned up correctly upon error. (#20)

When evaluating rules, upon error the last pushed scope wasn't being
popped from the stack of scopes. This causes incorrect behavior
when there are multiple definitions for the same rule name.

The fix is to make sure that the scopes are popped manually upon encountering errors.

Once the interpreter logic is locked down, then we need to clean up scope management
using Drop functions so that the cleanup happens even during short circuited return.

Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>

src/interpreter.rs

@@ -1310,6 +1310,7 @@ impl<'source> Interpreter<'source> {
                 Err(e) => {
                     // If the rule produces an error, save the error.
                     errors.push(e);
+                    self.scopes = scopes;
                     continue;
                 }
             };
@@ -1550,33 +1551,38 @@ impl<'source> Interpreter<'source> {
         span: &'source Span<'source>,
         bodies: &'source Vec<RuleBody<'source>>,
     ) -> Result<Value> {
-        let mut result = true;
-        self.scopes.push(Scope::new());
-
-        if bodies.is_empty() {
+        let result = if bodies.is_empty() {
             self.contexts.push(ctx.clone());
-            result = self.eval_output_expr()?;
+            self.eval_output_expr()
         } else {
+            let mut result = Ok(true);
             for body in bodies {
                 self.contexts.push(ctx.clone());
-                result = self.eval_query(&body.query)?;
+                result = self.eval_query(&body.query);
 
-                // The body evaluated successfully.
-                if result {
+                if matches!(&result, Ok(true) | Err(_)) {
                     break;
                 }
 
+                // TODO: Manage other scoped data.
+                self.scopes.pop();
                 if bodies.len() > 1 {
                     unimplemented!("else bodies");
                 }
             }
-        }
+            result
+        };
 
         let ctx = match self.contexts.pop() {
             Some(ctx) => ctx,
             _ => bail!("internal error: rule's context already popped"),
         };
 
+        let result = match result {
+            Ok(r) => r,
+            Err(e) => return Err(e),
+        };
+
         // Drop local variables and leave the local scope
         self.scopes.pop();
 

src/scheduler.rs

@@ -39,7 +39,6 @@ pub enum SortResult {
 }
 
 pub fn schedule<'a>(infos: &mut [StmtInfo<'a>]) -> Result<SortResult> {
-    println!("infos: {infos:?}");
     let num_statements = infos.len();
 
     // Mapping from each var to the list of statements that define it.

tests/interpreter/cases/call/or.yaml

@@ -130,10 +130,8 @@ cases:
         a2 = fcn(5)
     query: data.test
     want_result:
-      a1: 
-        set!: [hello world]
-      a2: 
-        set!: [6]
+      a1: "hello world"
+      a2: 6
 
   - note: or-all-error
     data: {}
@@ -151,4 +149,4 @@ cases:
 
         a1 = fcn("world")
     query: data.test
-    error: "`add` expects numeric argument. Got `\"world\"` instead"
+    error: "`add` expects numeric argument. Got `\"world\"` instead"