Skip to content

Commit c22da52

Browse files
refactor: return lazy boxed iterator from BuildTaskRunner
BuildTaskRunner::run previously collected the entire cargo message stream into a Vec, which forced every message to be parsed and allocated up front and prevented get_target_dir_from_output from short-circuiting once the matching cdylib artifact is found. Return Box<dyn Iterator<..>> instead so the runner forwards BuildTask::run's lazy stream while still being mockable (mockall cannot mock an opaque impl Trait return, but can mock a boxed trait object). BuildTask::run gains + use<> precise capturing so its owned, 'static iterator can move out of the temporary BuildTask and be boxed. Addresses PR review comment r3581534373. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fa978dd1-9d61-4cea-a03a-7ebc914228fa
1 parent 14ee218 commit c22da52

3 files changed

Lines changed: 24 additions & 8 deletions

File tree

crates/cargo-wdk/src/actions/build/build_task.rs

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,21 @@ pub struct BuildTaskRunner {}
5050
#[automock]
5151
#[allow(dead_code, clippy::unused_self, clippy::elidable_lifetime_names)]
5252
impl BuildTaskRunner {
53+
// Returns `Box<dyn Iterator<...>>` rather than `impl Iterator<...>` because
54+
// this method is `#[automock]`ed. mockall must be able to *name* the return
55+
// type to generate the mock's expectation storage, and it cannot mock an
56+
// opaque `impl Trait` return. Boxing into a trait object gives mockall a
57+
// concrete, nameable type while still forwarding `BuildTask::run`'s lazy
58+
// message stream, so consumers like `get_target_dir_from_output` can
59+
// short-circuit instead of parsing/allocating every cargo message up front.
5360
pub fn run<'a>(
5461
&self,
5562
params: &BuildTaskParams<'a>,
5663
command_exec: &CommandExec,
57-
) -> Result<Vec<Result<Message, std::io::Error>>, BuildTaskError> {
58-
BuildTask::new(*params, command_exec)
59-
.run()
60-
.map(Iterator::collect)
64+
) -> Result<Box<dyn Iterator<Item = Result<Message, std::io::Error>>>, BuildTaskError> {
65+
BuildTask::new(*params, command_exec).run().map(|messages| {
66+
Box::new(messages) as Box<dyn Iterator<Item = Result<Message, std::io::Error>>>
67+
})
6168
}
6269
}
6370

@@ -110,9 +117,14 @@ impl<'a> BuildTask<'a> {
110117
/// not a valid unicode
111118
/// * `BuildTaskError::CargoBuild` - If there is an error running the `cargo
112119
/// build` command
120+
// `+ use<>` opts this RPIT out of capturing the `&self` lifetime (edition
121+
// 2024 captures in-scope lifetimes by default). The returned iterator owns
122+
// its buffer (`Cursor<Vec<u8>>`), so it is effectively `'static`; opting out
123+
// of the capture lets `BuildTaskRunner::run` move it out of the temporary
124+
// `BuildTask` and box it as a `'static` trait object.
113125
pub fn run(
114126
&self,
115-
) -> Result<impl Iterator<Item = Result<Message, std::io::Error>>, BuildTaskError> {
127+
) -> Result<impl Iterator<Item = Result<Message, std::io::Error>> + use<>, BuildTaskError> {
116128
debug!("Running cargo build");
117129
let mut args = vec!["build".to_string()];
118130
args.push("--message-format=json-render-diagnostics".to_string());

crates/cargo-wdk/src/actions/build/mod.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -418,8 +418,7 @@ impl<'a> BuildAction<'a> {
418418
self.get_target_arch_from_cargo_rustc(working_dir)?
419419
};
420420
debug!("Target architecture for package: {package_name} is: {target_arch}");
421-
let target_dir =
422-
Self::get_target_dir_from_output(package, output_message_iter.into_iter())?;
421+
let target_dir = Self::get_target_dir_from_output(package, output_message_iter)?;
423422
debug!(
424423
"Target directory for package: {} is: {}",
425424
package_name,

crates/cargo-wdk/src/actions/build/tests.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -822,7 +822,12 @@ impl BuildActionHarness {
822822
&& params.features.features == expected_features.features
823823
})
824824
.once()
825-
.return_once(move |_, _| result);
825+
.return_once(move |_, _| {
826+
result.map(|messages| {
827+
Box::new(messages.into_iter())
828+
as Box<dyn Iterator<Item = Result<Message, io::Error>>>
829+
})
830+
});
826831
}
827832

828833
fn expect_package_runner(

0 commit comments

Comments
 (0)