feat(mir): Phase 285 P2.1 - KeepAlive instruction for weak ref semantics
Add KeepAlive instruction to fix hidden root problem where x = null
doesn't properly drop the strong reference.
Key changes:
- Add KeepAlive { values, drop_after } instruction to MIR
- Emit KeepAlive[drop=true] in build_assignment() before variable overwrite
- Emit KeepAlive[drop=false] in pop_lexical_scope() at scope end
- VM handler: when drop_after=true, remove ALL ValueIds pointing to
the same Arc (handles SSA Copy chains)
Test results:
- weak_upgrade_fail: exit 1 ✅ (weak_to_strong returns null after x=null)
- weak_basic: exit 2 ✅ (weak_to_strong succeeds while x alive)
- quick smoke: 154/154 PASS ✅
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@ -4,9 +4,7 @@
|
||||
// Test: weak_to_strong() fails after explicit drop (x = null)
|
||||
// Note: Uses explicit drop instead of block scope (block scope drop conformance is separate task)
|
||||
// Expected: exit 1 (weak_to_strong returns null as expected)
|
||||
// KNOWN ISSUE: Currently returns exit 0 (hidden root keeps strong ref alive)
|
||||
// VM: SKIP (Phase 285 P2.1 investigation required)
|
||||
// LLVM: SKIP (same issue expected)
|
||||
// Phase 285 P2.1: FIXED with KeepAlive instruction
|
||||
|
||||
box SomeBox {
|
||||
x
|
||||
|
||||
@ -133,8 +133,53 @@ impl MirInterpreter {
|
||||
MirInstruction::BarrierRead { .. }
|
||||
| MirInstruction::BarrierWrite { .. }
|
||||
| MirInstruction::Barrier { .. }
|
||||
| MirInstruction::Safepoint
|
||||
| MirInstruction::Nop => {}
|
||||
| MirInstruction::Safepoint => {}
|
||||
MirInstruction::KeepAlive { values, drop_after } => {
|
||||
// Phase 285 P2.1: Handle KeepAlive based on drop_after flag
|
||||
// - drop_after=true: Release values (for variable overwrite, enables weak ref failure)
|
||||
// - drop_after=false: Just keep alive (for scope end, values may be needed for PHI)
|
||||
if *drop_after {
|
||||
// IMPORTANT: Due to SSA Copy instructions, a single Box may have multiple
|
||||
// ValueIds pointing to it (e.g., %5 = NewBox, %6 = Copy %5).
|
||||
// We need to find and remove ALL ValueIds that point to the same Arc.
|
||||
use std::sync::Arc;
|
||||
use super::super::vm_types::VMValue;
|
||||
|
||||
// Collect raw pointers of Arcs being released
|
||||
let mut arc_ptrs: Vec<*const dyn crate::box_trait::NyashBox> = Vec::new();
|
||||
for v in values {
|
||||
if let Some(VMValue::BoxRef(arc)) = self.regs.get(v) {
|
||||
arc_ptrs.push(Arc::as_ptr(arc));
|
||||
}
|
||||
}
|
||||
|
||||
// Remove the specified values first
|
||||
for v in values {
|
||||
self.regs.remove(v);
|
||||
}
|
||||
|
||||
// Find and remove ALL other ValueIds that point to the same Arcs
|
||||
if !arc_ptrs.is_empty() {
|
||||
let to_remove: Vec<crate::mir::ValueId> = self.regs
|
||||
.iter()
|
||||
.filter_map(|(vid, val)| {
|
||||
if let VMValue::BoxRef(arc) = val {
|
||||
let ptr = Arc::as_ptr(arc);
|
||||
if arc_ptrs.contains(&ptr) {
|
||||
return Some(*vid);
|
||||
}
|
||||
}
|
||||
None
|
||||
})
|
||||
.collect();
|
||||
for vid in to_remove {
|
||||
self.regs.remove(&vid);
|
||||
}
|
||||
}
|
||||
}
|
||||
// If drop_after=false, do nothing (values stay alive)
|
||||
}
|
||||
MirInstruction::Nop => {}
|
||||
other => {
|
||||
return Err(self.err_invalid(format!(
|
||||
"MIR interp: unimplemented instruction: {:?}",
|
||||
|
||||
@ -601,6 +601,18 @@ impl MirBuilder {
|
||||
// causing stale references after LoopForm transformation renumbers blocks.
|
||||
// Result: VM would try to read undefined ValueIds (e.g., ValueId(270) at bb303).
|
||||
if !var_name.starts_with("__pin$") {
|
||||
// Phase 285 P2.1: Emit KeepAlive for previous value BEFORE updating variable_map
|
||||
// This ensures "alive until overwrite, then dropped" semantics
|
||||
// ⚠️ Termination guard: don't emit after return/throw
|
||||
if !self.is_current_block_terminated() {
|
||||
if let Some(prev) = self.variable_ctx.variable_map.get(&var_name).copied() {
|
||||
let _ = self.emit_instruction(MirInstruction::KeepAlive {
|
||||
values: vec![prev],
|
||||
drop_after: true, // Overwrite: drop old value after this
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// In SSA form, each assignment creates a new value
|
||||
self.variable_ctx
|
||||
.variable_map
|
||||
|
||||
@ -146,6 +146,8 @@ impl JoinIrIdRemapper {
|
||||
Print { value, .. } => vec![*value],
|
||||
Debug { value, .. } => vec![*value],
|
||||
DebugLog { values, .. } => values.clone(),
|
||||
// Phase 285 P2.1: KeepAlive collects all values
|
||||
KeepAlive { values, .. } => values.clone(),
|
||||
Throw { exception, .. } => vec![*exception],
|
||||
Catch {
|
||||
exception_value, ..
|
||||
@ -317,6 +319,11 @@ impl JoinIrIdRemapper {
|
||||
message: message.clone(),
|
||||
values: values.iter().map(|&v| remap(v)).collect(),
|
||||
},
|
||||
// Phase 285 P2.1: KeepAlive remaps all values
|
||||
KeepAlive { values, drop_after } => KeepAlive {
|
||||
values: values.iter().map(|&v| remap(v)).collect(),
|
||||
drop_after: *drop_after,
|
||||
},
|
||||
Throw { exception, effects } => Throw {
|
||||
exception: remap(*exception),
|
||||
effects: *effects,
|
||||
|
||||
@ -47,6 +47,24 @@ impl super::super::MirBuilder {
|
||||
.pop_lexical_scope()
|
||||
.expect("COMPILER BUG: pop_lexical_scope without push_lexical_scope");
|
||||
|
||||
// Phase 285 P2.1: Emit KeepAlive for all declared variables in this scope
|
||||
// This keeps strong refs alive until scope end (language scope semantics)
|
||||
// ⚠️ Termination guard: don't emit after return/throw
|
||||
if !self.is_current_block_terminated() {
|
||||
let keepalive_values: Vec<crate::mir::ValueId> = frame
|
||||
.declared
|
||||
.iter()
|
||||
.filter_map(|name| self.variable_ctx.variable_map.get(name).copied())
|
||||
.collect();
|
||||
|
||||
if !keepalive_values.is_empty() {
|
||||
let _ = self.emit_instruction(crate::mir::MirInstruction::KeepAlive {
|
||||
values: keepalive_values,
|
||||
drop_after: false, // Scope-end: don't drop, values may be needed for PHI
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Restore ValueId mappings
|
||||
for (name, previous) in frame.restore {
|
||||
match previous {
|
||||
|
||||
@ -220,6 +220,17 @@ pub enum MirInstruction {
|
||||
values: Vec<ValueId>,
|
||||
},
|
||||
|
||||
/// Phase 285 P2.1: Keep values alive until scope end
|
||||
/// `keepalive %v1 %v2 ...`
|
||||
/// Prevents DCE from removing values and maintains strong refs for language scope semantics.
|
||||
/// - drop_after=true: Release values after this instruction (for variable overwrite)
|
||||
/// - drop_after=false: Just keep alive (for scope end, values may be needed for PHI)
|
||||
KeepAlive {
|
||||
values: Vec<ValueId>,
|
||||
/// If true, VM should drop the values after processing this instruction
|
||||
drop_after: bool,
|
||||
},
|
||||
|
||||
/// Print instruction for console output
|
||||
/// `print %value`
|
||||
Print { value: ValueId, effects: EffectMask },
|
||||
|
||||
@ -138,6 +138,17 @@ impl fmt::Display for MirInstruction {
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
// Phase 285 P2.1: KeepAlive for language scope semantics
|
||||
MirInstruction::KeepAlive { values, drop_after } => {
|
||||
write!(f, "keepalive")?;
|
||||
if *drop_after {
|
||||
write!(f, "[drop]")?;
|
||||
}
|
||||
for v in values {
|
||||
write!(f, " {}", v)?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
_ => write!(f, "{:?}", self), // Fallback for other instructions
|
||||
}
|
||||
}
|
||||
|
||||
@ -28,6 +28,7 @@ impl MirInstruction {
|
||||
| MirInstruction::Phi { .. }
|
||||
| MirInstruction::TypeCheck { .. }
|
||||
| MirInstruction::Select { .. }
|
||||
| MirInstruction::KeepAlive { .. }
|
||||
| MirInstruction::Nop => EffectMask::PURE,
|
||||
|
||||
// Memory operations
|
||||
@ -131,6 +132,7 @@ impl MirInstruction {
|
||||
| MirInstruction::ArraySet { .. }
|
||||
| MirInstruction::Debug { .. }
|
||||
| MirInstruction::DebugLog { .. }
|
||||
| MirInstruction::KeepAlive { .. }
|
||||
| MirInstruction::Print { .. }
|
||||
| MirInstruction::Throw { .. }
|
||||
| MirInstruction::RefSet { .. }
|
||||
@ -235,6 +237,9 @@ impl MirInstruction {
|
||||
} => vec![*array, *index, *value],
|
||||
MirInstruction::DebugLog { values, .. } => values.clone(),
|
||||
|
||||
// Phase 285 P2.1: KeepAlive uses all values (keeps them alive for liveness analysis)
|
||||
MirInstruction::KeepAlive { values, .. } => values.clone(),
|
||||
|
||||
// Phase 256 P1.5: Select instruction uses cond, then_val, else_val
|
||||
MirInstruction::Select {
|
||||
cond,
|
||||
|
||||
@ -295,6 +295,19 @@ pub fn format_instruction(
|
||||
s
|
||||
}
|
||||
|
||||
// Phase 285 P2.1: KeepAlive for language scope semantics
|
||||
MirInstruction::KeepAlive { values, drop_after } => {
|
||||
let mut s = "keepalive".to_string();
|
||||
if *drop_after {
|
||||
s.push_str("[drop]");
|
||||
}
|
||||
for v in values {
|
||||
s.push(' ');
|
||||
s.push_str(&format!("{}", v));
|
||||
}
|
||||
s
|
||||
}
|
||||
|
||||
MirInstruction::Cast {
|
||||
dst,
|
||||
value,
|
||||
|
||||
@ -85,6 +85,8 @@ impl<'m> MirQuery for MirQueryBox<'m> {
|
||||
NewBox { args, .. } => args.clone(),
|
||||
Debug { value, .. } | Print { value, .. } => vec![*value],
|
||||
DebugLog { values, .. } => values.clone(),
|
||||
// Phase 285 P2.1: KeepAlive reads all values
|
||||
KeepAlive { values, .. } => values.clone(),
|
||||
Throw { exception, .. } => vec![*exception],
|
||||
Catch { .. } => Vec::new(),
|
||||
NewClosure { captures, me, .. } => {
|
||||
|
||||
@ -3,12 +3,24 @@
|
||||
# Purpose: Verify weak_to_strong() fails (returns null) after explicit drop
|
||||
# Uses phase285_p2_weak_upgrade_fail_min.hako
|
||||
#
|
||||
# SKIP: Known issue - hidden root keeps strong ref alive even after x=null (same as VM)
|
||||
# Investigation required in Phase 285 P2.1 to identify root holding location
|
||||
# Phase 285 P2.1: FIXED with KeepAlive instruction
|
||||
# - KeepAlive[drop] in build_assignment() emits before variable overwrite
|
||||
# - Ensures "alive until overwrite, then dropped" semantics
|
||||
|
||||
source "$(dirname "$0")/../../../lib/test_runner.sh"
|
||||
export SMOKES_USE_PYVM=0
|
||||
require_env || exit 2
|
||||
HAKO_FILE="apps/tests/phase285_p2_weak_upgrade_fail_min.hako"
|
||||
BACKEND="llvm"
|
||||
|
||||
test_skip "phase285_p2_weak_upgrade_fail_llvm" "Hidden root issue: strong ref not dropped after x=null (Phase 285 P2.1 investigation required, same as VM)"
|
||||
exit 0
|
||||
# Test expects exit 1 (weak_to_strong returns null after x=null)
|
||||
NYASH_LLVM_USE_HARNESS=1 ./target/release/hakorune --backend "$BACKEND" "$HAKO_FILE" >/dev/null 2>&1
|
||||
RC=$?
|
||||
|
||||
if [[ "$RC" -eq 1 ]]; then
|
||||
echo "✅ PASS: phase285_p2_weak_upgrade_fail_llvm"
|
||||
exit 0
|
||||
else
|
||||
echo "❌ FAIL: phase285_p2_weak_upgrade_fail_llvm"
|
||||
echo "Expected exit 1 (weak_to_strong null), got $RC"
|
||||
echo "---Full output (last 40 lines):---"
|
||||
NYASH_LLVM_USE_HARNESS=1 ./target/release/hakorune --backend "$BACKEND" "$HAKO_FILE" 2>&1 | tail -n 40
|
||||
exit 1
|
||||
fi
|
||||
|
||||
@ -3,12 +3,24 @@
|
||||
# Purpose: Verify weak_to_strong() fails (returns null) after explicit drop
|
||||
# Uses phase285_p2_weak_upgrade_fail_min.hako
|
||||
#
|
||||
# SKIP: Known issue - hidden root keeps strong ref alive even after x=null
|
||||
# Investigation required in Phase 285 P2.1 to identify root holding location
|
||||
# Phase 285 P2.1: FIXED with KeepAlive instruction
|
||||
# - KeepAlive[drop] in build_assignment() emits before variable overwrite
|
||||
# - Ensures "alive until overwrite, then dropped" semantics
|
||||
|
||||
source "$(dirname "$0")/../../../lib/test_runner.sh"
|
||||
export SMOKES_USE_PYVM=0
|
||||
require_env || exit 2
|
||||
HAKO_FILE="apps/tests/phase285_p2_weak_upgrade_fail_min.hako"
|
||||
BACKEND="vm"
|
||||
|
||||
test_skip "phase285_p2_weak_upgrade_fail_vm" "Hidden root issue: strong ref not dropped after x=null (Phase 285 P2.1 investigation required)"
|
||||
exit 0
|
||||
# Test expects exit 1 (weak_to_strong returns null after x=null)
|
||||
./target/release/hakorune --backend "$BACKEND" "$HAKO_FILE" >/dev/null 2>&1
|
||||
RC=$?
|
||||
|
||||
if [[ "$RC" -eq 1 ]]; then
|
||||
echo "✅ PASS: phase285_p2_weak_upgrade_fail_vm"
|
||||
exit 0
|
||||
else
|
||||
echo "❌ FAIL: phase285_p2_weak_upgrade_fail_vm"
|
||||
echo "Expected exit 1 (weak_to_strong null), got $RC"
|
||||
echo "---Full output (last 40 lines):---"
|
||||
./target/release/hakorune --backend "$BACKEND" "$HAKO_FILE" 2>&1 | tail -n 40
|
||||
exit 1
|
||||
fi
|
||||
|
||||
Reference in New Issue
Block a user