Skip to content

Commit 1aad10c

Browse files
authored
Remove dead Option layer from run_piped (#3634)
This is a clean-up PR: I noticed that the `run_piped` function has an `Option` layer that is unnecessary: - If the process launching fails, it returns an error. - If it succeeds, it returns `Ok(Some(process))` Thus, the `Option` layer is always `Some`. The main change is updating the function signature from: ```rust pub fn run_piped(verbosity: &impl Verbosity, mut cmd: Command) -> Result<Option<Child>> ``` to: ```rust pub fn run_piped(verbosity: &impl Verbosity, mut cmd: Command) -> Result<Child> ``` By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.
1 parent 20c3d69 commit 1aad10c

File tree

3 files changed

+50
-53
lines changed

3 files changed

+50
-53
lines changed

kani-driver/src/call_cargo.rs

Lines changed: 45 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -277,62 +277,59 @@ crate-type = ["lib"]
277277
fn run_build(&self, cargo_cmd: Command) -> Result<Vec<RustcArtifact>> {
278278
let support_color = std::io::stdout().is_terminal();
279279
let mut artifacts = vec![];
280-
if let Some(mut cargo_process) = self.run_piped(cargo_cmd)? {
281-
let reader = BufReader::new(cargo_process.stdout.take().unwrap());
282-
let mut error_count = 0;
283-
for message in Message::parse_stream(reader) {
284-
let message = message.unwrap();
285-
match message {
286-
Message::CompilerMessage(msg) => match msg.message.level {
287-
DiagnosticLevel::FailureNote => {
288-
print_msg(&msg.message, support_color)?;
289-
}
290-
DiagnosticLevel::Error => {
291-
error_count += 1;
292-
print_msg(&msg.message, support_color)?;
293-
}
294-
DiagnosticLevel::Ice => {
295-
print_msg(&msg.message, support_color)?;
296-
let _ = cargo_process.wait();
297-
return Err(anyhow::Error::msg(msg.message).context(format!(
298-
"Failed to compile `{}` due to an internal compiler error.",
299-
msg.target.name
300-
)));
301-
}
302-
_ => {
303-
if !self.args.common_args.quiet {
304-
print_msg(&msg.message, support_color)?;
305-
}
306-
}
307-
},
308-
Message::CompilerArtifact(rustc_artifact) => {
309-
// Compares two targets, and falls back to a weaker
310-
// comparison where we avoid dashes in their names.
311-
artifacts.push(rustc_artifact)
280+
let mut cargo_process = self.run_piped(cargo_cmd)?;
281+
let reader = BufReader::new(cargo_process.stdout.take().unwrap());
282+
let mut error_count = 0;
283+
for message in Message::parse_stream(reader) {
284+
let message = message.unwrap();
285+
match message {
286+
Message::CompilerMessage(msg) => match msg.message.level {
287+
DiagnosticLevel::FailureNote => {
288+
print_msg(&msg.message, support_color)?;
312289
}
313-
Message::BuildScriptExecuted(_) | Message::BuildFinished(_) => {
314-
// do nothing
290+
DiagnosticLevel::Error => {
291+
error_count += 1;
292+
print_msg(&msg.message, support_color)?;
315293
}
316-
Message::TextLine(msg) => {
317-
if !self.args.common_args.quiet {
318-
println!("{msg}");
319-
}
294+
DiagnosticLevel::Ice => {
295+
print_msg(&msg.message, support_color)?;
296+
let _ = cargo_process.wait();
297+
return Err(anyhow::Error::msg(msg.message).context(format!(
298+
"Failed to compile `{}` due to an internal compiler error.",
299+
msg.target.name
300+
)));
320301
}
321-
322-
// Non-exhaustive enum.
323302
_ => {
324303
if !self.args.common_args.quiet {
325-
println!("{message:?}");
304+
print_msg(&msg.message, support_color)?;
326305
}
327306
}
307+
},
308+
Message::CompilerArtifact(rustc_artifact) => {
309+
// Compares two targets, and falls back to a weaker
310+
// comparison where we avoid dashes in their names.
311+
artifacts.push(rustc_artifact)
312+
}
313+
Message::BuildScriptExecuted(_) | Message::BuildFinished(_) => {
314+
// do nothing
315+
}
316+
Message::TextLine(msg) => {
317+
if !self.args.common_args.quiet {
318+
println!("{msg}");
319+
}
320+
}
321+
322+
// Non-exhaustive enum.
323+
_ => {
324+
if !self.args.common_args.quiet {
325+
println!("{message:?}");
326+
}
328327
}
329328
}
330-
let status = cargo_process.wait()?;
331-
if !status.success() {
332-
bail!(
333-
"Failed to execute cargo ({status}). Found {error_count} compilation errors."
334-
);
335-
}
329+
}
330+
let status = cargo_process.wait()?;
331+
if !status.success() {
332+
bail!("Failed to execute cargo ({status}). Found {error_count} compilation errors.");
336333
}
337334
Ok(artifacts)
338335
}
@@ -383,7 +380,7 @@ crate-type = ["lib"]
383380
cmd.arg(pkg);
384381
// For some reason clippy cannot see that we are invoking wait() in the next line.
385382
#[allow(clippy::zombie_processes)]
386-
let mut process = self.run_piped(cmd)?.unwrap();
383+
let mut process = self.run_piped(cmd)?;
387384
let result = process.wait()?;
388385
if !result.success() {
389386
bail!("Failed to retrieve information for `{pkg}`");

kani-driver/src/call_cbmc.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,8 @@ impl KaniSession {
8888
cmd.arg("--json-ui");
8989

9090
// Spawn the CBMC process and process its output below
91-
let cbmc_process_opt = self.run_piped(cmd)?;
92-
let cbmc_process = cbmc_process_opt.ok_or(anyhow::Error::msg("Failed to run cbmc"))?;
91+
let cbmc_process =
92+
self.run_piped(cmd).map_err(|_| anyhow::Error::msg("Failed to run cbmc"))?;
9393
let output = process_cbmc_output(cbmc_process, |i| {
9494
kani_cbmc_output_filter(
9595
i,

kani-driver/src/session.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ impl KaniSession {
124124
}
125125

126126
/// Call [run_piped] with the verbosity configured by the user.
127-
pub fn run_piped(&self, cmd: Command) -> Result<Option<Child>> {
127+
pub fn run_piped(&self, cmd: Command) -> Result<Child> {
128128
run_piped(&self.args.common_args, cmd)
129129
}
130130

@@ -227,7 +227,7 @@ pub fn run_redirect(
227227
///
228228
/// NOTE: Unlike other `run_` functions, this function does not attempt to indicate
229229
/// the process exit code, you need to remember to check this yourself.
230-
pub fn run_piped(verbosity: &impl Verbosity, mut cmd: Command) -> Result<Option<Child>> {
230+
pub fn run_piped(verbosity: &impl Verbosity, mut cmd: Command) -> Result<Child> {
231231
if verbosity.verbose() {
232232
println!("[Kani] Running: `{}`", render_command(&cmd).to_string_lossy());
233233
}
@@ -237,7 +237,7 @@ pub fn run_piped(verbosity: &impl Verbosity, mut cmd: Command) -> Result<Option<
237237
.spawn()
238238
.context(format!("Failed to invoke {}", cmd.get_program().to_string_lossy()))?;
239239

240-
Ok(Some(process))
240+
Ok(process)
241241
}
242242

243243
/// Execute the provided function and measure the clock time it took for its execution.

0 commit comments

Comments
 (0)