From d0636f1e2483bc85b0a1929fbb94c206ad45a7b7 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Fri, 23 Sep 2022 21:07:11 +0300 Subject: refactor(tvix/eval): clean up unused attrpath normalisation logic The previous way of sanitising dynamic keys is going away as we're slowly introducing the new nested key logic. While touching this stuff, I've also changed all the related string types to SmolStr as that is more sensible for identifiers. Change-Id: If30c74151508719d646d0e68e7d6f62c36f4d23f Reviewed-on: https://cl.tvl.fyi/c/depot/+/6779 Tested-by: BuildkiteCI Reviewed-by: sterni --- tvix/eval/src/compiler/bindings.rs | 88 ++++++++++++++++---------------------- 1 file changed, 36 insertions(+), 52 deletions(-) (limited to 'tvix/eval/src') diff --git a/tvix/eval/src/compiler/bindings.rs b/tvix/eval/src/compiler/bindings.rs index 047b9d5b6cc3..d7a5513934a1 100644 --- a/tvix/eval/src/compiler/bindings.rs +++ b/tvix/eval/src/compiler/bindings.rs @@ -60,7 +60,7 @@ impl Compiler<'_> { kind: BindingsKind, count: &mut usize, node: &N, - ) -> Vec<(ast::Expr, String, Span)> + ) -> Vec<(ast::Expr, SmolStr, Span)> where N: ToSpan + ast::HasEntry, { @@ -69,7 +69,7 @@ impl Compiler<'_> { // declare them immediately. // // Inherits with namespaces are returned to the caller. - let mut inherit_froms: Vec<(ast::Expr, String, Span)> = vec![]; + let mut inherit_froms: Vec<(ast::Expr, SmolStr, Span)> = vec![]; for inherit in node.inherits() { match inherit.from() { @@ -107,7 +107,7 @@ impl Compiler<'_> { // Place key on the stack when compiling attribute sets. if kind.is_attrs() { - self.emit_constant(Value::String(SmolStr::new(&name).into()), &attr); + self.emit_constant(Value::String(name.clone().into()), &attr); let span = self.span_for(&attr); self.scope_mut().declare_phantom(span, true); } @@ -124,7 +124,7 @@ impl Compiler<'_> { let span = self.span_for(&attr); self.scope_mut().declare_phantom(span, false) } else { - self.declare_local(&attr, &name) + self.declare_local(&attr, name) }; self.scope_mut().mark_initialised(idx); @@ -160,7 +160,7 @@ impl Compiler<'_> { fn declare_namespaced_inherits( &mut self, kind: BindingsKind, - inherit_froms: Vec<(ast::Expr, String, Span)>, + inherit_froms: Vec<(ast::Expr, SmolStr, Span)>, bindings: &mut Vec, ) { for (from, name, span) in inherit_froms { @@ -170,7 +170,7 @@ impl Compiler<'_> { // consumed by `OpAttrs`). Some(KeySlot { slot: self.scope_mut().declare_phantom(span, false), - name: SmolStr::new(&name), + name: name.clone(), }) } else { None @@ -179,7 +179,9 @@ impl Compiler<'_> { let value_slot = match kind { // In recursive scopes, the value needs to be accessible on the // stack. - BindingsKind::LetIn | BindingsKind::RecAttrs => self.declare_local(&span, &name), + BindingsKind::LetIn | BindingsKind::RecAttrs => { + self.declare_local(&span, name.clone()) + } // In non-recursive attribute sets, the value is inaccessible // (only consumed by `OpAttrs`). @@ -191,7 +193,7 @@ impl Compiler<'_> { value_slot, binding: Binding::InheritFrom { namespace: from, - name: SmolStr::new(&name), + name, span, }, }); @@ -212,27 +214,34 @@ impl Compiler<'_> { for entry in node.attrpath_values() { *count += 1; - let mut path = match self.normalise_ident_path(entry.attrpath().unwrap().attrs()) { - Ok(p) => p, - Err(err) => { - self.errors.push(err); - continue; - } - }; - + let path = entry.attrpath().unwrap().attrs().collect::>(); if path.len() != 1 { - self.emit_error( - &entry, - ErrorKind::NotImplemented("nested bindings in recursive scope :("), - ); + self.emit_error(&entry, ErrorKind::NotImplemented("nested bindings :(")); continue; } - let key_span = self.span_for(&entry.attrpath().unwrap()); + let name = match self.expr_static_attr_str(&path[0]) { + Some(name) => name, + + None if kind.is_attrs() => { + self.emit_error( + &entry, + ErrorKind::NotImplemented("dynamic keys in `rec` sets"), + ); + continue; + } + + None => { + self.emit_error(&path[0], ErrorKind::DynamicKeyInScope("let-expression")); + continue; + } + }; + + let key_span = self.span_for(&path[0]); let key_slot = if kind.is_attrs() { Some(KeySlot { + name: name.clone(), slot: self.scope_mut().declare_phantom(key_span, false), - name: SmolStr::new(&path[0]), }) } else { None @@ -241,9 +250,7 @@ impl Compiler<'_> { let value_slot = match kind { // In recursive scopes, the value needs to be accessible on the // stack. - BindingsKind::LetIn | BindingsKind::RecAttrs => { - self.declare_local(&key_span, path.pop().unwrap()) - } + BindingsKind::LetIn | BindingsKind::RecAttrs => self.declare_local(&key_span, name), // In non-recursive attribute sets, the value is inaccessible // (only consumed by `OpAttrs`). @@ -273,7 +280,7 @@ impl Compiler<'_> { 'entries: for kv in entries { // Attempt to turn the attrpath into a list of static strings, but // abort this process if any dynamic fragments are encountered. - let static_attrpath: Option> = kv + let static_attrpath: Option> = kv .attrpath() .unwrap() .attrs() @@ -630,31 +637,8 @@ impl Compiler<'_> { idx } - /// Convert a single identifier path fragment of a let binding to a string - /// if possible, or raise an error about the node being dynamic. - fn binding_name(&self, node: ast::Attr) -> EvalResult { - match self.expr_static_attr_str(&node) { - Some(s) => Ok(s), - None => Err(Error { - // this code path will go away soon, hence the TODO below - kind: ErrorKind::DynamicKeyInScope("TODO"), - span: self.span_for(&node), - }), - } - } - - /// Normalises identifier fragments into a single string vector for - /// `let`-expressions; fails if fragments requiring dynamic computation are - /// encountered. - fn normalise_ident_path>( - &self, - path: I, - ) -> EvalResult> { - path.map(|node| self.binding_name(node)).collect() - } - /// Convert a non-dynamic string expression to a string if possible. - fn expr_static_str(&self, node: &ast::Str) -> Option { + fn expr_static_str(&self, node: &ast::Str) -> Option { let mut parts = node.normalized_parts(); if parts.len() != 1 { @@ -662,7 +646,7 @@ impl Compiler<'_> { } if let Some(ast::InterpolPart::Literal(lit)) = parts.pop() { - return Some(lit); + return Some(SmolStr::new(&lit)); } None @@ -671,7 +655,7 @@ impl Compiler<'_> { /// Convert the provided `ast::Attr` into a statically known string if /// possible. // TODO(tazjin): these should probably be SmolStr - fn expr_static_attr_str(&self, node: &ast::Attr) -> Option { + fn expr_static_attr_str(&self, node: &ast::Attr) -> Option { match node { ast::Attr::Ident(ident) => Some(ident.ident_token().unwrap().text().into()), ast::Attr::Str(s) => self.expr_static_str(s), -- cgit 1.4.1