Skip to content

Commit 55f8c66

Browse files
estebankestebank
committed
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.
1 parent 4cea2bc commit 55f8c66

File tree

12 files changed

+129
-21
lines changed

12 files changed

+129
-21
lines changed

‎compiler/rustc_hir_typeck/src/_match.rs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
107107
let (span, code) = match prior_arm {
108108
// The reason for the first arm to fail is not that the match arms diverge,
109109
// but rather that there's a prior obligation that doesn't hold.
110-
None => (arm_span, ObligationCauseCode::BlockTailExpression(arm.body.hir_id)),
110+
None => (
111+
arm_span,
112+
ObligationCauseCode::BlockTailExpression(
113+
arm.body.hir_id,
114+
scrut.hir_id,
115+
match_src,
116+
),
117+
),
111118
Some((prior_arm_block_id, prior_arm_ty, prior_arm_span)) => (
112119
expr.span,
113120
ObligationCauseCode::MatchExpressionArm(Box::new(MatchExpressionArmCause {
@@ -145,7 +152,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
145152
other_arms.remove(0);
146153
}
147154

148-
prior_arm = Some((arm_block_id, arm_ty, arm_span));
155+
if !arm_ty.is_never() {
156+
prior_arm = Some((arm_block_id, arm_ty, arm_span));
157+
}
149158
}
150159

151160
// If all of the arms in the `match` diverge,

‎compiler/rustc_hir_typeck/src/coercion.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1603,7 +1603,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
16031603
);
16041604
err.span_label(cause.span, "return type is not `()`");
16051605
}
1606-
ObligationCauseCode::BlockTailExpression(blk_id) => {
1606+
ObligationCauseCode::BlockTailExpression(blk_id, ..) => {
16071607
let parent_id = fcx.tcx.hir().parent_id(blk_id);
16081608
err = self.report_return_mismatched_types(
16091609
cause,

‎compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1580,7 +1580,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
15801580
let coerce = ctxt.coerce.as_mut().unwrap();
15811581
if let Some((tail_expr, tail_expr_ty)) = tail_expr_ty {
15821582
let span = self.get_expr_coercion_span(tail_expr);
1583-
let cause = self.cause(span, ObligationCauseCode::BlockTailExpression(blk.hir_id));
1583+
let cause = self.cause(
1584+
span,
1585+
ObligationCauseCode::BlockTailExpression(
1586+
blk.hir_id,
1587+
blk.hir_id,
1588+
hir::MatchSource::Normal,
1589+
),
1590+
);
15841591
let ty_for_diagnostic = coerce.merged_ty();
15851592
// We use coerce_inner here because we want to augment the error
15861593
// suggesting to wrap the block in square brackets if it might've

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

Lines changed: 66 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -743,6 +743,36 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
743743
ObligationCauseCode::Pattern { origin_expr: false, span: Some(span), .. } => {
744744
err.span_label(span, "expected due to this");
745745
}
746+
ObligationCauseCode::BlockTailExpression(
747+
_,
748+
scrut_hir_id,
749+
hir::MatchSource::TryDesugar,
750+
) => {
751+
if let Some(ty::error::ExpectedFound { expected, .. }) = exp_found {
752+
let scrut_expr = self.tcx.hir().expect_expr(scrut_hir_id);
753+
let scrut_ty = if let hir::ExprKind::Call(_, args) = &scrut_expr.kind {
754+
let arg_expr = args.first().expect("try desugaring call w/out arg");
755+
self.typeck_results.as_ref().and_then(|typeck_results| {
756+
typeck_results.expr_ty_opt(arg_expr)
757+
})
758+
} else {
759+
bug!("try desugaring w/out call expr as scrutinee");
760+
};
761+
762+
match scrut_ty {
763+
Some(ty) if expected == ty => {
764+
let source_map = self.tcx.sess.source_map();
765+
err.span_suggestion(
766+
source_map.end_point(cause.span()),
767+
"try removing this `?`",
768+
"",
769+
Applicability::MachineApplicable,
770+
);
771+
}
772+
_ => {}
773+
}
774+
}
775+
},
746776
ObligationCauseCode::MatchExpressionArm(box MatchExpressionArmCause {
747777
arm_block_id,
748778
arm_span,
@@ -1973,7 +2003,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
19732003
trace: &TypeTrace<'tcx>,
19742004
terr: TypeError<'tcx>,
19752005
) -> Vec<TypeErrorAdditionalDiags> {
1976-
use crate::traits::ObligationCauseCode::MatchExpressionArm;
2006+
use crate::traits::ObligationCauseCode::{BlockTailExpression, MatchExpressionArm};
19772007
let mut suggestions = Vec::new();
19782008
let span = trace.cause.span();
19792009
let values = self.resolve_vars_if_possible(trace.values);
@@ -1991,11 +2021,17 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
19912021
// specify a byte literal
19922022
(ty::Uint(ty::UintTy::U8), ty::Char) => {
19932023
if let Ok(code) = self.tcx.sess().source_map().span_to_snippet(span)
1994-
&& let Some(code) = code.strip_prefix('\'').and_then(|s| s.strip_suffix('\''))
1995-
&& !code.starts_with("\\u") // forbid all Unicode escapes
1996-
&& code.chars().next().is_some_and(|c| c.is_ascii()) // forbids literal Unicode characters beyond ASCII
2024+
&& let Some(code) =
2025+
code.strip_prefix('\'').and_then(|s| s.strip_suffix('\''))
2026+
// forbid all Unicode escapes
2027+
&& !code.starts_with("\\u")
2028+
// forbids literal Unicode characters beyond ASCII
2029+
&& code.chars().next().is_some_and(|c| c.is_ascii())
19972030
{
1998-
suggestions.push(TypeErrorAdditionalDiags::MeantByteLiteral { span, code: escape_literal(code) })
2031+
suggestions.push(TypeErrorAdditionalDiags::MeantByteLiteral {
2032+
span,
2033+
code: escape_literal(code),
2034+
})
19992035
}
20002036
}
20012037
// If a character was expected and the found expression is a string literal
@@ -2006,7 +2042,10 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
20062042
&& let Some(code) = code.strip_prefix('"').and_then(|s| s.strip_suffix('"'))
20072043
&& code.chars().count() == 1
20082044
{
2009-
suggestions.push(TypeErrorAdditionalDiags::MeantCharLiteral { span, code: escape_literal(code) })
2045+
suggestions.push(TypeErrorAdditionalDiags::MeantCharLiteral {
2046+
span,
2047+
code: escape_literal(code),
2048+
})
20102049
}
20112050
}
20122051
// If a string was expected and the found expression is a character literal,
@@ -2016,7 +2055,10 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
20162055
if let Some(code) =
20172056
code.strip_prefix('\'').and_then(|s| s.strip_suffix('\''))
20182057
{
2019-
suggestions.push(TypeErrorAdditionalDiags::MeantStrLiteral { span, code: escape_literal(code) })
2058+
suggestions.push(TypeErrorAdditionalDiags::MeantStrLiteral {
2059+
span,
2060+
code: escape_literal(code),
2061+
})
20202062
}
20212063
}
20222064
}
@@ -2025,17 +2067,24 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
20252067
(ty::Bool, ty::Tuple(list)) => if list.len() == 0 {
20262068
suggestions.extend(self.suggest_let_for_letchains(&trace.cause, span));
20272069
}
2028-
(ty::Array(_, _), ty::Array(_, _)) => suggestions.extend(self.suggest_specify_actual_length(terr, trace, span)),
2070+
(ty::Array(_, _), ty::Array(_, _)) => {
2071+
suggestions.extend(self.suggest_specify_actual_length(terr, trace, span))
2072+
}
20292073
_ => {}
20302074
}
20312075
}
20322076
let code = trace.cause.code();
2033-
if let &MatchExpressionArm(box MatchExpressionArmCause { source, .. }) = code
2034-
&& let hir::MatchSource::TryDesugar = source
2035-
&& let Some((expected_ty, found_ty, _, _)) = self.values_str(trace.values)
2036-
{
2037-
suggestions.push(TypeErrorAdditionalDiags::TryCannotConvert { found: found_ty.content(), expected: expected_ty.content() });
2038-
}
2077+
if let &(MatchExpressionArm(box MatchExpressionArmCause { source, .. })
2078+
| BlockTailExpression(.., source)
2079+
) = code
2080+
&& let hir::MatchSource::TryDesugar = source
2081+
&& let Some((expected_ty, found_ty, _, _)) = self.values_str(trace.values)
2082+
{
2083+
suggestions.push(TypeErrorAdditionalDiags::TryCannotConvert {
2084+
found: found_ty.content(),
2085+
expected: expected_ty.content(),
2086+
});
2087+
}
20392088
suggestions
20402089
}
20412090

@@ -2905,6 +2954,9 @@ impl<'tcx> ObligationCauseExt<'tcx> for ObligationCause<'tcx> {
29052954
CompareImplItemObligation { kind: ty::AssocKind::Const, .. } => {
29062955
ObligationCauseFailureCode::ConstCompat { span, subdiags }
29072956
}
2957+
BlockTailExpression(.., hir::MatchSource::TryDesugar) => {
2958+
ObligationCauseFailureCode::TryCompat { span, subdiags }
2959+
}
29082960
MatchExpressionArm(box MatchExpressionArmCause { source, .. }) => match source {
29092961
hir::MatchSource::TryDesugar => {
29102962
ObligationCauseFailureCode::TryCompat { span, subdiags }

‎compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
146146

147147
if let SubregionOrigin::Subtype(box TypeTrace { cause, .. }) = sub_origin {
148148
if let ObligationCauseCode::ReturnValue(hir_id)
149-
| ObligationCauseCode::BlockTailExpression(hir_id) = cause.code()
149+
| ObligationCauseCode::BlockTailExpression(hir_id, ..) = cause.code()
150150
{
151151
let parent_id = tcx.hir().get_parent_item(*hir_id);
152152
if let Some(fn_decl) = tcx.hir().fn_decl_by_hir_id(parent_id.into()) {

‎compiler/rustc_middle/src/traits/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ pub enum ObligationCauseCode<'tcx> {
402402
OpaqueReturnType(Option<(Ty<'tcx>, Span)>),
403403

404404
/// Block implicit return
405-
BlockTailExpression(hir::HirId),
405+
BlockTailExpression(hir::HirId, hir::HirId, hir::MatchSource),
406406

407407
/// #[feature(trivial_bounds)] is not enabled
408408
TrivialBound,

‎compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2700,7 +2700,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
27002700
| ObligationCauseCode::MatchImpl(..)
27012701
| ObligationCauseCode::ReturnType
27022702
| ObligationCauseCode::ReturnValue(_)
2703-
| ObligationCauseCode::BlockTailExpression(_)
2703+
| ObligationCauseCode::BlockTailExpression(..)
27042704
| ObligationCauseCode::AwaitableExpr(_)
27052705
| ObligationCauseCode::ForLoopIterator
27062706
| ObligationCauseCode::QuestionMark

‎tests/ui/did_you_mean/compatible-variants.stderr‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ LL + Some(())
6161
error[E0308]: `?` operator has incompatible types
6262
--> $DIR/compatible-variants.rs:35:5
6363
|
64+
LL | fn d() -> Option<()> {
65+
| ---------- expected `Option<()>` because of return type
6466
LL | c()?
6567
| ^^^^ expected `Option<()>`, found `()`
6668
|

‎tests/ui/issues/issue-51632-try-desugar-incompatible-types.stderr‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
error[E0308]: `?` operator has incompatible types
22
--> $DIR/issue-51632-try-desugar-incompatible-types.rs:8:5
33
|
4+
LL | fn forbidden_narratives() -> Result<isize, ()> {
5+
| ----------------- expected `Result<isize, ()>` because of return type
46
LL | missing_discourses()?
57
| ^^^^^^^^^^^^^^^^^^^^^ expected `Result<isize, ()>`, found `isize`
68
|

‎tests/ui/match/non-first-arm-doesnt-match-expected-return-type.rs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
#![allow(unused)]
2+
3+
fn test(shouldwe: Option<u32>, shouldwe2: Option<u32>) -> u32 {
4+
//~^ NOTE expected `u32` because of return type
5+
match shouldwe {
6+
Some(val) => {
7+
match shouldwe2 {
8+
Some(val) => {
9+
return val;
10+
}
11+
None => (), //~ ERROR mismatched types
12+
//~^ NOTE expected `u32`, found `()`
13+
}
14+
}
15+
None => return 12,
16+
}
17+
}
18+
19+
fn main() {
20+
println!("returned {}", test(None, Some(5)));
21+
}

0 commit comments

Comments
 (0)