From a52db148203824484caa82a23fe88f0a137274ee Mon Sep 17 00:00:00 2001
From: Vincent Ambo <mail@tazj.in>
Date: Sat, 27 Aug 2022 17:16:46 +0300
Subject: feat(tvix/eval): detect illegally shadowed variables

Nix does not allow things like `let a = 1; a = 2; in a`, but doing it
across depths is allowed.

Change-Id: I6a259f8b01a254b433b58c736e245c9c764641b6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6301
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
---
 tvix/eval/src/compiler/mod.rs | 31 +++++++++++++++++++++++++++++++
 tvix/eval/src/errors.rs       |  3 +++
 2 files changed, 34 insertions(+)

diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs
index 38eaddddc16b..eefcbfcd65a0 100644
--- a/tvix/eval/src/compiler/mod.rs
+++ b/tvix/eval/src/compiler/mod.rs
@@ -54,6 +54,14 @@ impl Depth {
             Depth::At(ours) => *ours > theirs,
         }
     }
+
+    /// Does this variable live below the other given depth?
+    fn below(&self, theirs: usize) -> bool {
+        match self {
+            Depth::Unitialised => false,
+            Depth::At(ours) => *ours < theirs,
+        }
+    }
 }
 
 /// Represents a single local already known to the compiler.
@@ -1013,6 +1021,26 @@ impl Compiler {
             self.scope_mut().poison(global_ident, depth);
         }
 
+        let mut shadowed = false;
+        for local in self.scope().locals.iter().rev() {
+            if local.depth.below(self.scope().scope_depth) {
+                // Shadowing identifiers from higher scopes is allowed.
+                break;
+            }
+
+            if local.name == name {
+                shadowed = true;
+                break;
+            }
+        }
+
+        if shadowed {
+            self.emit_error(
+                node.clone(),
+                ErrorKind::VariableAlreadyDefined(name.clone()),
+            );
+        }
+
         self.scope_mut().locals.push(Local {
             name,
             depth: Depth::At(depth),
@@ -1022,6 +1050,9 @@ impl Compiler {
         });
     }
 
+    /// Declare a local variable that occupies a stack slot and should
+    /// be accounted for, but is not directly accessible by users
+    /// (e.g. attribute sets used for `with`).
     fn declare_phantom(&mut self) {
         let depth = self.scope().scope_depth;
         self.scope_mut().locals.push(Local {
diff --git a/tvix/eval/src/errors.rs b/tvix/eval/src/errors.rs
index 96217cabaf4b..49a051c16eee 100644
--- a/tvix/eval/src/errors.rs
+++ b/tvix/eval/src/errors.rs
@@ -32,6 +32,9 @@ pub enum ErrorKind {
     // Unknown variable in dynamic scope (with, rec, ...).
     UnknownDynamicVariable(String),
 
+    // User is defining the same variable twice at the same depth.
+    VariableAlreadyDefined(String),
+
     // Attempt to call something that is not callable.
     NotCallable,
 
-- 
cgit 1.4.1