Skip to content

Commit

Permalink
Do if-expression obligation stuff less eagerly
Browse files Browse the repository at this point in the history
  • Loading branch information
@compiler-errors
compiler-errors committed Jul 21, 2022
1 parent 3d9dd68 commit 99c3257
Show file tree
Hide file tree
Showing 11 changed files with 411 additions and 350 deletions.
10 changes: 10 additions & 0 deletions compiler/rustc_hir/src/hir.rs
View file
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
335 changes: 297 additions & 38 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
View file

Large diffs are not rendered by default.

16 changes: 9 additions & 7 deletions compiler/rustc_middle/src/traits/mod.rs
View file
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 @@ -498,12 +498,14 @@ pub struct MatchExpressionArmCause<'tcx> {
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
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,
}
124 changes: 55 additions & 69 deletions compiler/rustc_typeck/src/check/_match.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,13 +216,17 @@ impl<'a, 'tcx> FnCtxt<'a, '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)
(
self.find_block_span(blk),
prior_arm_ty
.and_then(|prior_arm_ty| self.could_remove_semicolon(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));
let semi_span_prev = self.could_remove_semicolon(blk, arm_ty);
semi_span = semi_span_prev;
}
}
Expand Down Expand Up @@ -313,7 +317,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 +343,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 +484,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
84 changes: 2 additions & 82 deletions compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,15 @@ use rustc_middle::ty::{
};
use rustc_session::lint;
use rustc_span::hygiene::DesugaringKind;
use rustc_span::source_map::{original_sp, DUMMY_SP};
use rustc_span::symbol::{kw, sym, Ident};
use rustc_span::{self, BytePos, Span};
use rustc_span::{Span, DUMMY_SP};
use rustc_trait_selection::infer::InferCtxtExt as _;
use rustc_trait_selection::traits::error_reporting::InferCtxtExt as _;
use rustc_trait_selection::traits::{
self, ObligationCause, ObligationCauseCode, StatementAsExpression, TraitEngine, TraitEngineExt,
self, ObligationCause, ObligationCauseCode, TraitEngine, TraitEngineExt,
};

use std::collections::hash_map::Entry;
use std::iter;
use std::slice;

impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Expand Down Expand Up @@ -1059,84 +1057,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
));
}

pub(in super::super) fn could_remove_semicolon(
&self,
blk: &'tcx hir::Block<'tcx>,
expected_ty: Ty<'tcx>,
) -> Option<(Span, StatementAsExpression)> {
// Be helpful when the user wrote `{... expr;}` and
// taking the `;` off is enough to fix the error.
let last_stmt = blk.stmts.last()?;
let hir::StmtKind::Semi(ref last_expr) = last_stmt.kind else {
return None;
};
let last_expr_ty = self.node_ty(last_expr.hir_id);
let needs_box = match (last_expr_ty.kind(), expected_ty.kind()) {
(ty::Opaque(last_def_id, _), ty::Opaque(exp_def_id, _))
if last_def_id == exp_def_id =>
{
StatementAsExpression::CorrectType
}
(ty::Opaque(last_def_id, last_bounds), ty::Opaque(exp_def_id, exp_bounds)) => {
debug!(
"both opaque, likely future {:?} {:?} {:?} {:?}",
last_def_id, last_bounds, exp_def_id, exp_bounds
);

let last_local_id = last_def_id.as_local()?;
let exp_local_id = exp_def_id.as_local()?;

match (
&self.tcx.hir().expect_item(last_local_id).kind,
&self.tcx.hir().expect_item(exp_local_id).kind,
) {
(
hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds: last_bounds, .. }),
hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds: exp_bounds, .. }),
) if iter::zip(*last_bounds, *exp_bounds).all(|(left, right)| {
match (left, right) {
(
hir::GenericBound::Trait(tl, ml),
hir::GenericBound::Trait(tr, mr),
) if tl.trait_ref.trait_def_id() == tr.trait_ref.trait_def_id()
&& ml == mr =>
{
true
}
(
hir::GenericBound::LangItemTrait(langl, _, _, argsl),
hir::GenericBound::LangItemTrait(langr, _, _, argsr),
) if langl == langr => {
// FIXME: consider the bounds!
debug!("{:?} {:?}", argsl, argsr);
true
}
_ => false,
}
}) =>
{
StatementAsExpression::NeedsBoxing
}
_ => StatementAsExpression::CorrectType,
}
}
_ => StatementAsExpression::CorrectType,
};
if (matches!(last_expr_ty.kind(), ty::Error(_))
|| self.can_sub(self.param_env, last_expr_ty, expected_ty).is_err())
&& matches!(needs_box, StatementAsExpression::CorrectType)
{
return None;
}
let span = if last_stmt.span.from_expansion() {
let mac_call = original_sp(last_stmt.span, blk.span);
self.tcx.sess.source_map().mac_call_stmt_semi_span(mac_call)?
} else {
last_stmt.span.with_lo(last_stmt.span.hi() - BytePos(1))
};
Some((span, needs_box))
}

// Instantiates the given path, which must refer to an item with the given
// number of type parameters and type.
#[instrument(skip(self, span), level = "debug")]
Expand Down
Loading

0 comments on commit 99c3257

Please sign in to comment.