From 9973ddfcba57e1a2d1a72351814210ab652bd860 Mon Sep 17 00:00:00 2001
From: Vincent Ambo <mail@tazj.in>
Date: Sat, 3 Sep 2022 00:26:32 +0300
Subject: refactor(tvix/eval): refactor locals to use an enum for phantoms

Instead of using sentinel values and an additional bool, this tracks
the identifier of a local as an enum that is either a statically known
name, or a phantom.

To make this work correctly some more locals related logic has been
encapsulated in the `scope` module, which is a good thing (that's the
goal).

Phantom values are now not initialised by default, but the only
current call site of phantoms (`with` expression compilation) performs
the initialisation right away.

This commit changes no actual functionality right now, but paves the
way for fixing an issue related to `let` bodies.

Change-Id: I679f93a59a4daeacfe40f4012263cfb7bc05034e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6421
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
---
 tvix/eval/src/compiler/scope.rs | 50 +++++++++++++++++++++++++++++------------
 1 file changed, 36 insertions(+), 14 deletions(-)

(limited to 'tvix/eval/src/compiler/scope.rs')

diff --git a/tvix/eval/src/compiler/scope.rs b/tvix/eval/src/compiler/scope.rs
index 7d557a7d2fd3..78012add40aa 100644
--- a/tvix/eval/src/compiler/scope.rs
+++ b/tvix/eval/src/compiler/scope.rs
@@ -19,13 +19,23 @@ use smol_str::SmolStr;
 
 use crate::opcode::{StackIdx, UpvalueIdx};
 
+#[derive(Debug)]
+enum LocalName {
+    /// Normally declared local with a statically known name.
+    Ident(String),
+
+    /// Phantom stack value (e.g. attribute set used for `with`) that
+    /// must be accounted for to calculate correct stack offsets.
+    Phantom,
+}
+
 /// Represents a single local already known to the compiler.
 #[derive(Debug)]
 pub struct Local {
-    // Definition name, which can be different kinds of tokens (plain
-    // string or identifier). Nix does not allow dynamic names inside
-    // of `let`-expressions.
-    pub name: String,
+    // Identifier of this local. This is always a statically known
+    // value (Nix does not allow dynamic identifier names in locals),
+    // or a "phantom" value not accessible by users.
+    name: LocalName,
 
     // Source span at which this local was declared.
     pub span: codemap::Span,
@@ -36,10 +46,6 @@ pub struct Local {
     // Is this local initialised?
     pub initialised: bool,
 
-    // Phantom locals are not actually accessible by users (e.g.
-    // intermediate values used for `with`).
-    pub phantom: bool,
-
     // Is this local known to have been used at all?
     pub used: bool,
 
@@ -53,6 +59,24 @@ impl Local {
     pub fn above(&self, theirs: usize) -> bool {
         self.depth > theirs
     }
+
+    /// Does the name of this local match the given string?
+    pub fn has_name(&self, other: &str) -> bool {
+        match &self.name {
+            LocalName::Ident(name) => name == other,
+
+            // Phantoms are *never* accessible by a name.
+            LocalName::Phantom => false,
+        }
+    }
+
+    /// Is this local intentionally ignored? (i.e. name starts with `_`)
+    pub fn is_ignored(&self) -> bool {
+        match &self.name {
+            LocalName::Ident(name) => name.starts_with('_'),
+            LocalName::Phantom => false,
+        }
+    }
 }
 
 /// Represents the current position of a local as resolved in a scope.
@@ -196,7 +220,7 @@ impl Scope {
     /// 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() {
-            if !local.phantom && local.name == name {
+            if local.has_name(name) {
                 local.used = true;
 
                 // This local is still being initialised, meaning that
@@ -219,12 +243,11 @@ impl Scope {
     pub fn declare_phantom(&mut self, span: codemap::Span) -> LocalIdx {
         let idx = self.locals.len();
         self.locals.push(Local {
+            name: LocalName::Phantom,
             span,
             depth: self.scope_depth,
-            initialised: true,
+            initialised: false,
             needs_finaliser: false,
-            name: "".into(),
-            phantom: true,
             used: true,
         });
 
@@ -235,12 +258,11 @@ impl Scope {
     pub fn declare_local(&mut self, name: String, span: codemap::Span) -> LocalIdx {
         let idx = self.locals.len();
         self.locals.push(Local {
-            name,
+            name: LocalName::Ident(name),
             span,
             depth: self.scope_depth,
             initialised: false,
             needs_finaliser: false,
-            phantom: false,
             used: false,
         });
 
-- 
cgit 1.4.1