Skip to content

Commit

Permalink
Rollup merge of rust-lang#114819 - estebank:issue-78124, r=compiler-e…
Browse files Browse the repository at this point in the history
…rrors

Point at return type when it influences non-first `match` arm

When encountering code like

```rust
fn foo() -> i32 {
    match 0 {
        1 => return 0,
        2 => "",
        _ => 1,
    }
}
```

Point at the return type and not at the prior arm, as that arm has type `!` which isn't influencing the arm corresponding to arm `2`.

Fix rust-lang#78124.
  • Loading branch information
matthiaskrgr authored Aug 15, 2023
2 parents 28295db + 58aa903 commit d53aff0
Show file tree
Hide file tree
Showing 38 changed files with 157 additions and 73 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1648,7 +1648,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
hir::ExprKind::Match(
scrutinee,
arena_vec![self; break_arm, continue_arm],
hir::MatchSource::TryDesugar,
hir::MatchSource::TryDesugar(scrutinee.hir_id),
)
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2148,7 +2148,7 @@ pub enum MatchSource {
/// A desugared `for _ in _ { .. }` loop.
ForLoopDesugar,
/// A desugared `?` operator.
TryDesugar,
TryDesugar(HirId),
/// A desugared `<expr>.await`.
AwaitDesugar,
/// A desugared `format_args!()`.
Expand All @@ -2162,7 +2162,7 @@ impl MatchSource {
match self {
Normal => "match",
ForLoopDesugar => "for",
TryDesugar => "?",
TryDesugar(_) => "?",
AwaitDesugar => ".await",
FormatArgs => "format_args!()",
}
Expand Down
12 changes: 9 additions & 3 deletions compiler/rustc_hir_typeck/src/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let (span, code) = match prior_arm {
// The reason for the first arm to fail is not that the match arms diverge,
// but rather that there's a prior obligation that doesn't hold.
None => (arm_span, ObligationCauseCode::BlockTailExpression(arm.body.hir_id)),
None => {
(arm_span, ObligationCauseCode::BlockTailExpression(arm.body.hir_id, match_src))
}
Some((prior_arm_block_id, prior_arm_ty, prior_arm_span)) => (
expr.span,
ObligationCauseCode::MatchExpressionArm(Box::new(MatchExpressionArmCause {
Expand All @@ -120,7 +122,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
scrut_span: scrut.span,
source: match_src,
prior_arms: other_arms.clone(),
scrut_hir_id: scrut.hir_id,
opt_suggest_box_span,
})),
),
Expand All @@ -145,7 +146,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
other_arms.remove(0);
}

prior_arm = Some((arm_block_id, arm_ty, arm_span));
if !arm_ty.is_never() {
// When a match arm has type `!`, then it doesn't influence the expected type for
// the following arm. If all of the prior arms are `!`, then the influence comes
// from elsewhere and we shouldn't point to any previous arm.
prior_arm = Some((arm_block_id, arm_ty, arm_span));
}
}

// If all of the arms in the `match` diverge,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_typeck/src/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1603,7 +1603,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
);
err.span_label(cause.span, "return type is not `()`");
}
ObligationCauseCode::BlockTailExpression(blk_id) => {
ObligationCauseCode::BlockTailExpression(blk_id, ..) => {
let parent_id = fcx.tcx.hir().parent_id(blk_id);
err = self.report_return_mismatched_types(
cause,
Expand Down Expand Up @@ -1751,7 +1751,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
) && !in_external_macro(fcx.tcx.sess, cond_expr.span)
&& !matches!(
cond_expr.kind,
hir::ExprKind::Match(.., hir::MatchSource::TryDesugar)
hir::ExprKind::Match(.., hir::MatchSource::TryDesugar(_))
)
{
err.span_label(cond_expr.span, "expected this to be `()`");
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1580,7 +1580,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let coerce = ctxt.coerce.as_mut().unwrap();
if let Some((tail_expr, tail_expr_ty)) = tail_expr_ty {
let span = self.get_expr_coercion_span(tail_expr);
let cause = self.cause(span, ObligationCauseCode::BlockTailExpression(blk.hir_id));
let cause = self.cause(
span,
ObligationCauseCode::BlockTailExpression(blk.hir_id, hir::MatchSource::Normal),
);
let ty_for_diagnostic = coerce.merged_ty();
// We use coerce_inner here because we want to augment the error
// suggesting to wrap the block in square brackets if it might've
Expand Down
84 changes: 67 additions & 17 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -743,6 +743,35 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
ObligationCauseCode::Pattern { origin_expr: false, span: Some(span), .. } => {
err.span_label(span, "expected due to this");
}
ObligationCauseCode::BlockTailExpression(
_,
hir::MatchSource::TryDesugar(scrut_hir_id),
) => {
if let Some(ty::error::ExpectedFound { expected, .. }) = exp_found {
let scrut_expr = self.tcx.hir().expect_expr(scrut_hir_id);
let scrut_ty = if let hir::ExprKind::Call(_, args) = &scrut_expr.kind {
let arg_expr = args.first().expect("try desugaring call w/out arg");
self.typeck_results.as_ref().and_then(|typeck_results| {
typeck_results.expr_ty_opt(arg_expr)
})
} else {
bug!("try desugaring w/out call expr as scrutinee");
};

match scrut_ty {
Some(ty) if expected == ty => {
let source_map = self.tcx.sess.source_map();
err.span_suggestion(
source_map.end_point(cause.span()),
"try removing this `?`",
"",
Applicability::MachineApplicable,
);
}
_ => {}
}
}
},
ObligationCauseCode::MatchExpressionArm(box MatchExpressionArmCause {
arm_block_id,
arm_span,
Expand All @@ -752,12 +781,11 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
prior_arm_ty,
source,
ref prior_arms,
scrut_hir_id,
opt_suggest_box_span,
scrut_span,
..
}) => match source {
hir::MatchSource::TryDesugar => {
hir::MatchSource::TryDesugar(scrut_hir_id) => {
if let Some(ty::error::ExpectedFound { expected, .. }) = exp_found {
let scrut_expr = self.tcx.hir().expect_expr(scrut_hir_id);
let scrut_ty = if let hir::ExprKind::Call(_, args) = &scrut_expr.kind {
Expand Down Expand Up @@ -1973,7 +2001,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
trace: &TypeTrace<'tcx>,
terr: TypeError<'tcx>,
) -> Vec<TypeErrorAdditionalDiags> {
use crate::traits::ObligationCauseCode::MatchExpressionArm;
use crate::traits::ObligationCauseCode::{BlockTailExpression, MatchExpressionArm};
let mut suggestions = Vec::new();
let span = trace.cause.span();
let values = self.resolve_vars_if_possible(trace.values);
Expand All @@ -1991,11 +2019,17 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
// specify a byte literal
(ty::Uint(ty::UintTy::U8), ty::Char) => {
if let Ok(code) = self.tcx.sess().source_map().span_to_snippet(span)
&& let Some(code) = code.strip_prefix('\'').and_then(|s| s.strip_suffix('\''))
&& !code.starts_with("\\u") // forbid all Unicode escapes
&& code.chars().next().is_some_and(|c| c.is_ascii()) // forbids literal Unicode characters beyond ASCII
&& let Some(code) =
code.strip_prefix('\'').and_then(|s| s.strip_suffix('\''))
// forbid all Unicode escapes
&& !code.starts_with("\\u")
// forbids literal Unicode characters beyond ASCII
&& code.chars().next().is_some_and(|c| c.is_ascii())
{
suggestions.push(TypeErrorAdditionalDiags::MeantByteLiteral { span, code: escape_literal(code) })
suggestions.push(TypeErrorAdditionalDiags::MeantByteLiteral {
span,
code: escape_literal(code),
})
}
}
// If a character was expected and the found expression is a string literal
Expand All @@ -2006,7 +2040,10 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
&& let Some(code) = code.strip_prefix('"').and_then(|s| s.strip_suffix('"'))
&& code.chars().count() == 1
{
suggestions.push(TypeErrorAdditionalDiags::MeantCharLiteral { span, code: escape_literal(code) })
suggestions.push(TypeErrorAdditionalDiags::MeantCharLiteral {
span,
code: escape_literal(code),
})
}
}
// If a string was expected and the found expression is a character literal,
Expand All @@ -2016,7 +2053,10 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
if let Some(code) =
code.strip_prefix('\'').and_then(|s| s.strip_suffix('\''))
{
suggestions.push(TypeErrorAdditionalDiags::MeantStrLiteral { span, code: escape_literal(code) })
suggestions.push(TypeErrorAdditionalDiags::MeantStrLiteral {
span,
code: escape_literal(code),
})
}
}
}
Expand All @@ -2025,17 +2065,24 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
(ty::Bool, ty::Tuple(list)) => if list.len() == 0 {
suggestions.extend(self.suggest_let_for_letchains(&trace.cause, span));
}
(ty::Array(_, _), ty::Array(_, _)) => suggestions.extend(self.suggest_specify_actual_length(terr, trace, span)),
(ty::Array(_, _), ty::Array(_, _)) => {
suggestions.extend(self.suggest_specify_actual_length(terr, trace, span))
}
_ => {}
}
}
let code = trace.cause.code();
if let &MatchExpressionArm(box MatchExpressionArmCause { source, .. }) = code
&& let hir::MatchSource::TryDesugar = source
&& let Some((expected_ty, found_ty, _, _)) = self.values_str(trace.values)
{
suggestions.push(TypeErrorAdditionalDiags::TryCannotConvert { found: found_ty.content(), expected: expected_ty.content() });
}
if let &(MatchExpressionArm(box MatchExpressionArmCause { source, .. })
| BlockTailExpression(.., source)
) = code
&& let hir::MatchSource::TryDesugar(_) = source
&& let Some((expected_ty, found_ty, _, _)) = self.values_str(trace.values)
{
suggestions.push(TypeErrorAdditionalDiags::TryCannotConvert {
found: found_ty.content(),
expected: expected_ty.content(),
});
}
suggestions
}

Expand Down Expand Up @@ -2905,8 +2952,11 @@ impl<'tcx> ObligationCauseExt<'tcx> for ObligationCause<'tcx> {
CompareImplItemObligation { kind: ty::AssocKind::Const, .. } => {
ObligationCauseFailureCode::ConstCompat { span, subdiags }
}
BlockTailExpression(.., hir::MatchSource::TryDesugar(_)) => {
ObligationCauseFailureCode::TryCompat { span, subdiags }
}
MatchExpressionArm(box MatchExpressionArmCause { source, .. }) => match source {
hir::MatchSource::TryDesugar => {
hir::MatchSource::TryDesugar(_) => {
ObligationCauseFailureCode::TryCompat { span, subdiags }
}
_ => ObligationCauseFailureCode::MatchCompat { span, subdiags },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {

if let SubregionOrigin::Subtype(box TypeTrace { cause, .. }) = sub_origin {
if let ObligationCauseCode::ReturnValue(hir_id)
| ObligationCauseCode::BlockTailExpression(hir_id) = cause.code()
| ObligationCauseCode::BlockTailExpression(hir_id, ..) = cause.code()
{
let parent_id = tcx.hir().get_parent_item(*hir_id);
if let Some(fn_decl) = tcx.hir().fn_decl_by_hir_id(parent_id.into()) {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/thir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ pub enum ExprKind<'tcx> {
/// A `match` expression.
Match {
scrutinee: ExprId,
scrutinee_hir_id: hir::HirId,
arms: Box<[ArmId]>,
},
/// A block.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/thir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ pub fn walk_expr<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, expr: &Exp
visitor.visit_expr(&visitor.thir()[expr]);
}
Loop { body } => visitor.visit_expr(&visitor.thir()[body]),
Match { scrutinee, ref arms } => {
Match { scrutinee, ref arms, .. } => {
visitor.visit_expr(&visitor.thir()[scrutinee]);
for &arm in &**arms {
visitor.visit_arm(&visitor.thir()[arm]);
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ pub enum ObligationCauseCode<'tcx> {
OpaqueReturnType(Option<(Ty<'tcx>, Span)>),

/// Block implicit return
BlockTailExpression(hir::HirId),
BlockTailExpression(hir::HirId, hir::MatchSource),

/// #[feature(trivial_bounds)] is not enabled
TrivialBound,
Expand Down Expand Up @@ -543,7 +543,6 @@ pub struct MatchExpressionArmCause<'tcx> {
pub scrut_span: Span,
pub source: hir::MatchSource,
pub prior_arms: Vec<Span>,
pub scrut_hir_id: hir::HirId,
pub opt_suggest_box_span: Option<Span>,
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ impl<'tcx, 'body> ParseCtxt<'tcx, 'body> {
let target = self.parse_block(args[1])?;
self.parse_call(args[2], destination, target)
},
ExprKind::Match { scrutinee, arms } => {
ExprKind::Match { scrutinee, arms, .. } => {
let discr = self.parse_operand(*scrutinee)?;
self.parse_match(arms, expr.span).map(|t| TerminatorKind::SwitchInt { discr, targets: t })
},
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/build/expr/into.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
ExprKind::Block { block: ast_block } => {
this.ast_block(destination, block, ast_block, source_info)
}
ExprKind::Match { scrutinee, ref arms } => {
ExprKind::Match { scrutinee, ref arms, .. } => {
this.match_expr(destination, expr_span, block, &this.thir[scrutinee], arms)
}
ExprKind::If { cond, then, else_opt, if_then_scope } => {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_build/src/thir/cx/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,7 @@ impl<'tcx> Cx<'tcx> {
},
hir::ExprKind::Match(ref discr, ref arms, _) => ExprKind::Match {
scrutinee: self.mirror_expr(discr),
scrutinee_hir_id: discr.hir_id,
arms: arms.iter().map(|a| self.convert_arm(a)).collect(),
},
hir::ExprKind::Loop(ref body, ..) => {
Expand Down
8 changes: 5 additions & 3 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,12 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for MatchVisitor<'a, '_, 'tcx> {
});
return;
}
ExprKind::Match { scrutinee, box ref arms } => {
ExprKind::Match { scrutinee, scrutinee_hir_id, box ref arms } => {
let source = match ex.span.desugaring_kind() {
Some(DesugaringKind::ForLoop) => hir::MatchSource::ForLoopDesugar,
Some(DesugaringKind::QuestionMark) => hir::MatchSource::TryDesugar,
Some(DesugaringKind::QuestionMark) => {
hir::MatchSource::TryDesugar(scrutinee_hir_id)
}
Some(DesugaringKind::Await) => hir::MatchSource::AwaitDesugar,
_ => hir::MatchSource::Normal,
};
Expand Down Expand Up @@ -277,7 +279,7 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
| hir::MatchSource::FormatArgs => report_arm_reachability(&cx, &report),
// Unreachable patterns in try and await expressions occur when one of
// the arms are an uninhabited type. Which is OK.
hir::MatchSource::AwaitDesugar | hir::MatchSource::TryDesugar => {}
hir::MatchSource::AwaitDesugar | hir::MatchSource::TryDesugar(_) => {}
}

// Check if the match is exhaustive.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/thir/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ impl<'a, 'tcx> ThirPrinter<'a, 'tcx> {
print_indented!(self, format!("pat: {:?}", pat), depth_lvl + 1);
print_indented!(self, "}", depth_lvl);
}
Match { scrutinee, arms } => {
Match { scrutinee, arms, .. } => {
print_indented!(self, "Match {", depth_lvl);
print_indented!(self, "scrutinee:", depth_lvl + 1);
self.print_expr(*scrutinee, depth_lvl + 2);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_passes/src/check_const.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ impl NonConstExpr {

Self::Loop(ForLoop) | Self::Match(ForLoopDesugar) => &[sym::const_for],

Self::Match(TryDesugar) => &[sym::const_try],
Self::Match(TryDesugar(_)) => &[sym::const_try],

// All other expressions are allowed.
Self::Loop(Loop | While) | Self::Match(Normal | FormatArgs) => &[],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2700,7 +2700,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
| ObligationCauseCode::MatchImpl(..)
| ObligationCauseCode::ReturnType
| ObligationCauseCode::ReturnValue(_)
| ObligationCauseCode::BlockTailExpression(_)
| ObligationCauseCode::BlockTailExpression(..)
| ObligationCauseCode::AwaitableExpr(_)
| ObligationCauseCode::ForLoopIterator
| ObligationCauseCode::QuestionMark
Expand Down
3 changes: 2 additions & 1 deletion src/tools/clippy/clippy_lints/src/dereference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -802,7 +802,8 @@ fn in_postfix_position<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> boo
match parent.kind {
ExprKind::Call(child, _) | ExprKind::MethodCall(_, child, _, _) | ExprKind::Index(child, _, _)
if child.hir_id == e.hir_id => true,
ExprKind::Field(_, _) | ExprKind::Match(_, _, MatchSource::TryDesugar | MatchSource::AwaitDesugar) => true,
ExprKind::Match(.., MatchSource::TryDesugar(_) | MatchSource::AwaitDesugar)
| ExprKind::Field(_, _) => true,
_ => false,
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1038,7 +1038,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
wild_in_or_pats::check(cx, arms);
}

if source == MatchSource::TryDesugar {
if let MatchSource::TryDesugar(_) = source {
try_err::check(cx, expr, ex);
}

Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/matches/try_err.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, scrutine

/// Finds function return type by examining return expressions in match arms.
fn find_return_type<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx ExprKind<'_>) -> Option<Ty<'tcx>> {
if let ExprKind::Match(_, arms, MatchSource::TryDesugar) = expr {
if let ExprKind::Match(_, arms, MatchSource::TryDesugar(_)) = expr {
for arm in *arms {
if let ExprKind::Ret(Some(ret)) = arm.body.kind {
return Some(cx.typeck_results().expr_ty(ret));
Expand Down
Loading

0 comments on commit d53aff0

Please sign in to comment.