Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Aug 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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));
estebank marked this conversation as resolved.
Show resolved Hide resolved
}
}

// 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 @@ -733,6 +733,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