Skip to content

Commit 30a6d21

Browse files
committed
Return (Result<(),E>,) in drain_the_queue
This refactors the drain_the_queue function to return a tuple containing Result. That way, it's still not possible to use ? or try! to handle errors, but for readers of the function declaration it's clearer now that the error actually indicates one. Bonus: it makes the calling code of drain_the_queue simpler.
1 parent 65d57e6 commit 30a6d21

File tree

1 file changed

+11
-12
lines changed

1 file changed

+11
-12
lines changed

src/cargo/core/compiler/job_queue.rs

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -457,10 +457,8 @@ impl<'cfg> JobQueue<'cfg> {
457457
.map(move |srv| srv.start(move |msg| messages.push(Message::FixDiagnostic(msg))));
458458

459459
crossbeam_utils::thread::scope(move |scope| {
460-
match state.drain_the_queue(cx, plan, scope, &helper) {
461-
Some(err) => Err(err),
462-
None => Ok(()),
463-
}
460+
let (result,) = state.drain_the_queue(cx, plan, scope, &helper);
461+
result
464462
})
465463
.expect("child threads shouldn't panic")
466464
}
@@ -691,15 +689,16 @@ impl<'cfg> DrainState<'cfg> {
691689
/// This is the "main" loop, where Cargo does all work to run the
692690
/// compiler.
693691
///
694-
/// This returns an Option to prevent the use of `?` on `Result` types
695-
/// because it is important for the loop to carefully handle errors.
692+
/// This returns a tuple of `Result` to prevent the use of `?` on
693+
/// `Result` types because it is important for the loop to
694+
/// carefully handle errors.
696695
fn drain_the_queue(
697696
mut self,
698697
cx: &mut Context<'_, '_>,
699698
plan: &mut BuildPlan,
700699
scope: &Scope<'_>,
701700
jobserver_helper: &HelperThread,
702-
) -> Option<anyhow::Error> {
701+
) -> (Result<(), anyhow::Error>,) {
703702
trace!("queue: {:#?}", self.queue);
704703

705704
// Iteratively execute the entire dependency graph. Each turn of the
@@ -769,7 +768,7 @@ impl<'cfg> DrainState<'cfg> {
769768
if error.is_some() {
770769
crate::display_error(&e, &mut cx.bcx.config.shell());
771770
} else {
772-
return Some(e);
771+
return (Err(e),);
773772
}
774773
}
775774
if cx.bcx.build_config.emit_json() {
@@ -782,13 +781,13 @@ impl<'cfg> DrainState<'cfg> {
782781
if error.is_some() {
783782
crate::display_error(&e.into(), &mut shell);
784783
} else {
785-
return Some(e.into());
784+
return (Err(e.into()),);
786785
}
787786
}
788787
}
789788

790789
if let Some(e) = error {
791-
Some(e)
790+
(Err(e),)
792791
} else if self.queue.is_empty() && self.pending_queue.is_empty() {
793792
let message = format!(
794793
"{} [{}] target(s) in {}",
@@ -800,10 +799,10 @@ impl<'cfg> DrainState<'cfg> {
800799
self.emit_future_incompat(cx);
801800
}
802801

803-
None
802+
(Ok(()),)
804803
} else {
805804
debug!("queue: {:#?}", self.queue);
806-
Some(internal("finished with jobs still left in the queue"))
805+
(Err(internal("finished with jobs still left in the queue")),)
807806
}
808807
}
809808

0 commit comments

Comments
 (0)