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
This commit is contained in:
Vincent Ambo 2022-09-03 00:26:32 +03:00 committed by tazjin
parent 7bc6e5984d
commit 9973ddfcba
2 changed files with 48 additions and 22 deletions

View file

@ -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,
});