Skip to content

Commit 8926dac

Browse files
And for patterns too
1 parent 99c3257 commit 8926dac

File tree

5 files changed

+184
-119
lines changed

5 files changed

+184
-119
lines changed

compiler/rustc_infer/src/infer/error_reporting/mod.rs

+89-78
Original file line numberDiff line numberDiff line change
@@ -614,13 +614,16 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
614614
err.span_label(span, "expected due to this");
615615
}
616616
ObligationCauseCode::MatchExpressionArm(box MatchExpressionArmCause {
617-
semi_span,
617+
arm_block_id,
618+
arm_span,
619+
arm_ty,
620+
prior_arm_block_id,
621+
prior_arm_span,
622+
prior_arm_ty,
618623
source,
619624
ref prior_arms,
620-
last_ty,
621625
scrut_hir_id,
622626
opt_suggest_box_span,
623-
arm_span,
624627
scrut_span,
625628
..
626629
}) => match source {
@@ -651,10 +654,10 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
651654
}
652655
}
653656
_ => {
654-
// `last_ty` can be `!`, `expected` will have better info when present.
657+
// `prior_arm_ty` can be `!`, `expected` will have better info when present.
655658
let t = self.resolve_vars_if_possible(match exp_found {
656659
Some(ty::error::ExpectedFound { expected, .. }) => expected,
657-
_ => last_ty,
660+
_ => prior_arm_ty,
658661
});
659662
let source_map = self.tcx.sess.source_map();
660663
let mut any_multiline_arm = source_map.is_multiline(arm_span);
@@ -679,37 +682,15 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
679682
};
680683
let msg = "`match` arms have incompatible types";
681684
err.span_label(outer_error_span, msg);
682-
if let Some((sp, boxed)) = semi_span {
683-
if let (StatementAsExpression::NeedsBoxing, [.., prior_arm]) =
684-
(boxed, &prior_arms[..])
685-
{
686-
err.multipart_suggestion(
687-
"consider removing this semicolon and boxing the expressions",
688-
vec![
689-
(prior_arm.shrink_to_lo(), "Box::new(".to_string()),
690-
(prior_arm.shrink_to_hi(), ")".to_string()),
691-
(arm_span.shrink_to_lo(), "Box::new(".to_string()),
692-
(arm_span.shrink_to_hi(), ")".to_string()),
693-
(sp, String::new()),
694-
],
695-
Applicability::HasPlaceholders,
696-
);
697-
} else if matches!(boxed, StatementAsExpression::NeedsBoxing) {
698-
err.span_suggestion_short(
699-
sp,
700-
"consider removing this semicolon and boxing the expressions",
701-
"",
702-
Applicability::MachineApplicable,
703-
);
704-
} else {
705-
err.span_suggestion_short(
706-
sp,
707-
"consider removing this semicolon",
708-
"",
709-
Applicability::MachineApplicable,
710-
);
711-
}
712-
}
685+
self.suggest_remove_semi_or_return_binding(
686+
err,
687+
prior_arm_block_id,
688+
prior_arm_ty,
689+
prior_arm_span,
690+
arm_block_id,
691+
arm_ty,
692+
arm_span,
693+
);
713694
if let Some(ret_sp) = opt_suggest_box_span {
714695
// Get return type span and point to it.
715696
self.suggest_boxing_for_return_impl_trait(
@@ -734,48 +715,15 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
734715
if let Some(sp) = outer_span {
735716
err.span_label(sp, "`if` and `else` have incompatible types");
736717
}
737-
let semicolon = if let hir::Node::Block(blk) = self.tcx.hir().get(then_id)
738-
&& let Some(remove_semicolon) = self.could_remove_semicolon(blk, else_ty)
739-
{
740-
Some(remove_semicolon)
741-
} else if let hir::Node::Block(blk) = self.tcx.hir().get(else_id)
742-
&& let Some(remove_semicolon) = self.could_remove_semicolon(blk, then_ty)
743-
{
744-
Some(remove_semicolon)
745-
} else {
746-
None
747-
};
748-
if let Some((sp, boxed)) = semicolon {
749-
if matches!(boxed, StatementAsExpression::NeedsBoxing) {
750-
err.multipart_suggestion(
751-
"consider removing this semicolon and boxing the expression",
752-
vec![
753-
(then_span.shrink_to_lo(), "Box::new(".to_string()),
754-
(then_span.shrink_to_hi(), ")".to_string()),
755-
(else_span.shrink_to_lo(), "Box::new(".to_string()),
756-
(else_span.shrink_to_hi(), ")".to_string()),
757-
(sp, String::new()),
758-
],
759-
Applicability::MachineApplicable,
760-
);
761-
} else {
762-
err.span_suggestion_short(
763-
sp,
764-
"consider removing this semicolon",
765-
"",
766-
Applicability::MachineApplicable,
767-
);
768-
}
769-
} else {
770-
let suggested = if let hir::Node::Block(blk) = self.tcx.hir().get(then_id) {
771-
self.consider_returning_binding(blk, else_ty, err)
772-
} else {
773-
false
774-
};
775-
if !suggested && let hir::Node::Block(blk) = self.tcx.hir().get(else_id) {
776-
self.consider_returning_binding(blk, then_ty, err);
777-
}
778-
}
718+
self.suggest_remove_semi_or_return_binding(
719+
err,
720+
Some(then_id),
721+
then_ty,
722+
then_span,
723+
Some(else_id),
724+
else_ty,
725+
else_span,
726+
);
779727
if let Some(ret_sp) = opt_suggest_box_span {
780728
self.suggest_boxing_for_return_impl_trait(
781729
err,
@@ -800,6 +748,69 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
800748
}
801749
}
802750

751+
fn suggest_remove_semi_or_return_binding(
752+
&self,
753+
err: &mut Diagnostic,
754+
first_id: Option<hir::HirId>,
755+
first_ty: Ty<'tcx>,
756+
first_span: Span,
757+
second_id: Option<hir::HirId>,
758+
second_ty: Ty<'tcx>,
759+
second_span: Span,
760+
) {
761+
let semicolon =
762+
if let Some(first_id) = first_id
763+
&& let hir::Node::Block(blk) = self.tcx.hir().get(first_id)
764+
&& let Some(remove_semicolon) = self.could_remove_semicolon(blk, second_ty)
765+
{
766+
Some(remove_semicolon)
767+
} else if let Some(second_id) = second_id
768+
&& let hir::Node::Block(blk) = self.tcx.hir().get(second_id)
769+
&& let Some(remove_semicolon) = self.could_remove_semicolon(blk, first_ty)
770+
{
771+
Some(remove_semicolon)
772+
} else {
773+
None
774+
};
775+
if let Some((sp, boxed)) = semicolon {
776+
if matches!(boxed, StatementAsExpression::NeedsBoxing) {
777+
err.multipart_suggestion(
778+
"consider removing this semicolon and boxing the expressions",
779+
vec![
780+
(first_span.shrink_to_lo(), "Box::new(".to_string()),
781+
(first_span.shrink_to_hi(), ")".to_string()),
782+
(second_span.shrink_to_lo(), "Box::new(".to_string()),
783+
(second_span.shrink_to_hi(), ")".to_string()),
784+
(sp, String::new()),
785+
],
786+
Applicability::MachineApplicable,
787+
);
788+
} else {
789+
err.span_suggestion_short(
790+
sp,
791+
"consider removing this semicolon",
792+
"",
793+
Applicability::MachineApplicable,
794+
);
795+
}
796+
} else {
797+
let suggested =
798+
if let Some(first_id) = first_id
799+
&& let hir::Node::Block(blk) = self.tcx.hir().get(first_id)
800+
{
801+
self.consider_returning_binding(blk, second_ty, err)
802+
} else {
803+
false
804+
};
805+
if !suggested
806+
&& let Some(second_id) = second_id
807+
&& let hir::Node::Block(blk) = self.tcx.hir().get(second_id)
808+
{
809+
self.consider_returning_binding(blk, first_ty, err);
810+
}
811+
}
812+
}
813+
803814
fn suggest_boxing_for_return_impl_trait(
804815
&self,
805816
err: &mut Diagnostic,

compiler/rustc_middle/src/traits/mod.rs

+5-2
Original file line numberDiff line numberDiff line change
@@ -488,12 +488,15 @@ impl<'tcx> ty::Lift<'tcx> for StatementAsExpression {
488488

489489
#[derive(Clone, Debug, PartialEq, Eq, Hash, Lift)]
490490
pub struct MatchExpressionArmCause<'tcx> {
491+
pub arm_block_id: Option<hir::HirId>,
492+
pub arm_ty: Ty<'tcx>,
491493
pub arm_span: Span,
494+
pub prior_arm_block_id: Option<hir::HirId>,
495+
pub prior_arm_ty: Ty<'tcx>,
496+
pub prior_arm_span: Span,
492497
pub scrut_span: Span,
493-
pub semi_span: Option<(Span, StatementAsExpression)>,
494498
pub source: hir::MatchSource,
495499
pub prior_arms: Vec<Span>,
496-
pub last_ty: Ty<'tcx>,
497500
pub scrut_hir_id: hir::HirId,
498501
pub opt_suggest_box_span: Option<Span>,
499502
}

compiler/rustc_typeck/src/check/_match.rs

+19-38
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ use rustc_span::Span;
99
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
1010
use rustc_trait_selection::traits::{
1111
IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode,
12-
StatementAsExpression,
1312
};
1413

1514
impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
@@ -75,8 +74,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
7574
};
7675

7776
let mut other_arms = vec![]; // Used only for diagnostics.
78-
let mut prior_arm_ty = None;
79-
for (i, arm) in arms.iter().enumerate() {
77+
let mut prior_arm = None;
78+
for arm in arms {
8079
if let Some(g) = &arm.guard {
8180
self.diverges.set(Diverges::Maybe);
8281
match g {
@@ -96,21 +95,28 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
9695

9796
let opt_suggest_box_span = self.opt_suggest_box_span(arm_ty, orig_expected);
9897

99-
let (arm_span, semi_span) =
100-
self.get_appropriate_arm_semicolon_removal_span(&arms, i, prior_arm_ty, arm_ty);
101-
let (span, code) = match i {
98+
let (arm_block_id, arm_span) = if let hir::ExprKind::Block(blk, _) = arm.body.kind {
99+
(Some(blk.hir_id), self.find_block_span(blk))
100+
} else {
101+
(None, arm.body.span)
102+
};
103+
104+
let (span, code) = match prior_arm {
102105
// The reason for the first arm to fail is not that the match arms diverge,
103106
// but rather that there's a prior obligation that doesn't hold.
104-
0 => (arm_span, ObligationCauseCode::BlockTailExpression(arm.body.hir_id)),
105-
_ => (
107+
None => (arm_span, ObligationCauseCode::BlockTailExpression(arm.body.hir_id)),
108+
Some((prior_arm_block_id, prior_arm_ty, prior_arm_span)) => (
106109
expr.span,
107110
ObligationCauseCode::MatchExpressionArm(Box::new(MatchExpressionArmCause {
111+
arm_block_id,
108112
arm_span,
113+
arm_ty,
114+
prior_arm_block_id,
115+
prior_arm_ty,
116+
prior_arm_span,
109117
scrut_span: scrut.span,
110-
semi_span,
111118
source: match_src,
112119
prior_arms: other_arms.clone(),
113-
last_ty: prior_arm_ty.unwrap(),
114120
scrut_hir_id: scrut.hir_id,
115121
opt_suggest_box_span,
116122
})),
@@ -139,7 +145,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
139145
let ret_ty = ret_coercion.borrow().expected_ty();
140146
let ret_ty = self.inh.infcx.shallow_resolve(ret_ty);
141147
self.can_coerce(arm_ty, ret_ty)
142-
&& prior_arm_ty.map_or(true, |t| self.can_coerce(t, ret_ty))
148+
&& prior_arm.map_or(true, |(_, t, _)| self.can_coerce(t, ret_ty))
143149
// The match arms need to unify for the case of `impl Trait`.
144150
&& !matches!(ret_ty.kind(), ty::Opaque(..))
145151
}
@@ -181,7 +187,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
181187
if other_arms.len() > 5 {
182188
other_arms.remove(0);
183189
}
184-
prior_arm_ty = Some(arm_ty);
190+
191+
prior_arm = Some((arm_block_id, arm_ty, arm_span));
185192
}
186193

187194
// If all of the arms in the `match` diverge,
@@ -207,32 +214,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
207214
match_ty
208215
}
209216

210-
fn get_appropriate_arm_semicolon_removal_span(
211-
&self,
212-
arms: &'tcx [hir::Arm<'tcx>],
213-
i: usize,
214-
prior_arm_ty: Option<Ty<'tcx>>,
215-
arm_ty: Ty<'tcx>,
216-
) -> (Span, Option<(Span, StatementAsExpression)>) {
217-
let arm = &arms[i];
218-
let (arm_span, mut semi_span) = if let hir::ExprKind::Block(blk, _) = &arm.body.kind {
219-
(
220-
self.find_block_span(blk),
221-
prior_arm_ty
222-
.and_then(|prior_arm_ty| self.could_remove_semicolon(blk, prior_arm_ty)),
223-
)
224-
} else {
225-
(arm.body.span, None)
226-
};
227-
if semi_span.is_none() && i > 0 {
228-
if let hir::ExprKind::Block(blk, _) = &arms[i - 1].body.kind {
229-
let semi_span_prev = self.could_remove_semicolon(blk, arm_ty);
230-
semi_span = semi_span_prev;
231-
}
232-
}
233-
(arm_span, semi_span)
234-
}
235-
236217
/// When the previously checked expression (the scrutinee) diverges,
237218
/// warn the user about the match arms being unreachable.
238219
fn warn_arms_when_scrutinee_diverges(&self, arms: &'tcx [hir::Arm<'tcx>]) {

src/test/ui/suggestions/return-bindings.rs

+24
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,28 @@ fn d(opt_str: Option<String>) {
2424
};
2525
}
2626

27+
fn d2(opt_str: Option<String>) {
28+
let s = if let Some(s) = opt_str {
29+
} else {
30+
String::new()
31+
//~^ ERROR `if` and `else` have incompatible types
32+
};
33+
}
34+
35+
fn e(opt_str: Option<String>) {
36+
let s: String = match opt_str {
37+
Some(s) => {}
38+
//~^ ERROR mismatched types
39+
None => String::new(),
40+
};
41+
}
42+
43+
fn e2(opt_str: Option<String>) {
44+
let s = match opt_str {
45+
Some(s) => {}
46+
None => String::new(),
47+
//~^ ERROR `match` arms have incompatible types
48+
};
49+
}
50+
2751
fn main() {}

0 commit comments

Comments
 (0)