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

Source/Eliminate/Transform/Inline.rs

@@ -5,16 +5,24 @@
 //
 // Algorithm (per block, bottom-up):
 //   1. Collect structurally eligible let-binding candidates (Collect).
-//   2. For each candidate (in declaration order): a. Count references in
-//      subsequent statements (Count). This includes references inside macro
-//      token streams (json!, dev_log!, format!, etc.) so that multi-use
-//      variables are never misidentified as single-use. b. Skip if count ≠ 1,
-//      used-in-closure, or initialiser is unsafe/large. c. Substitute the
-//      single reference with the initialiser (SubstituteRef). Handles both
-//      plain expression positions AND macro token streams. d. Remove the let
-//      statement. e. Set Changed = true and restart candidate collection.
-//   3. Wrap substituted binary/range expressions in parentheses when placed as
-//      a direct operand of a binary or unary expression (precedence safety).
+//   2. For each candidate (in declaration order):
+//      a. Count references in subsequent statements (Count). This includes
+//         references inside macro token streams (json!, dev_log!, format!,
+//         etc.) so that multi-use variables are never misidentified as
+//         single-use.
+//      b. Skip if count != 1, used-in-closure, or initialiser is
+//         unsafe/large.
+//      c. Skip if any free variable inside the initialiser is moved by value
+//         in the statements between the candidate declaration and the
+//         substitution site (IsFreeVarSafe). This prevents E0382 borrow-of-
+//         moved-value errors introduced by inlining clone() helpers.
+//      d. Substitute the single reference with the initialiser (SubstituteRef).
+//         Handles both plain expression positions AND macro token streams.
+//      e. Remove the let statement.
+//      f. Set Changed = true and restart candidate collection.
+//   3. Wrap substituted binary/range expressions in parentheses when placed
+//      as a direct operand of a binary or unary expression (precedence
+//      safety).
 //=============================================================================//
 
 use proc_macro2::{Group, TokenStream, TokenTree};
@@ -50,13 +58,27 @@ impl<'a> Eliminator<'a> {
 					continue;
 				}
 
+				let StmtsAfter = &Block.stmts[Candidate.StmtIndex + 1..];
+
 				let (RefCount, InClosure) =
-					Count::CountReferences(&Candidate.Ident, &Block.stmts[Candidate.StmtIndex + 1..]);
+					Count::CountReferences(&Candidate.Ident, StmtsAfter);
 
 				if RefCount != 1 || InClosure {
 					continue;
 				}
 
+				// Find the index of the substitution site within StmtsAfter
+				// (the first statement that contains a reference to Candidate).
+				// We need the slice of statements that come BEFORE that site
+				// to check whether any free variable in Init is moved there.
+				let SubstSiteOffset = FindSubstSite(StmtsAfter, &Candidate.Ident);
+
+				let StmtsBetween = &StmtsAfter[..SubstSiteOffset];
+
+				if !Safe::IsFreeVarSafe(&Candidate.Init, StmtsBetween) {
+					continue;
+				}
+
 				let Substituted =
 					SubstituteRef(&mut Block.stmts[Candidate.StmtIndex + 1..], &Candidate.Ident, &Candidate.Init);
 
@@ -91,9 +113,9 @@ impl<'a> VisitMut for Eliminator<'a> {
 // Public: SubstituteRef
 // ---------------------------------------------------------------------------
 
-/// Replace the first occurrence of `Target` (as a plain identifier expression
-/// OR as an identifier token inside a macro's token stream) in `Stmts` with
-/// `Replacement`.  Returns `true` when the substitution was performed.
+/// Replace the first occurrence of Target (as a plain identifier expression
+/// OR as an identifier token inside a macro's token stream) in Stmts with
+/// Replacement. Returns true when the substitution was performed.
 pub fn SubstituteRef(Stmts:&mut [Stmt], Target:&str, Replacement:&Expr) -> bool {
 	let mut Sub = Substitutor { Target, Replacement, Substituted:false, InBinaryOperandPosition:false };
 
@@ -108,6 +130,26 @@ pub fn SubstituteRef(Stmts:&mut [Stmt], Target:&str, Replacement:&Expr) -> bool
 	Sub.Substituted
 }
 
+// ---------------------------------------------------------------------------
+// Internal: FindSubstSite
+// ---------------------------------------------------------------------------
+
+/// Return the index within Stmts of the first statement that contains a
+/// reference to Target. Returns Stmts.len() (one past end) when not found,
+/// which causes StmtsBetween to be the full slice - the conservative safe
+/// choice.
+fn FindSubstSite(Stmts:&[Stmt], Target:&str) -> usize {
+	for (I, Stmt) in Stmts.iter().enumerate() {
+		let (Count, _) = Count::CountReferences(Target, std::slice::from_ref(Stmt));
+
+		if Count > 0 {
+			return I;
+		}
+	}
+
+	Stmts.len()
+}
+
 // ---------------------------------------------------------------------------
 // Internal: Substitutor
 // ---------------------------------------------------------------------------
@@ -117,7 +159,7 @@ struct Substitutor<'a> {
 	Replacement:&'a Expr,
 	Substituted:bool,
 	/// True when the current AST position is a direct operand of a binary or
-	/// unary expression - used to decide whether to wrap `Replacement`.
+	/// unary expression - used to decide whether to wrap Replacement.
 	InBinaryOperandPosition:bool,
 }
 
@@ -183,9 +225,8 @@ impl<'a> VisitMut for Substitutor<'a> {
 		}
 	}
 
-	/// Substitute inside macro token streams (e.g. `json!(…)`, `dev_log!(…)`).
-	/// The default VisitMut does NOT recurse into `Macro::tokens`, so we do it
-	/// manually via raw token-tree manipulation.
+	/// Substitute inside macro token streams (e.g. json!(), dev_log!()).
+	/// The default VisitMut does NOT recurse into Macro::tokens.
 	fn visit_expr_macro_mut(&mut self, Node:&mut syn::ExprMacro) {
 		if self.Substituted {
 			return;
@@ -202,9 +243,9 @@ impl<'a> VisitMut for Substitutor<'a> {
 		}
 	}
 
-	/// syn v2 separates top-level macro statements (`dev_log!("{}", X);`) into
-	/// `Stmt::Macro(StmtMacro)`, which is never routed through
-	/// `visit_expr_macro_mut`.  Mirror the same substitution here.
+	/// syn v2 separates top-level macro statements (dev_log!("{}", X);) into
+	/// Stmt::Macro(StmtMacro), which is never routed through
+	/// visit_expr_macro_mut. Mirror the same substitution here.
 	fn visit_stmt_macro_mut(&mut self, Node:&mut syn::StmtMacro) {
 		if self.Substituted {
 			return;
@@ -244,8 +285,6 @@ impl<'a> VisitMut for Substitutor<'a> {
 // Token-stream helpers
 // ---------------------------------------------------------------------------
 
-/// Convert a `syn::Expr` to a `proc_macro2::TokenStream` by calling
-/// `quote::ToTokens::to_tokens`.
 fn ExprToTokenStream(E:&Expr) -> TokenStream {
 	let mut Tokens = TokenStream::new();
 
@@ -254,9 +293,6 @@ fn ExprToTokenStream(E:&Expr) -> TokenStream {
 	Tokens
 }
 
-/// Walk `Tokens` and replace the first `Ident` token exactly equal to `Target`
-/// with `Replacement` (a pre-rendered `TokenStream`).  Recurses into `Group`
-/// delimiters.  Returns `(new_stream, found)`.
 fn SubstituteInTokenStream(Tokens:TokenStream, Target:&str, Replacement:&TokenStream) -> (TokenStream, bool) {
 	let mut Result:Vec<TokenTree> = Vec::new();
 
@@ -271,7 +307,6 @@ fn SubstituteInTokenStream(Tokens:TokenStream, Target:&str, Replacement:&TokenSt
 
 		match Tree {
 			TokenTree::Ident(ref I) if I.to_string() == Target => {
-				// Extend with the replacement's token trees.
 				Result.extend(Replacement.clone());
 
 				Found = true;
@@ -400,7 +435,6 @@ mod Tests {
 
 	// --- Macro substitution tests -------------------------------------------
 
-	/// Variable used only inside a json! macro gets inlined into the macro.
 	#[test]
 	fn InlineIntoJsonMacro() {
 		AssertEliminates(
@@ -414,7 +448,6 @@ mod Tests {
 		);
 	}
 
-	/// Variable used in BOTH a macro and a plain expression: multi-use, kept.
 	#[test]
 	fn MacroAndExprMultiUseKept() {
 		AssertUnchanged(
@@ -426,7 +459,6 @@ mod Tests {
 		);
 	}
 
-	/// Variable used twice inside the same macro: multi-use, kept.
 	#[test]
 	fn MacroDoubleUseKept() {
 		AssertUnchanged(
@@ -437,7 +469,67 @@ mod Tests {
 		);
 	}
 
-	// --- URL-parsing pattern (primary motivating example) -------------------
+	// --- Free-variable move-safety tests (regression for #56) ---------------
+
+	/// display = path.clone() must NOT be inlined when path is moved into a
+	/// struct field between the declaration and the devlog use site.
+	/// This is the exact pattern from AirClient::get_file_info that produced
+	/// E0382 after eliminate ran on Source/Air.
+	#[test]
+	fn DisplayCloneKeptWhenOriginalMovedFirst() {
+		AssertUnchanged(
+			r#"
+				pub async fn get_file_info(request_id: String, path: String) -> Result<(), E> {
+					let path_display = path.clone();
+					client
+						.get_file_info(Request::new(FileInfoRequest { request_id, path }))
+						.await?;
+					devlog!("{}", path_display, path.clone());
+					Ok(())
+				}
+			"#,
+		);
+	}
+
+	/// Same pattern with section instead of path (AirClient::get_configuration).
+	#[test]
+	fn SectionDisplayCloneKeptWhenOriginalMovedFirst() {
+		AssertUnchanged(
+			r#"
+				pub async fn get_configuration(request_id: String, section: String) -> Result<(), E> {
+					let section_display = section.clone();
+					client
+						.get_configuration(Request::new(ConfigurationRequest { request_id, section }))
+						.await?;
+					devlog!("{}", section_display, section.clone());
+					Ok(())
+				}
+			"#,
+		);
+	}
+
+	/// When path is NOT moved between decl and use, the clone helper should
+	/// still be inlined (no false positive that would block legitimate inlines).
+	#[test]
+	fn DisplayCloneInlinedWhenOriginalNotMoved() {
+		AssertEliminates(
+			r#"
+				pub async fn log_path(path: String) -> Result<(), E> {
+					let path_display = path.clone();
+					devlog!("{}", path_display);
+					Ok(())
+				}
+			"#,
+			r#"
+				pub async fn log_path(path: String) -> Result<(), E> {
+					devlog!("{}", path.clone());
+					Ok(())
+				}
+			"#,
+		);
+	}
+
+	// --- URL-parsing pattern ------------------------------------------------
 
 	#[test]
 	fn UrlPatternInlined() {
@@ -492,8 +584,6 @@ mod Tests {
 
 	#[test]
 	fn ClosureCaptureKept() {
-		// F (the closure value) is single-use and is inlined.
-		// X (captured inside the closure) is conservatively kept.
 		AssertEliminates(
 			"fn f() { let X = heavy(); let F = move || X; call(F); }",
 			"fn f() { let X = heavy(); call(move || X); }",