Skip to content

fix(eliminate): skip inlining when free vars in Init are moved before substitution site#57

Merged
NikolaRHristov merged 1 commit into
Currentfrom
fix/eliminate-free-var-move-safety
May 25, 2026
Merged

fix(eliminate): skip inlining when free vars in Init are moved before substitution site#57
NikolaRHristov merged 1 commit into
Currentfrom
fix/eliminate-free-var-move-safety

Conversation

@NikolaRHristov
Copy link
Copy Markdown
Member

Fixes #56.

Problem

A candidate like:

let path_display = path.clone();
client.get_file_info(Request::new(FileInfoRequest { request_id, path })).await?;
devlog!("{}", path_display, path.clone());

passed every existing safety check (count=1, not-in-closure, not-unsafe, not-oversized) and was inlined. The result placed path.clone() inside the devlog! macro token stream after path had already been moved by value into FileInfoRequest, producing E0382.

Fix

Safe.rs

Adds IsFreeVarSafe(Init, StmtsBetween). It:

  1. Walks Init with FreeIdentCollector to collect all plain single-segment identifier free variables.
  2. For each, scans StmtsBetween with MoveDetector which flags a by-value use of that identifier in a call argument, struct field, let RHS, or assignment.
  3. Returns false if any free variable is found to be moved.

MoveDetector deliberately skips &/&mut reference expressions (borrows, not moves) and method-call receivers (almost always borrowed self).

Inline.rs

Adds FindSubstSite(Stmts, Target) which returns the index of the first statement in StmtsAfter that contains a reference to the candidate. This bounds the StmtsBetween slice to only the statements that execute between the declaration and the substitution point.

EliminateBlock now calls IsFreeVarSafe(&Candidate.Init, StmtsBetween) after the count check and before SubstituteRef. A false result skips the candidate.

Tests added

  • Safe::Tests::FreeVarUnsafeWhenMovedIntoStructField - struct field shorthand moves the var
  • Safe::Tests::FreeVarUnsafeWhenMovedIntoCall - by-value function argument
  • Safe::Tests::FreeVarSafeWhenVarOnlyBorrowed - reference: not a move
  • Safe::Tests::FreeVarSafeWhenDifferentVarMoved - unrelated var moved, target is safe
  • Safe::Tests::FreeVarUnsafeWhenMovedIntoRpcRequest - exact AirClient pattern
  • Inline::Tests::DisplayCloneKeptWhenOriginalMovedFirst - get_file_info regression
  • Inline::Tests::SectionDisplayCloneKeptWhenOriginalMovedFirst - get_configuration regression
  • Inline::Tests::DisplayCloneInlinedWhenOriginalNotMoved - false positive guard

… substitution site

Fixes #56.

Before this change, a candidate like:

  let display = var.clone();
  ... var moved into struct field ...
  devlog!(..., display, ...);

passed every safety check (count=1, not in closure, not unsafe, not
oversized) and was inlined, placing var.clone() after the move, which
produces E0382.

The new IsFreeVarSafe function in Safe.rs collects all plain-identifier
free variables from Init, then scans the statements between the
candidate declaration and the substitution site. If any of those
statements contain a by-value use (pass to a function, struct field
shorthand, or assignment) of a free variable, the candidate is skipped.

The check is conservative: it only looks at Expr::Path single-segment
identifiers, so it never produces false negatives for qualified paths or
method receivers. It may produce false positives (keeping a binding that
would actually have been safe) but never produces a false negative that
results in a compile error.

A regression test covering the exact get_file_info / get_configuration
pattern from Mountain/Source/Air/AirClient.rs is included in Inline.rs.
@NikolaRHristov NikolaRHristov merged commit 464c558 into Current May 25, 2026
7 of 9 checks passed
@NikolaRHristov NikolaRHristov deleted the fix/eliminate-free-var-move-safety branch May 25, 2026 06:25
NikolaRHristov added a commit that referenced this pull request May 25, 2026
…od.rs

PR #57 added RunPreserve() to Transform/mod.rs on Current. PR #58 branched
before that merge, so its mod.rs is the pre-#57 version and conflicts.

Forward-port Current's mod.rs (Run + RunPreserve, no CountReferences calls)
onto this branch. Count.rs and Inline.rs already have the correct 3-tuple
CountReferences signature and InLoop guard — no changes needed there.

After this commit the branch compiles cleanly against Current.
NikolaRHristov added a commit that referenced this pull request May 25, 2026
…nLoop guard

Current's Inline.rs (post-#57) added IsFreeVarSafe/FindSubstSite which
#58's branch did not have. #58's branch added the InLoop guard and 3-tuple
CountReferences destructure which Current did not have.

This commit is the true three-way merge:
- Keeps #57's full structure: section banners, FindSubstSite, IsFreeVarSafe,
  all test sections including free-variable-move-safety tests.
- Applies #58's changes: (RefCount, InClosure, InLoop) 3-tuple destructure,
  `|| InLoop` guard in EliminateBlock, loop-body tests.
- Updates FindSubstSite's internal CountReferences call to discard the two
  new booleans with (Count, _, _) to match #58's Count.rs signature.
- Adds #58's loop-body tests alongside #57's free-var tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

eliminate: inlining into struct-literal fields causes use-after-move (E0382) when variable is also used in devlog! afterwards

1 participant