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>
This commit is contained in:
		
							parent
							
								
									2cd08d136e
								
							
						
					
					
						commit
						932a70d0c2
					
				
					 2 changed files with 24 additions and 13 deletions
				
			
		|  | @ -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)))); | ||||
|         } | ||||
| 
 | ||||
|  |  | |||
|  | @ -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() { | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue