Skip to content

Improve suggestions for returning binding #99539

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jul 22, 2022
Merged

Improve suggestions for returning binding #99539

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3d9dd68
Resolve vars in same_type_modulo_infer
compiler-errors Jul 20, 2022
99c3257
Do if-expression obligation stuff less eagerly
compiler-errors Jul 21, 2022
8926dac
And for patterns too
compiler-errors Jul 21, 2022
cd89978
Generalize same_type_modulo_infer
compiler-errors Jul 21, 2022
3eef023
Address more nits
compiler-errors Jul 21, 2022
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
10 changes: 10 additions & 0 deletions compiler/rustc_hir/src/hir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -954,6 +954,16 @@ pub struct Block<'hir> {
pub targeted_by_break: bool,
}

impl<'hir> Block<'hir> {
pub fn peel_blocks(&self) -> &Block<'hir> {
let mut block = self;
while let Some(Expr { kind: ExprKind::Block(inner_block, _), .. }) = block.expr {
block = inner_block;
}
block
}
}

#[derive(Debug, HashStable_Generic)]
pub struct Pat<'hir> {
#[stable_hasher(ignore)]
Expand Down
514 changes: 397 additions & 117 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
View file
Open in desktop

Large diffs are not rendered by default.

23 changes: 14 additions & 9 deletions compiler/rustc_middle/src/traits/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ pub enum ObligationCauseCode<'tcx> {
ConstPatternStructural,

/// Computing common supertype in an if expression
IfExpression(Box<IfExpressionCause>),
IfExpression(Box<IfExpressionCause<'tcx>>),

/// Computing common supertype of an if expression with no else counter-part
IfExpressionWithNoElse,
Expand Down Expand Up @@ -488,22 +488,27 @@ impl<'tcx> ty::Lift<'tcx> for StatementAsExpression {

#[derive(Clone, Debug, PartialEq, Eq, Hash, Lift)]
pub struct MatchExpressionArmCause<'tcx> {
pub arm_block_id: Option<hir::HirId>,
pub arm_ty: Ty<'tcx>,
pub arm_span: Span,
pub prior_arm_block_id: Option<hir::HirId>,
pub prior_arm_ty: Ty<'tcx>,
pub prior_arm_span: Span,
pub scrut_span: Span,
pub semi_span: Option<(Span, StatementAsExpression)>,
pub source: hir::MatchSource,
pub prior_arms: Vec<Span>,
pub last_ty: Ty<'tcx>,
pub scrut_hir_id: hir::HirId,
pub opt_suggest_box_span: Option<Span>,
}

#[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub struct IfExpressionCause {
pub then: Span,
pub else_sp: Span,
pub outer: Option<Span>,
pub semicolon: Option<(Span, StatementAsExpression)>,
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
#[derive(Lift, TypeFoldable, TypeVisitable)]
pub struct IfExpressionCause<'tcx> {
pub then_id: hir::HirId,
pub else_id: hir::HirId,
pub then_ty: Ty<'tcx>,
pub else_ty: Ty<'tcx>,
pub outer_span: Option<Span>,
pub opt_suggest_box_span: Option<Span>,
}

Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_middle/src/traits/structural_impls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ impl<N: fmt::Debug> fmt::Debug for traits::ImplSourceConstDestructData<N> {
// Lift implementations

TrivialTypeTraversalAndLiftImpls! {
super::IfExpressionCause,
super::ImplSourceDiscriminantKindData,
super::ImplSourcePointeeData,
}
3 changes: 1 addition & 2 deletions compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ use rustc_hir::intravisit::Visitor;
use rustc_hir::GenericParam;
use rustc_hir::Item;
use rustc_hir::Node;
use rustc_infer::infer::error_reporting::same_type_modulo_infer;
use rustc_infer::traits::TraitEngine;
use rustc_middle::traits::select::OverflowError;
use rustc_middle::ty::abstract_const::NotConstEvaluatable;
Expand Down Expand Up @@ -640,7 +639,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
if expected.len() == 1 { "" } else { "s" },
)
);
} else if !same_type_modulo_infer(given_ty, expected_ty) {
} else if !self.same_type_modulo_infer(given_ty, expected_ty) {
// Print type mismatch
let (expected_args, given_args) =
self.cmp(given_ty, expected_ty);
Expand Down
169 changes: 68 additions & 101 deletions compiler/rustc_typeck/src/check/_match.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use rustc_span::Span;
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
use rustc_trait_selection::traits::{
IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode,
StatementAsExpression,
};

impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Expand Down Expand Up @@ -75,8 +74,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
};

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

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

let (arm_span, semi_span) =
self.get_appropriate_arm_semicolon_removal_span(&arms, i, prior_arm_ty, arm_ty);
let (span, code) = match i {
let (arm_block_id, arm_span) = if let hir::ExprKind::Block(blk, _) = arm.body.kind {
(Some(blk.hir_id), self.find_block_span(blk))
} else {
(None, arm.body.span)
};

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.
0 => (arm_span, ObligationCauseCode::BlockTailExpression(arm.body.hir_id)),
_ => (
None => (arm_span, ObligationCauseCode::BlockTailExpression(arm.body.hir_id)),
Some((prior_arm_block_id, prior_arm_ty, prior_arm_span)) => (
expr.span,
ObligationCauseCode::MatchExpressionArm(Box::new(MatchExpressionArmCause {
arm_block_id,
arm_span,
arm_ty,
prior_arm_block_id,
prior_arm_ty,
prior_arm_span,
scrut_span: scrut.span,
semi_span,
source: match_src,
prior_arms: other_arms.clone(),
last_ty: prior_arm_ty.unwrap(),
scrut_hir_id: scrut.hir_id,
opt_suggest_box_span,
})),
Expand Down Expand Up @@ -139,7 +145,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let ret_ty = ret_coercion.borrow().expected_ty();
let ret_ty = self.inh.infcx.shallow_resolve(ret_ty);
self.can_coerce(arm_ty, ret_ty)
&& prior_arm_ty.map_or(true, |t| self.can_coerce(t, ret_ty))
&& prior_arm.map_or(true, |(_, t, _)| self.can_coerce(t, ret_ty))
// The match arms need to unify for the case of `impl Trait`.
&& !matches!(ret_ty.kind(), ty::Opaque(..))
}
Expand Down Expand Up @@ -181,7 +187,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if other_arms.len() > 5 {
other_arms.remove(0);
}
prior_arm_ty = Some(arm_ty);

prior_arm = Some((arm_block_id, arm_ty, arm_span));
}

// If all of the arms in the `match` diverge,
Expand All @@ -207,28 +214,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
match_ty
}

fn get_appropriate_arm_semicolon_removal_span(
&self,
arms: &'tcx [hir::Arm<'tcx>],
i: usize,
prior_arm_ty: Option<Ty<'tcx>>,
arm_ty: Ty<'tcx>,
) -> (Span, Option<(Span, StatementAsExpression)>) {
let arm = &arms[i];
let (arm_span, mut semi_span) = if let hir::ExprKind::Block(blk, _) = &arm.body.kind {
self.find_block_span(blk, prior_arm_ty)
} else {
(arm.body.span, None)
};
if semi_span.is_none() && i > 0 {
if let hir::ExprKind::Block(blk, _) = &arms[i - 1].body.kind {
let (_, semi_span_prev) = self.find_block_span(blk, Some(arm_ty));
semi_span = semi_span_prev;
}
}
(arm_span, semi_span)
}

/// When the previously checked expression (the scrutinee) diverges,
/// warn the user about the match arms being unreachable.
fn warn_arms_when_scrutinee_diverges(&self, arms: &'tcx [hir::Arm<'tcx>]) {
Expand Down Expand Up @@ -313,7 +298,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
else_ty: Ty<'tcx>,
opt_suggest_box_span: Option<Span>,
) -> ObligationCause<'tcx> {
let mut outer_sp = if self.tcx.sess.source_map().is_multiline(span) {
let mut outer_span = if self.tcx.sess.source_map().is_multiline(span) {
// The `if`/`else` isn't in one line in the output, include some context to make it
// clear it is an if/else expression:
// ```
Expand All @@ -339,69 +324,67 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
None
};

let mut remove_semicolon = None;
let error_sp = if let ExprKind::Block(block, _) = &else_expr.kind {
let (error_sp, semi_sp) = self.find_block_span(block, Some(then_ty));
remove_semicolon = semi_sp;
if block.expr.is_none() && block.stmts.is_empty() {
// Avoid overlapping spans that aren't as readable:
// ```
// 2 | let x = if true {
// | _____________-
// 3 | | 3
// | | - expected because of this
// 4 | | } else {
// | |____________^
// 5 | ||
// 6 | || };
// | || ^
// | ||_____|
// | |______if and else have incompatible types
// | expected integer, found `()`
// ```
// by not pointing at the entire expression:
// ```
// 2 | let x = if true {
// | ------- `if` and `else` have incompatible types
// 3 | 3
// | - expected because of this
// 4 | } else {
// | ____________^
// 5 | |
// 6 | | };
// | |_____^ expected integer, found `()`
// ```
if outer_sp.is_some() {
outer_sp = Some(self.tcx.sess.source_map().guess_head_span(span));
}
let (error_sp, else_id) = if let ExprKind::Block(block, _) = &else_expr.kind {
let block = block.peel_blocks();

// Avoid overlapping spans that aren't as readable:
// ```
// 2 | let x = if true {
// | _____________-
// 3 | | 3
// | | - expected because of this
// 4 | | } else {
// | |____________^
// 5 | ||
// 6 | || };
// | || ^
// | ||_____|
// | |______if and else have incompatible types
// | expected integer, found `()`
// ```
// by not pointing at the entire expression:
// ```
// 2 | let x = if true {
// | ------- `if` and `else` have incompatible types
// 3 | 3
// | - expected because of this
// 4 | } else {
// | ____________^
// 5 | |
// 6 | | };
// | |_____^ expected integer, found `()`
// ```
if block.expr.is_none() && block.stmts.is_empty()
&& let Some(outer_span) = &mut outer_span
{
*outer_span = self.tcx.sess.source_map().guess_head_span(*outer_span);
}
error_sp

(self.find_block_span(block), block.hir_id)
} else {
// shouldn't happen unless the parser has done something weird
else_expr.span
(else_expr.span, else_expr.hir_id)
};

// Compute `Span` of `then` part of `if`-expression.
let then_sp = if let ExprKind::Block(block, _) = &then_expr.kind {
let (then_sp, semi_sp) = self.find_block_span(block, Some(else_ty));
remove_semicolon = remove_semicolon.or(semi_sp);
let then_id = if let ExprKind::Block(block, _) = &then_expr.kind {
let block = block.peel_blocks();
// Exclude overlapping spans
if block.expr.is_none() && block.stmts.is_empty() {
outer_sp = None; // same as in `error_sp`; cleanup output
outer_span = None;
}
then_sp
block.hir_id
} else {
// shouldn't happen unless the parser has done something weird
then_expr.span
then_expr.hir_id
};

// Finally construct the cause:
self.cause(
error_sp,
ObligationCauseCode::IfExpression(Box::new(IfExpressionCause {
then: then_sp,
else_sp: error_sp,
outer: outer_sp,
semicolon: remove_semicolon,
else_id,
then_id,
then_ty,
else_ty,
outer_span,
opt_suggest_box_span,
})),
)
Expand Down Expand Up @@ -482,22 +465,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

fn find_block_span(
&self,
block: &'tcx hir::Block<'tcx>,
expected_ty: Option<Ty<'tcx>>,
) -> (Span, Option<(Span, StatementAsExpression)>) {
if let Some(expr) = &block.expr {
(expr.span, None)
} else if let Some(stmt) = block.stmts.last() {
// possibly incorrect trailing `;` in the else arm
(stmt.span, expected_ty.and_then(|ty| self.could_remove_semicolon(block, ty)))
} else {
// empty block; point at its entirety
(block.span, None)
}
}

// When we have a `match` as a tail expression in a `fn` with a returned `impl Trait`
// we check if the different arms would work with boxed trait objects instead and
// provide a structured suggestion in that case.
Expand Down
Loading