Refactor binder to use local scopes
Removes global variables and related logic from the Binder, simplifying its responsibilities to managing local scopes and function compilers. This change also streamlines the `define_variable` and `resolve_local` methods within `FunctionCompiler` and removes unused fields from `FunctionCompiler` and `Binder`.
This commit is contained in:
+107
-115
@@ -12,9 +12,6 @@ use std::rc::Rc;
|
||||
pub struct LocalInfo {
|
||||
pub addr: Address,
|
||||
pub identity: Identity,
|
||||
// Note: Binder doesn't strictly need the type anymore,
|
||||
// but it might be useful for built-ins during resolution.
|
||||
// For now we keep it as Any or Unknown.
|
||||
pub _ty: StaticType,
|
||||
}
|
||||
|
||||
@@ -36,23 +33,15 @@ struct FunctionCompiler {
|
||||
scopes: Vec<CompilerScope>,
|
||||
slot_count: u32,
|
||||
upvalues: Vec<Address>,
|
||||
fixed_scope_idx: i32,
|
||||
is_root: bool,
|
||||
}
|
||||
|
||||
impl FunctionCompiler {
|
||||
fn new(
|
||||
fixed_scope_idx: i32,
|
||||
identity: Identity,
|
||||
is_root: bool,
|
||||
) -> Self {
|
||||
fn new(identity: Identity) -> Self {
|
||||
Self {
|
||||
identity,
|
||||
scopes: vec![CompilerScope::new()],
|
||||
slot_count: 0,
|
||||
upvalues: Vec::new(),
|
||||
fixed_scope_idx,
|
||||
is_root,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -64,22 +53,7 @@ impl FunctionCompiler {
|
||||
self.scopes.pop();
|
||||
}
|
||||
|
||||
fn define_variable(
|
||||
&mut self,
|
||||
name: &Symbol,
|
||||
identity: Identity,
|
||||
globals: &Rc<RefCell<HashMap<Symbol, (GlobalIdx, Identity)>>>,
|
||||
) -> Result<Address, String> {
|
||||
let current_scope_idx = self.scopes.len() as i32 - 1;
|
||||
|
||||
// 1. Check if the current scope is immutable (e.g. Scope 0 after bootstrapping)
|
||||
if current_scope_idx <= self.fixed_scope_idx {
|
||||
return Err(format!(
|
||||
"Cannot define '{}': Scope {} is frozen/immutable.",
|
||||
name.name, current_scope_idx
|
||||
));
|
||||
}
|
||||
|
||||
fn define_variable(&mut self, name: &Symbol, identity: Identity) -> Result<Address, String> {
|
||||
let current_scope = self.scopes.last_mut().unwrap();
|
||||
if current_scope.locals.contains_key(name) {
|
||||
return Err(format!(
|
||||
@@ -88,49 +62,23 @@ impl FunctionCompiler {
|
||||
));
|
||||
}
|
||||
|
||||
// 2. Only Scope 0 of the ROOT function is the Global scope in Myc
|
||||
if self.is_root && current_scope_idx == 0 {
|
||||
let mut globals_map = globals.borrow_mut();
|
||||
let addr = if let Some((idx, existing_id)) = globals_map.get(name) {
|
||||
if *existing_id != identity {
|
||||
return Err(format!("Variable '{}' is already defined in global scope.", name.name));
|
||||
}
|
||||
Address::Global(*idx)
|
||||
} else {
|
||||
let idx = GlobalIdx(globals_map.len() as u32);
|
||||
globals_map.insert(name.clone(), (idx, identity.clone()));
|
||||
Address::Global(idx)
|
||||
};
|
||||
|
||||
current_scope.locals.insert(
|
||||
name.clone(),
|
||||
LocalInfo {
|
||||
addr,
|
||||
identity,
|
||||
_ty: StaticType::Any,
|
||||
},
|
||||
);
|
||||
Ok(addr)
|
||||
} else {
|
||||
// Local scope (Block or Lambda)
|
||||
let slot = LocalSlot(self.slot_count);
|
||||
current_scope.locals.insert(
|
||||
name.clone(),
|
||||
LocalInfo {
|
||||
addr: Address::Local(slot),
|
||||
identity,
|
||||
_ty: StaticType::Any,
|
||||
},
|
||||
);
|
||||
self.slot_count += 1;
|
||||
Ok(Address::Local(slot))
|
||||
}
|
||||
let slot = LocalSlot(self.slot_count);
|
||||
current_scope.locals.insert(
|
||||
name.clone(),
|
||||
LocalInfo {
|
||||
addr: Address::Local(slot),
|
||||
identity,
|
||||
_ty: StaticType::Any,
|
||||
},
|
||||
);
|
||||
self.slot_count += 1;
|
||||
Ok(Address::Local(slot))
|
||||
}
|
||||
|
||||
fn resolve_local(&self, sym: &Symbol) -> Option<LocalInfo> {
|
||||
for scope in self.scopes.iter().rev() {
|
||||
fn resolve_local(&self, sym: &Symbol) -> Option<(LocalInfo, usize)> {
|
||||
for (idx, scope) in self.scopes.iter().enumerate().rev() {
|
||||
if let Some(info) = scope.locals.get(sym) {
|
||||
return Some(info.clone());
|
||||
return Some((info.clone(), idx));
|
||||
}
|
||||
}
|
||||
None
|
||||
@@ -146,22 +94,17 @@ impl FunctionCompiler {
|
||||
}
|
||||
}
|
||||
|
||||
/// Defines the context in which a node is being bound.
|
||||
/// This allows the binder to enforce rules like "statements cannot be used as values".
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum ExprContext {
|
||||
/// The node must return a value (e.g., RHS of an assignment, function argument).
|
||||
Expression,
|
||||
/// The node is used as a standalone instruction (e.g., inside a block).
|
||||
Statement,
|
||||
}
|
||||
|
||||
pub struct Binder {
|
||||
functions: Vec<FunctionCompiler>,
|
||||
// Globals mapping: Symbol -> (Index, DefinitionIdentity)
|
||||
globals: Rc<RefCell<HashMap<Symbol, (GlobalIdx, Identity)>>>,
|
||||
// Map of Declaration Identity -> List of Lambda Identities that capture it
|
||||
capture_map: HashMap<Identity, std::collections::HashSet<Identity>>,
|
||||
fixed_scope_idx: i32,
|
||||
}
|
||||
|
||||
impl Binder {
|
||||
@@ -173,14 +116,13 @@ impl Binder {
|
||||
functions: Vec::new(),
|
||||
globals,
|
||||
capture_map: HashMap::new(),
|
||||
fixed_scope_idx,
|
||||
};
|
||||
binder.functions.push(FunctionCompiler::new(
|
||||
fixed_scope_idx,
|
||||
crate::ast::types::NodeIdentity::new(crate::ast::types::SourceLocation {
|
||||
line: 0,
|
||||
col: 0,
|
||||
}),
|
||||
true, // is_root = true for the script itself
|
||||
));
|
||||
binder
|
||||
}
|
||||
@@ -194,7 +136,6 @@ impl Binder {
|
||||
let mut binder = Self::new(globals, fixed_scope_idx);
|
||||
let bound = binder.bind(node, ExprContext::Expression, diagnostics);
|
||||
|
||||
// Convert HashSet to sorted Vec
|
||||
let final_captures = binder
|
||||
.capture_map
|
||||
.into_iter()
|
||||
@@ -211,8 +152,53 @@ impl Binder {
|
||||
_kind: crate::ast::compiler::bound_nodes::DeclarationKind,
|
||||
diag: &mut Diagnostics,
|
||||
) -> Option<Address> {
|
||||
let current_fn_idx = self.functions.len() - 1;
|
||||
|
||||
if current_fn_idx == 0 {
|
||||
let current_scope_idx = self.functions[0].scopes.len() as i32 - 1;
|
||||
if current_scope_idx <= self.fixed_scope_idx {
|
||||
diag.push_error(
|
||||
format!(
|
||||
"Cannot define '{}': Scope {} is frozen/immutable.",
|
||||
name.name, current_scope_idx
|
||||
),
|
||||
None,
|
||||
);
|
||||
return None;
|
||||
}
|
||||
|
||||
if current_scope_idx == 0 {
|
||||
let mut globals_map = self.globals.borrow_mut();
|
||||
let addr = if let Some((idx, existing_id)) = globals_map.get(name) {
|
||||
if *existing_id != identity {
|
||||
diag.push_error(
|
||||
format!("Variable '{}' is already defined in global scope.", name.name),
|
||||
None,
|
||||
);
|
||||
return None;
|
||||
}
|
||||
Address::Global(*idx)
|
||||
} else {
|
||||
let idx = GlobalIdx(globals_map.len() as u32);
|
||||
globals_map.insert(name.clone(), (idx, identity.clone()));
|
||||
Address::Global(idx)
|
||||
};
|
||||
|
||||
let current_scope = self.functions[0].scopes.last_mut().unwrap();
|
||||
current_scope.locals.insert(
|
||||
name.clone(),
|
||||
LocalInfo {
|
||||
addr,
|
||||
identity,
|
||||
_ty: StaticType::Any,
|
||||
},
|
||||
);
|
||||
return Some(addr);
|
||||
}
|
||||
}
|
||||
|
||||
let current_fn = self.functions.last_mut().unwrap();
|
||||
match current_fn.define_variable(name, identity, &self.globals) {
|
||||
match current_fn.define_variable(name, identity) {
|
||||
Ok(addr) => Some(addr),
|
||||
Err(e) => {
|
||||
diag.push_error(e, None);
|
||||
@@ -221,7 +207,12 @@ impl Binder {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn bind(&mut self, node: &Node<UntypedKind>, ctx: ExprContext, diag: &mut Diagnostics) -> BoundNode {
|
||||
pub fn bind(
|
||||
&mut self,
|
||||
node: &Node<UntypedKind>,
|
||||
ctx: ExprContext,
|
||||
diag: &mut Diagnostics,
|
||||
) -> BoundNode {
|
||||
match &node.kind {
|
||||
UntypedKind::Nop => self.make_node(node.identity.clone(), BoundKind::Nop),
|
||||
UntypedKind::Constant(v) => {
|
||||
@@ -281,11 +272,10 @@ impl Binder {
|
||||
Some(node.identity.clone()),
|
||||
);
|
||||
}
|
||||
// Special case: Single identifier (to support recursion)
|
||||
if let UntypedKind::Identifier(ref name) = target.kind {
|
||||
let addr_opt = self.declare_variable(
|
||||
name,
|
||||
node.identity.clone(), // Identity of the Def node
|
||||
node.identity.clone(),
|
||||
crate::ast::compiler::bound_nodes::DeclarationKind::Variable,
|
||||
diag,
|
||||
);
|
||||
@@ -299,16 +289,13 @@ impl Binder {
|
||||
addr,
|
||||
kind: crate::ast::compiler::bound_nodes::DeclarationKind::Variable,
|
||||
value: Rc::new(val_node),
|
||||
captured_by: Vec::new(), // Will be filled by post-pass
|
||||
captured_by: Vec::new(),
|
||||
},
|
||||
)
|
||||
} else {
|
||||
self.make_node(node.identity.clone(), BoundKind::Error)
|
||||
}
|
||||
} else {
|
||||
// Complex Destructuring Pattern
|
||||
// NOTE: Destructuring definitions are NOT recursive by default
|
||||
// (the variables are only available AFTER the definition)
|
||||
let val_node = self.bind(value, ExprContext::Expression, diag);
|
||||
let target_node = self.bind_pattern(target, DeclarationKind::Variable, diag);
|
||||
|
||||
@@ -354,7 +341,8 @@ impl Binder {
|
||||
for input in inputs {
|
||||
bound_inputs.push(Rc::new(self.bind(input, ExprContext::Expression, diag)));
|
||||
}
|
||||
let bound_lambda = Rc::new(self.bind(lambda.as_ref(), ExprContext::Expression, diag));
|
||||
let bound_lambda =
|
||||
Rc::new(self.bind(lambda.as_ref(), ExprContext::Expression, diag));
|
||||
self.make_node(
|
||||
node.identity.clone(),
|
||||
BoundKind::Pipe {
|
||||
@@ -368,17 +356,12 @@ impl Binder {
|
||||
UntypedKind::Lambda { params, body } => {
|
||||
let identity = node.identity.clone();
|
||||
self.functions
|
||||
.push(FunctionCompiler::new(-1, identity.clone(), false));
|
||||
.push(FunctionCompiler::new(identity.clone()));
|
||||
|
||||
// 1. Bind the parameter pattern/tuple
|
||||
let params_bound = self.bind_pattern(params, DeclarationKind::Parameter, diag);
|
||||
|
||||
// 2. Bind the body
|
||||
let body_bound = self.bind(body, ExprContext::Expression, diag);
|
||||
|
||||
let compiled_fn = self.functions.pop().unwrap();
|
||||
|
||||
// 3. Static optimization: count total parameters needed in flat argument list
|
||||
fn count_params(node: &BoundNode) -> Option<u32> {
|
||||
match &node.kind {
|
||||
BoundKind::Define {
|
||||
@@ -444,7 +427,11 @@ impl Binder {
|
||||
self.functions.last_mut().unwrap().push_scope();
|
||||
let mut bound_exprs = Vec::new();
|
||||
for (i, expr) in exprs.iter().enumerate() {
|
||||
let expr_ctx = if i == exprs.len() - 1 { ctx } else { ExprContext::Statement };
|
||||
let expr_ctx = if i == exprs.len() - 1 {
|
||||
ctx
|
||||
} else {
|
||||
ExprContext::Statement
|
||||
};
|
||||
bound_exprs.push(Rc::new(self.bind(expr, expr_ctx, diag)));
|
||||
}
|
||||
self.functions.last_mut().unwrap().pop_scope();
|
||||
@@ -517,7 +504,13 @@ impl Binder {
|
||||
| UntypedKind::Placeholder(_)
|
||||
| UntypedKind::Splice(_)
|
||||
| UntypedKind::MacroDecl { .. } => {
|
||||
diag.push_error(format!("Macro construct {:?} found in Binder. Macros must be expanded before binding.", node.kind), Some(node.identity.clone()));
|
||||
diag.push_error(
|
||||
format!(
|
||||
"Macro construct {:?} found in Binder. Macros must be expanded before binding.",
|
||||
node.kind
|
||||
),
|
||||
Some(node.identity.clone()),
|
||||
);
|
||||
self.make_node(node.identity.clone(), BoundKind::Error)
|
||||
}
|
||||
|
||||
@@ -544,22 +537,30 @@ impl Binder {
|
||||
) -> Option<Address> {
|
||||
let current_fn_idx = self.functions.len() - 1;
|
||||
|
||||
// 1. Try local in current function
|
||||
if let Some(info) = self.functions[current_fn_idx].resolve_local(sym) {
|
||||
if let Some((info, _)) = self.functions[current_fn_idx].resolve_local(sym) {
|
||||
return Some(info.addr);
|
||||
}
|
||||
|
||||
// 2. Try enclosing scopes (capture chain)
|
||||
for i in (0..current_fn_idx).rev() {
|
||||
if let Some(info) = self.functions[i].resolve_local(sym) {
|
||||
// If the resolved address is Global, we don't need to capture it as an upvalue
|
||||
if let Some((info, scope_idx)) = self.functions[i].resolve_local(sym) {
|
||||
let is_frozen_root = i == 0 && (scope_idx as i32) <= self.fixed_scope_idx;
|
||||
|
||||
if let Address::Global(_) = info.addr {
|
||||
return Some(info.addr);
|
||||
}
|
||||
|
||||
|
||||
let mut addr = info.addr;
|
||||
|
||||
// Record the capture for each lambda level in between
|
||||
if is_frozen_root {
|
||||
if let Address::Local(slot) = addr {
|
||||
addr = Address::Global(GlobalIdx(slot.0));
|
||||
}
|
||||
}
|
||||
|
||||
if let Address::Global(_) = addr {
|
||||
return Some(addr);
|
||||
}
|
||||
|
||||
for k in (i + 1)..=current_fn_idx {
|
||||
let lambda_id = self.functions[k].identity.clone();
|
||||
self.capture_map
|
||||
@@ -572,13 +573,11 @@ impl Binder {
|
||||
}
|
||||
}
|
||||
|
||||
// 3. Try Global
|
||||
let globals = self.globals.borrow();
|
||||
if let Some((idx, _)) = globals.get(sym) {
|
||||
return Some(Address::Global(*idx));
|
||||
}
|
||||
|
||||
// 4. Global Fallback
|
||||
if sym.context.is_some() {
|
||||
let fallback_sym = Symbol {
|
||||
name: sym.name.clone(),
|
||||
@@ -612,7 +611,7 @@ impl Binder {
|
||||
addr,
|
||||
kind,
|
||||
value: Rc::new(self.make_node(node.identity.clone(), BoundKind::Nop)),
|
||||
captured_by: Vec::new(), // Filled by post-pass
|
||||
captured_by: Vec::new(),
|
||||
},
|
||||
)
|
||||
} else {
|
||||
@@ -699,7 +698,6 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn test_upvalue_capture_sets_is_boxed() {
|
||||
// Wrap in a lambda to ensure 'x' is a local variable, not a global
|
||||
let source = "(fn [] (do (def x 10) (def f (fn [] x)) x))";
|
||||
let mut parser = Parser::new(source);
|
||||
let untyped = parser.parse_expression();
|
||||
@@ -708,7 +706,6 @@ mod tests {
|
||||
let mut diagnostics = Diagnostics::new();
|
||||
let (bound, captures) = Binder::bind_root(globals, 0, &untyped, &mut diagnostics).unwrap();
|
||||
|
||||
// Structure: Lambda -> Block -> [ Define(x), Define(f), Get(x) ]
|
||||
if let BoundKind::Lambda { body, .. } = &bound.kind {
|
||||
if let BoundKind::Block { exprs } = &body.kind {
|
||||
let x_decl = &exprs[0];
|
||||
@@ -716,19 +713,16 @@ mod tests {
|
||||
assert!(matches!(addr, Address::Local(_)));
|
||||
assert!(
|
||||
captures.contains_key(&x_decl.identity),
|
||||
"Variable 'x' should have capturers because it is used in lambda 'f'"
|
||||
"Variable 'x' should have capturers"
|
||||
);
|
||||
} else {
|
||||
panic!(
|
||||
"First expression in block should be Define, got {:?}",
|
||||
x_decl.kind
|
||||
);
|
||||
panic!("First expression should be Define");
|
||||
}
|
||||
} else {
|
||||
panic!("Lambda body should be a Block, got {:?}", body.kind);
|
||||
panic!("Lambda body should be a Block");
|
||||
}
|
||||
} else {
|
||||
panic!("Root should be a Lambda, got {:?}", bound.kind);
|
||||
panic!("Root should be a Lambda");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -780,13 +774,11 @@ mod tests {
|
||||
fn test_repro_global_redefinition() {
|
||||
let globals = Rc::new(RefCell::new(HashMap::new()));
|
||||
|
||||
// First run: defines 'x'
|
||||
let source1 = "(def x 1) 1";
|
||||
let untyped1 = Parser::new(source1).parse_expression();
|
||||
let mut diagnostics = Diagnostics::new();
|
||||
assert!(Binder::bind_root(globals.clone(), 0, &untyped1, &mut diagnostics).is_ok());
|
||||
|
||||
// Second run: attempts to redefine 'x' in the same global environment
|
||||
let source2 = "(def x 2) 2";
|
||||
let untyped2 = Parser::new(source2).parse_expression();
|
||||
let mut diagnostics2 = Diagnostics::new();
|
||||
|
||||
Reference in New Issue
Block a user