From bff5d2d237a78170b6763f62f2bce88ba16c4f51 Mon Sep 17 00:00:00 2001 From: brockelmore <31553173+brockelmore@users.noreply.github.com> Date: Sat, 7 Mar 2026 22:09:50 -0800 Subject: [PATCH] fix: resolve name collision between contract and free functions When a contract function and free function share the same name, the lookup previously found the contract function first, causing infinite recursion during IR lowering. Now free functions are checked first in both lower_function_call and lower_internal_function_body. Also makes internal function params memory-backed (LetBind) so parameter reassignment emits VarStore instead of leaving orphan values on the stack. Includes minimal reproduction test (test_name_collision.edge). Co-Authored-By: Claude Opus 4.6 --- crates/e2e/tests/suites/examples.rs | 13 +++++++ crates/ir/src/to_egglog/calls.rs | 23 ++++++----- crates/ir/src/to_egglog/function.rs | 52 ++++++++++++++++--------- examples/tests/test_name_collision.edge | 12 ++++++ 4 files changed, 72 insertions(+), 28 deletions(-) create mode 100644 examples/tests/test_name_collision.edge diff --git a/crates/e2e/tests/suites/examples.rs b/crates/e2e/tests/suites/examples.rs index 6e86b25..d6e9f53 100644 --- a/crates/e2e/tests/suites/examples.rs +++ b/crates/e2e/tests/suites/examples.rs @@ -217,6 +217,19 @@ fn test_traits_contract_compiles() { assert!(!bc.is_empty(), "test_traits.edge produced empty bytecode"); } +// ============================================================================= +// regression tests +// ============================================================================= + +#[test] +fn test_name_collision_compiles() { + let bc = compile_contract("examples/tests/test_name_collision.edge"); + assert!( + !bc.is_empty(), + "test_name_collision.edge produced empty bytecode" + ); +} + // ============================================================================= // utils/ // ============================================================================= diff --git a/crates/ir/src/to_egglog/calls.rs b/crates/ir/src/to_egglog/calls.rs index bea551e..cbb996c 100644 --- a/crates/ir/src/to_egglog/calls.rs +++ b/crates/ir/src/to_egglog/calls.rs @@ -110,24 +110,27 @@ impl AstToEgglog { return self.inline_function_call(&info.params, &info.body, args); } - // Check contract functions — emit Call (not inline) - if let Some((name, params, _body)) = self - .contract_functions + // Check non-comptime free functions — emit Call (not inline). + // Free functions are checked before contract functions so that a + // contract function calling a same-named free function resolves to + // the free function (not to itself, which would infinite-recurse). + if let Some(info) = self + .free_fn_bodies .iter() - .find(|(name, _, _)| *name == fn_name) + .find(|f| f.name == fn_name && !f.is_comptime) .cloned() { - return self.emit_call(&name, ¶ms, args); + return self.emit_call(&info.name, &info.params, args); } - // Check non-comptime free functions — emit Call (not inline) - if let Some(info) = self - .free_fn_bodies + // Check contract functions — emit Call (not inline) + if let Some((name, params, _body)) = self + .contract_functions .iter() - .find(|f| f.name == fn_name && !f.is_comptime) + .find(|(name, _, _)| *name == fn_name) .cloned() { - return self.emit_call(&info.name, &info.params, args); + return self.emit_call(&name, ¶ms, args); } // Check generic function templates diff --git a/crates/ir/src/to_egglog/function.rs b/crates/ir/src/to_egglog/function.rs index 05fbaa5..4cc46da 100644 --- a/crates/ir/src/to_egglog/function.rs +++ b/crates/ir/src/to_egglog/function.rs @@ -436,7 +436,7 @@ impl AstToEgglog { /// Lower an internal function body once as a Function node. /// Uses `inline_depth` > 0 so `return` produces just the value (not `ReturnOp`). - /// Parameters are bound via Arg/Get so the body works as a standalone subroutine. + /// Parameters are memory-backed (LetBind) so reassignment emits VarStore. pub(crate) fn lower_internal_function_body( &mut self, name: &str, @@ -467,25 +467,31 @@ impl AstToEgglog { ); let out_ty = EvmType::Base(EvmBaseType::UIntT(256)); // TODO: derive from return type - // Bind parameters via Arg/Get + // Bind parameters as memory-backed variables so that reassignment + // (e.g. `x = x / 2`) emits VarStore (pushes nothing) instead of + // replacing the binding value (which pushes a value onto the stack + // and causes stack depth mismatches between if-branches). self.scopes.push(Scope::new()); let arg_expr = Rc::new(EvmExpr::Arg(in_ty.clone(), self.current_ctx.clone())); + let mut param_var_names = Vec::new(); for (i, (param_name, param_ty)) in params.iter().enumerate() { let ty = self.lower_type_sig(param_ty); - let param_val = if params.len() == 1 { - Rc::clone(&arg_expr) - } else { - ast_helpers::get(Rc::clone(&arg_expr), i) - }; + let var_name = format!("{name}__param_{param_name}"); let binding = VarBinding { - value: param_val, - location: DataLocation::Stack, + value: ast_helpers::var(var_name.clone()), + location: DataLocation::Memory, storage_slot: None, _ty: ty, - let_bind_name: None, + let_bind_name: Some(var_name.clone()), composite_type: None, composite_base: None, }; + let init = if params.len() == 1 { + Rc::clone(&arg_expr) + } else { + ast_helpers::get(Rc::clone(&arg_expr), i) + }; + param_var_names.push((var_name, init)); self.scopes .last_mut() .expect("scope stack empty") @@ -495,23 +501,33 @@ impl AstToEgglog { // Lower body with inline_depth > 0 so return produces value, not ReturnOp self.inline_depth += 1; - // Find the body from contract_functions or free_fn_bodies + // Find the body — prefer free_fn_bodies over contract_functions. + // If both contain a function with the same name, the free function + // is the intended callee (the contract function is the *caller* + // whose body we're currently lowering, so picking it would recurse). let body = self - .contract_functions + .free_fn_bodies .iter() - .find(|(n, _, _)| n == name) - .map(|(_, _, b)| b.clone()) + .find(|f| f.name == name) + .map(|f| f.body.clone()) .or_else(|| { - self.free_fn_bodies + self.contract_functions .iter() - .find(|f| f.name == name) - .map(|f| f.body.clone()) + .find(|(n, _, _)| n == name) + .map(|(_, _, b)| b.clone()) }) .ok_or_else(|| IrError::Unsupported(format!("internal function not found: {name}")))?; - let body_ir = self.lower_code_block(&body)?; + let mut body_ir = self.lower_code_block(&body)?; self.inline_depth -= 1; self.scopes.pop(); + // Wrap body in LetBind nodes for each parameter (innermost first). + // inline_depth was > 0 during lowering, so skip Drop (same as + // lower_code_block's pattern for inlined locals). + for (var_name, init) in param_var_names.into_iter().rev() { + body_ir = ast_helpers::let_bind(var_name, init, body_ir); + } + self.current_ctx = saved_ctx; self.current_state = saved_state; diff --git a/examples/tests/test_name_collision.edge b/examples/tests/test_name_collision.edge new file mode 100644 index 0000000..ca41c9b --- /dev/null +++ b/examples/tests/test_name_collision.edge @@ -0,0 +1,12 @@ +// Regression test: contract function and free function share the same name. +// This should produce a compile error about the name conflict. + +fn helper(x: u256) -> (u256) { + return x + 1; +} + +contract NameCollision { + pub fn helper(a: u256) -> (u256) { + return helper(a); + } +}