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

[NikolaRHristov]

Summary

Maintain eliminate inlines a single-use let binding into a struct field initializer before a subsequent devlog! macro call that also references the same variable. Because eliminate counts the devlog! token-stream reference as a separate use and considers only the first use site, it incorrectly moves the value into the struct literal and then the devlog! line borrows the already-moved value, producing E0382.

Reproduction

~/Developer/Application/CodeEditorLand/Land/Target/release/Maintain eliminate \
  --path ./Source/Air \
  --verbose

Then cargo build (or cargo check) on Element/Mountain:

error[E0382]: borrow of moved value: `path`
   --> Element/Mountain/Source/Air/AirClient.rs:631:7
    |
611 |     pub async fn get_file_info(&self, request_id:String, path:String) -> ...
    |                                                          ---- move occurs because `path` has type `std::string::String`
...
623 |                 .get_file_info(Request::new(FileInfoRequest { request_id, path }))
    |                                                                           ---- value moved here
...
631 |                         path.clone(),
    |                         ^^^^ value borrowed here after move

error[E0382]: borrow of moved value: `section`
   --> Element/Mountain/Source/Air/AirClient.rs:949:7
    |
928 |         section:String,
    |         ------- move occurs because `section` has type `std::string::String`
...
941 |                 .get_configuration(Request::new(ConfigurationRequest { request_id, section }))
    |                                                                                    ------- value moved here
...
949 |                         section.clone(),
    |                         ^^^^^^^ value borrowed here after move

Original Code (before eliminate)

In get_file_info, the original pattern was:

let path_display = path.clone();            // used in devlog! below
let request = FileInfoRequest { request_id, path };  // path moved here
...
devlog!("...", path_display, path.clone(), ...);

path_display was a dedicated clone captured before the move. eliminate saw path_display as a single-use variable and inlined it, collapsing the pattern to:

// after eliminate:
.get_file_info(Request::new(FileInfoRequest { request_id, path }))
// path is now moved ^^ ...
devlog!("...", path.clone(), path.clone(), ...)  // E0382: path already moved

The same pattern occurs in get_configuration with section_display / section.

Root Cause

Count::CountReferences counts identifiers in subsequent statements including inside macro token streams. When the pattern is:

let display = var.clone();   // (A) single-use candidate
// ... var is used (moved) later at (B)
devlog!(..., display, ...);  // (C) — the only use of `display`

display genuinely has exactly one reference (at C), so eliminate inlines var.clone() at C and removes the let display line. But the reference count for var itself is computed separately. If var appears only in (B) from the perspective of the plain-expression counter (the struct field), but also appears later in a devlog! token stream at (C) via the original display name—after inlining, the devlog! now references var.clone() where var has already been moved at (B).

The core issue is that eliminate does not check whether inlining a clone() initializer into a later statement would leave the original variable (var) in a moved state at an earlier point in the same block. The substitution is order-aware for the candidate variable, but not for the variables inside the candidate's initializer expression.

Expected Behaviour

If inlining display = var.clone() at its use site would place a clone of var after a point where var has already been moved (or after it is consumed by another expression), the binding must be kept as-is. The safety check should verify that all free variables in the candidate's initializer expression remain valid (not moved) at every point after the candidate's original declaration site and up to and including the substitution site.

Suggested Fix

In Source/Eliminate/Transform/Safe.rs (or as an additional guard in Inline::EliminateBlock before calling SubstituteRef):

1. Collect the set of free identifiers in Candidate.Init.
2. For each such identifier, scan the statements between Candidate.StmtIndex and the substitution site (the use of the candidate) to check whether any of them move the free identifier (i.e., pass it by value to a function, use it in a struct literal field without .clone(), or assign it to another binding without a reference).
3. If any free identifier in Init would be in a moved state at the substitution site, skip inlining this candidate.

This is conservative but correct. A more precise version would use dataflow / borrow analysis, but a simple "does a statement between declaration and use consume this identifier by value" heuristic would catch the exact patterns above.

Affected Functions

• AirClient::get_file_info — path_display = path.clone() inlined, path later moved into FileInfoRequest
• AirClient::get_configuration — section_display = section.clone() inlined, section later moved into ConfigurationRequest

Likely affects any function where a display-style clone variable is declared before the original value is moved into a struct or function call.

Context

• Invocation: Maintain eliminate --path ./Source/Air --verbose
• Branch: Current
• Introduced by the same run that caused eliminate: full-file AST reprint destroys comments, blank lines, and structural formatting #55 (full-file AST reprint)
• String does not implement Copy, so field shorthand syntax in struct literals ({ field } instead of { field: field.clone() }) always moves the value

[NikolaRHristov]

Additional findings from deeper analysis

F — Top-level dispatch in Library.rs is manual string-matching, not clap subcommands

The current dispatch logic in Library.rs manually inspects args[1]:

let is_eliminate_mode = first_arg == "eliminate" || first_arg == "--eliminate";
// ...
if is_eliminate_mode {
    args.remove(1);
    match Eliminate::CLI::Cli::try_parse_from(args) { ... }
}

This creates several silent failure modes:

• Maintain --path foo eliminate → routes to legacy build mode, not eliminate.
• Maintain build eliminate --path foo → removes args[1] = "build" and passes ["eliminate", "--path", "foo"] to Build’s CLI, likely failing with a confusing error.
• Any future subcommand addition requires touching the same manual if-chain and keeping the ordering correct.

The correct fix is a top-level #[derive(clap::Parser)] with #[clap(subcommand)] variants for Build, Run, and Eliminate, replacing all of the is_*_mode checks. Each variant forwards to its own CLI::Cli::execute().

---

G — InlineComments flag name implies comment preservation; it does not

Definition::Options::InlineComments and --inline-comments in CLI.rs sound to a user like “keep comments when inlining” or “preserve comments in output”. The actual semantics are:

When true, bindings that carry leading doc-comments or #[...] attributes are still eligible for inlining.

So the flag name controls eligibility of attributed let bindings for inlining, not comment preservation. Comments in the rest of the file are lost regardless of this flag (as described in #55). A user who adds --inline-comments hoping to keep their section-separator // comments will see no improvement.

Suggested rename: --include-attributed or --inline-attributed (for the flag) and InlineAttributed: bool (for the struct field), with updated docs clarifying that this controls whether let bindings with #[...] attributes or doc-comments are candidates for inlining.