From 932a70d0c25a3d7b8cfb31788348e850648e2aa1 Mon Sep 17 00:00:00 2001
From: Vincent Ambo <mail@tazj.in>
Date: Sat, 27 Aug 2022 19:55:02 +0300
Subject: refactor(tvix/eval): track with stack size as a simple integer

The `With` struct no longer contained any internals after the cleanup
logic for the stack had been moved into Compiler::compile_with,
leaving the `Vec<With>` to essentially act as a counter for the number
of things on the with stack.

That's inefficient of course, so with this commit it actually becomes
an integer (with an encapsulated API within scope::Scope).

Change-Id: I67a00987fc8b46b30d369a96d41e83c8af5b1998
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6311
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
---
 tvix/eval/src/compiler/mod.rs   | 12 ++++++------
 tvix/eval/src/compiler/scope.rs | 25 ++++++++++++++++++-------
 2 files changed, 24 insertions(+), 13 deletions(-)

(limited to 'tvix/eval/src/compiler')

diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs
index 17ea8921a0c3..2fac7560a6f6 100644
--- a/tvix/eval/src/compiler/mod.rs
+++ b/tvix/eval/src/compiler/mod.rs
@@ -29,7 +29,7 @@ use crate::opcode::{CodeIdx, Count, JumpOffset, OpCode, StackIdx, UpvalueIdx};
 use crate::value::{Closure, Lambda, Value};
 use crate::warnings::{EvalWarning, WarningKind};
 
-use self::scope::{Depth, Local, LocalPosition, Scope, Upvalue, With};
+use self::scope::{Depth, Local, LocalPosition, Scope, Upvalue};
 
 /// Represents the result of compiling a piece of Nix code. If
 /// compilation was successful, the resulting bytecode can be passed
@@ -601,7 +601,7 @@ impl Compiler {
                 // Within a `let` binding, inheriting from the outer
                 // scope is a no-op *if* the identifier can be
                 // statically resolved.
-                None if self.scope().with_stack.is_empty() => {
+                None if !self.scope().has_with() => {
                     self.emit_warning(inherit.syntax().clone(), WarningKind::UselessInherit);
                     continue;
                 }
@@ -699,7 +699,7 @@ impl Compiler {
                     return;
                 }
 
-                if self.scope().with_stack.is_empty() {
+                if !self.scope().has_with() {
                     self.emit_error(node.syntax().clone(), ErrorKind::UnknownStaticVariable);
                     return;
                 }
@@ -727,7 +727,7 @@ impl Compiler {
         self.compile(node.namespace().unwrap());
         self.declare_phantom();
 
-        self.scope_mut().with_stack.push(With {});
+        self.scope_mut().push_with();
 
         let with_idx = self
             .scope()
@@ -745,7 +745,7 @@ impl Compiler {
         self.compile(node.body().unwrap());
 
         self.chunk().push_op(OpCode::OpPopWith);
-        self.scope_mut().with_stack.pop();
+        self.scope_mut().pop_with();
         self.end_scope();
     }
 
@@ -991,7 +991,7 @@ impl Compiler {
 
         // Determine whether the upvalue is a dynamic variable in the
         // enclosing context.
-        if !self.contexts[ctx_idx - 1].scope.with_stack.is_empty() {
+        if self.contexts[ctx_idx - 1].scope.has_with() {
             return Some(self.add_upvalue(ctx_idx, Upvalue::Dynamic(SmolStr::new(name))));
         }
 
diff --git a/tvix/eval/src/compiler/scope.rs b/tvix/eval/src/compiler/scope.rs
index 93aa39a0d86f..e295731e3258 100644
--- a/tvix/eval/src/compiler/scope.rs
+++ b/tvix/eval/src/compiler/scope.rs
@@ -65,10 +65,6 @@ pub struct Local {
     pub used: bool,
 }
 
-/// Represents an entry on the runtime with-stack.
-#[derive(Debug)]
-pub struct With {}
-
 /// Represents the current position of a local as resolved in a scope.
 pub enum LocalPosition {
     /// Local is not known in this scope.
@@ -112,9 +108,8 @@ pub struct Scope {
     // How many scopes "deep" are these locals?
     pub scope_depth: usize,
 
-    // Stack indices of attribute sets currently in scope through
-    // `with`.
-    pub with_stack: Vec<With>,
+    // Current size of the `with`-stack at runtime.
+    with_stack_size: usize,
 
     // Users are allowed to override globally defined symbols like
     // `true`, `false` or `null` in scopes. We call this "scope
@@ -151,6 +146,22 @@ impl Scope {
             .retain(|_, poisoned_at| *poisoned_at != depth);
     }
 
+    /// Increase the `with`-stack size of this scope.
+    pub fn push_with(&mut self) {
+        self.with_stack_size += 1;
+    }
+
+    /// Decrease the `with`-stack size of this scope.
+    pub fn pop_with(&mut self) {
+        self.with_stack_size -= 1;
+    }
+
+    /// Does this scope currently require dynamic runtime resolution
+    /// of identifiers that could not be found?
+    pub fn has_with(&self) -> bool {
+        self.with_stack_size > 0
+    }
+
     /// Resolve the stack index of a statically known local.
     pub fn resolve_local(&mut self, name: &str) -> LocalPosition {
         for (idx, local) in self.locals.iter_mut().enumerate().rev() {
-- 
cgit 1.4.1