about summary refs log tree commit diff
path: root/tvix
diff options
context:
space:
mode:
Diffstat (limited to 'tvix')
-rw-r--r--tvix/eval/src/compiler/bindings.rs126
-rw-r--r--tvix/eval/src/errors.rs15
2 files changed, 116 insertions, 25 deletions
diff --git a/tvix/eval/src/compiler/bindings.rs b/tvix/eval/src/compiler/bindings.rs
index 7d6b30569392..5fa08210fd46 100644
--- a/tvix/eval/src/compiler/bindings.rs
+++ b/tvix/eval/src/compiler/bindings.rs
@@ -4,8 +4,29 @@
 //! In the case of recursive scopes these cases share almost all of their
 //! (fairly complex) logic.
 
+use rnix::ast::AttrpathValue;
+
 use super::*;
 
+/// What kind of bindings scope is being compiled?
+#[derive(Clone, Copy, PartialEq)]
+enum BindingsKind {
+    /// Standard `let ... in ...`-expression.
+    LetIn,
+
+    /// Non-recursive attribute set.
+    Attrs,
+
+    /// Recursive attribute set.
+    RecAttrs,
+}
+
+impl BindingsKind {
+    fn is_attrs(&self) -> bool {
+        matches!(self, BindingsKind::Attrs | BindingsKind::RecAttrs)
+    }
+}
+
 // Data structures to track the bindings observed in the second pass, and
 // forward the information needed to compile their value.
 enum Binding {
@@ -20,6 +41,18 @@ enum Binding {
     },
 }
 
+impl Binding {
+    fn merge(&mut self, _expr: ast::Expr) -> Option<ErrorKind> {
+        match self {
+            Binding::InheritFrom { name, .. } => {
+                Some(ErrorKind::UnmergeableInherit { name: name.clone() })
+            }
+
+            Binding::Plain { .. } => todo!(),
+        }
+    }
+}
+
 enum KeySlot {
     /// There is no key slot (`let`-expressions do not emit their key).
     None { name: SmolStr },
@@ -38,6 +71,19 @@ struct TrackedBinding {
     binding: Binding,
 }
 
+impl TrackedBinding {
+    /// Does this binding match the given key?
+    ///
+    /// Used to determine which binding to merge another one into.
+    fn matches(&self, key: &str) -> bool {
+        match &self.key_slot {
+            KeySlot::None { name } => name == key,
+            KeySlot::Static { name, .. } => name == key,
+            KeySlot::Dynamic { .. } => false,
+        }
+    }
+}
+
 struct TrackedBindings {
     kind: BindingsKind,
     bindings: Vec<TrackedBinding>,
@@ -51,6 +97,46 @@ impl TrackedBindings {
         }
     }
 
+    /// Attempt to merge an entry into an existing matching binding, assuming
+    /// that the provided binding is mergable (i.e. either a nested key or an
+    /// attribute set literal).
+    ///
+    /// Returns true if the binding was merged, false if it needs to be compiled
+    /// separately as a new binding.
+    fn try_merge(&mut self, c: &mut Compiler, entry: &AttrpathValue) -> bool {
+        let path = entry.attrpath().unwrap().attrs().collect::<Vec<_>>();
+        let value = entry.value().unwrap();
+
+        // If the path only has one element, and if the entry is not an attribute
+        // set literal, the entry can not be merged.
+        if path.len() == 1 && !matches!(value, ast::Expr::AttrSet(_)) {
+            return false;
+        }
+
+        // If the first element of the path is not statically known, the entry
+        // can not be merged.
+        let name = match c.expr_static_attr_str(&path[0]) {
+            Some(name) => name,
+            None => return false,
+        };
+
+        // If there is no existing binding with this key, the entry can not be
+        // merged.
+        // TODO: benchmark whether using a map or something is useful over the
+        // `find` here
+        let binding = match self.bindings.iter_mut().find(|b| b.matches(&name)) {
+            Some(b) => b,
+            None => return false,
+        };
+
+        // No more excuses ... the binding can be merged!
+        if let Some(err) = binding.binding.merge(value) {
+            c.emit_error(entry, err);
+        }
+
+        true
+    }
+
     /// Add a completely new binding to the tracked bindings.
     fn track_new(&mut self, key_slot: KeySlot, value_slot: LocalIdx, binding: Binding) {
         self.bindings.push(TrackedBinding {
@@ -61,25 +147,6 @@ impl TrackedBindings {
     }
 }
 
-/// What kind of bindings scope is being compiled?
-#[derive(Clone, Copy, PartialEq)]
-enum BindingsKind {
-    /// Standard `let ... in ...`-expression.
-    LetIn,
-
-    /// Non-recursive attribute set.
-    Attrs,
-
-    /// Recursive attribute set.
-    RecAttrs,
-}
-
-impl BindingsKind {
-    fn is_attrs(&self) -> bool {
-        matches!(self, BindingsKind::Attrs | BindingsKind::RecAttrs)
-    }
-}
-
 /// AST-traversing functions related to bindings.
 impl Compiler<'_> {
     /// Compile all inherits of a node with entries that do *not* have a
@@ -242,16 +309,25 @@ impl Compiler<'_> {
         N: ToSpan + ast::HasEntry,
     {
         for entry in node.attrpath_values() {
+            if bindings.try_merge(self, &entry) {
+                // Binding is nested, or already exists and was merged, move on.
+                continue;
+            }
+
             *count += 1;
 
-            let mut path = entry.attrpath().unwrap().attrs().collect::<Vec<_>>();
-            if path.len() != 1 {
+            let (key, mut path) = {
+                let mut path = entry.attrpath().unwrap().attrs();
+                (path.next().unwrap(), path.peekable())
+            };
+
+            if path.peek().is_some() {
                 self.emit_error(&entry, ErrorKind::NotImplemented("nested bindings :("));
                 continue;
             }
 
-            let key_span = self.span_for(&path[0]);
-            let key_slot = match self.expr_static_attr_str(&path[0]) {
+            let key_span = self.span_for(&key);
+            let key_slot = match self.expr_static_attr_str(&key) {
                 Some(name) if kind.is_attrs() => KeySlot::Static {
                     name,
                     slot: self.scope_mut().declare_phantom(key_span, false),
@@ -260,12 +336,12 @@ impl Compiler<'_> {
                 Some(name) => KeySlot::None { name },
 
                 None if kind.is_attrs() => KeySlot::Dynamic {
-                    attr: path.pop().unwrap(),
+                    attr: key,
                     slot: self.scope_mut().declare_phantom(key_span, false),
                 },
 
                 None => {
-                    self.emit_error(&path[0], ErrorKind::DynamicKeyInScope("let-expression"));
+                    self.emit_error(&key, ErrorKind::DynamicKeyInScope("let-expression"));
                     continue;
                 }
             };
diff --git a/tvix/eval/src/errors.rs b/tvix/eval/src/errors.rs
index 822ebdbc8cb4..92ca59f105a4 100644
--- a/tvix/eval/src/errors.rs
+++ b/tvix/eval/src/errors.rs
@@ -4,6 +4,7 @@ use std::{fmt::Display, num::ParseIntError};
 
 use codemap::{CodeMap, Span};
 use codemap_diagnostic::{Diagnostic, Emitter, Level, SpanLabel, SpanStyle};
+use smol_str::SmolStr;
 
 use crate::Value;
 
@@ -88,6 +89,12 @@ pub enum ErrorKind {
         length: i64,
     },
 
+    // Errors specific to nested attribute sets and merges thereof.
+    /// Nested attributes can not be merged with an inherited value.
+    UnmergeableInherit {
+        name: SmolStr,
+    },
+
     /// Tvix internal warning for features triggered by users that are
     /// not actually implemented yet, and without which eval can not
     /// proceed.
@@ -242,6 +249,13 @@ to a missing value in the attribute set(s) included via `with`."#,
                 )
             }
 
+            ErrorKind::UnmergeableInherit { name } => {
+                format!(
+                    "cannot merge a nested attribute set into the inherited entry '{}'",
+                    name
+                )
+            }
+
             ErrorKind::NotImplemented(feature) => {
                 format!("feature not yet implemented in Tvix: {}", feature)
             }
@@ -275,6 +289,7 @@ to a missing value in the attribute set(s) included via `with`."#,
             ErrorKind::ParseIntError(_) => "E021",
             ErrorKind::NegativeLength { .. } => "E022",
             ErrorKind::TailEmptyList { .. } => "E023",
+            ErrorKind::UnmergeableInherit { .. } => "E024",
             ErrorKind::NotImplemented(_) => "E999",
         }
     }