From 449df13c9cbe70298675f8426b3aa70215997b0c Mon Sep 17 00:00:00 2001 From: Brummel Date: Sun, 31 May 2026 18:01:24 +0200 Subject: [PATCH] fix(check): preserve fn-type modes through Subst::apply MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Subst::apply` rebuilt every `Type::Fn` with `param_modes: vec![]` and `ret_mode: Implicit`, discarding the modes the surrounding type carried. That was harmless while nothing downstream of inference read fn-type modes — but it is a latent under-specification: substitution is meant to be type-preserving, and a fn type's modes are part of its identity (an `(own T) -> (own U)` contract is not the same contract as its borrow-returning sibling). Modes are positional over `params`, and substitution remaps each param type element-wise without changing the arity, so the mode lists stay aligned — preserving them is a clone, not a re-derivation. The typed-MIR boundary (milestone 0060) makes this load-bearing: a node type synthesised by `lower_to_mir::synth_pure` ends with `subst.apply`, and the codegen drop path reads `ret_mode == Own` off a call's callee node to decide RC trackability. With the modes stripped here, an Own-returning call read back `Implicit`, nothing was trackable, and a heap-allocated binder leaked at scope close. Preserving the modes restores that signal at the source rather than re-deriving it in codegen (which is exactly the re-derivation the milestone exists to delete). Safe by construction: `unify`'s Fn arm destructures modes with `..` and compares only params/ret/effects, so carrying modes through `apply` cannot change unification; and `Type`'s custom `PartialEq` already treats `Implicit` and `Own` as interchangeable (only `Borrow` is distinct), so equality outcomes are unaffected except where preserving a `Borrow` is the more correct answer anyway. Whole `ailang-check` suite stays green. RED pin: crates/ailang-check/tests/subst_preserves_modes.rs. --- crates/ailang-check/src/lib.rs | 15 ++++- .../tests/subst_preserves_modes.rs | 56 +++++++++++++++++++ 2 files changed, 68 insertions(+), 3 deletions(-) create mode 100644 crates/ailang-check/tests/subst_preserves_modes.rs diff --git a/crates/ailang-check/src/lib.rs b/crates/ailang-check/src/lib.rs index 0819691..72649f3 100644 --- a/crates/ailang-check/src/lib.rs +++ b/crates/ailang-check/src/lib.rs @@ -90,12 +90,21 @@ impl Subst { name: name.clone(), args: args.iter().map(|a| self.apply(a)).collect(), }, - Type::Fn { params, ret, effects, .. } => Type::Fn { + // Modes are part of a fn type's identity (an `Own`-returning + // contract differs from a `Borrow`-returning one) and are + // positional over `params` — substitution remaps each param + // type element-wise without changing the arity, so the mode + // lists stay aligned. Preserve them: substitution must be + // type-preserving, and the typed-MIR boundary reads `ret_mode` + // off the substituted node type to decide RC drop. `unify` + // ignores modes (its Fn arm destructures them with `..`), so + // carrying them through here cannot change unification. + Type::Fn { params, param_modes, ret, ret_mode, effects } => Type::Fn { params: params.iter().map(|p| self.apply(p)).collect(), ret: Box::new(self.apply(ret)), effects: effects.clone(), - param_modes: vec![], - ret_mode: ParamMode::Implicit, + param_modes: param_modes.clone(), + ret_mode: ret_mode.clone(), }, Type::Forall { vars, constraints, body } => Type::Forall { vars: vars.clone(), diff --git a/crates/ailang-check/tests/subst_preserves_modes.rs b/crates/ailang-check/tests/subst_preserves_modes.rs new file mode 100644 index 0000000..b180e64 --- /dev/null +++ b/crates/ailang-check/tests/subst_preserves_modes.rs @@ -0,0 +1,56 @@ +//! mir.1b producer pin: `Subst::apply` must preserve fn-type modes. +//! +//! The typed-MIR boundary carries the modes `check` proved. The +//! codegen drop path reads `ret_mode == Own` off a call's callee node +//! to decide RC trackability — an Own-returning call hands a fresh heap +//! allocation to the caller's frame, so the binder needs an +//! `ailang_rc_dec` at scope close. `Subst::apply` is the last transform +//! `lower_to_mir::synth_pure` runs on every node type, so an `apply` +//! that rebuilds `Type::Fn` with empty `param_modes` / `Implicit` +//! `ret_mode` silently erases the Own signal: the callee node reads +//! back `Implicit`, nothing is trackable, and a heap-`Str` binder leaks +//! (`frees=0`). This pins that `apply` is mode-preserving — a +//! type-preserving substitution must not drop the modes that are part +//! of a fn type's identity. + +use ailang_core::ast::ParamMode; +use ailang_core::Type; +use ailang_check::Subst; + +#[test] +fn subst_apply_preserves_fn_ret_and_param_modes() { + // `(borrow Int) -> (own Str)` — an identity substitution must + // round-trip both the borrow param-mode and the Own ret-mode + // unchanged. (`Own` is the mode the drop path actually reads; + // `Borrow` is the one the custom `Type` equality keeps distinct, + // so it doubles as a witness that a stripped result is observable.) + let fn_ty = Type::Fn { + params: vec![Type::int()], + param_modes: vec![ParamMode::Borrow], + ret: Box::new(Type::str_()), + ret_mode: ParamMode::Own, + effects: vec![], + }; + + let applied = Subst::default().apply(&fn_ty); + + let Type::Fn { + param_modes, + ret_mode, + .. + } = applied + else { + panic!("apply of a Fn type is a Fn type"); + }; + + assert!( + matches!(ret_mode, ParamMode::Own), + "ret_mode Own must survive Subst::apply (the drop path reads it \ + for RC trackability), got {ret_mode:?}" + ); + assert_eq!( + param_modes.first(), + Some(&ParamMode::Borrow), + "param_modes must survive Subst::apply, got {param_modes:?}" + ); +}