Skip to content

Commit

Permalink
Review comments: use newtype instead of bool
Browse files Browse the repository at this point in the history
  • Loading branch information
@estebank
estebank committed Oct 23, 2020
1 parent 671d7c4 commit 62ba365
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 43 deletions.
5 changes: 3 additions & 2 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
View file
Expand Up @@ -54,6 +54,7 @@ use crate::infer::OriginalQueryValues;
use crate::traits::error_reporting::report_object_safety_error;
use crate::traits::{
IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode,
StatementAsExpression,
};

use rustc_data_structures::fx::{FxHashMap, FxHashSet};
Expand Down Expand Up @@ -689,7 +690,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
let msg = "`match` arms have incompatible types";
err.span_label(outer_error_span, msg);
if let Some((sp, boxed)) = semi_span {
if boxed {
if matches!(boxed, StatementAsExpression::NeedsBoxing) {
err.span_suggestion_verbose(
sp,
"consider removing this semicolon and boxing the expression",
Expand Down Expand Up @@ -727,7 +728,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
err.span_label(sp, "`if` and `else` have incompatible types");
}
if let Some((sp, boxed)) = semicolon {
if boxed {
if matches!(boxed, StatementAsExpression::NeedsBoxing) {
err.span_suggestion_verbose(
sp,
"consider removing this semicolon and boxing the expression",
Expand Down
17 changes: 15 additions & 2 deletions compiler/rustc_middle/src/traits/mod.rs
View file
Expand Up @@ -340,11 +340,24 @@ impl ObligationCauseCode<'_> {
#[cfg(target_arch = "x86_64")]
static_assert_size!(ObligationCauseCode<'_>, 32);

#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
pub enum StatementAsExpression {
CorrectType,
NeedsBoxing,
}

impl<'tcx> ty::Lift<'tcx> for StatementAsExpression {
type Lifted = StatementAsExpression;
fn lift_to_tcx(self, _tcx: TyCtxt<'tcx>) -> Option<StatementAsExpression> {
Some(self)
}
}

#[derive(Clone, Debug, PartialEq, Eq, Hash, Lift)]
pub struct MatchExpressionArmCause<'tcx> {
pub arm_span: Span,
pub scrut_span: Span,
pub semi_span: Option<(Span, bool)>,
pub semi_span: Option<(Span, StatementAsExpression)>,
pub source: hir::MatchSource,
pub prior_arms: Vec<Span>,
pub last_ty: Ty<'tcx>,
Expand All @@ -357,7 +370,7 @@ pub struct IfExpressionCause {
pub then: Span,
pub else_sp: Span,
pub outer: Option<Span>,
pub semicolon: Option<(Span, bool)>,
pub semicolon: Option<(Span, StatementAsExpression)>,
pub opt_suggest_box_span: Option<Span>,
}

Expand Down
39 changes: 26 additions & 13 deletions compiler/rustc_typeck/src/check/_match.rs
View file
Expand Up @@ -9,6 +9,7 @@ use rustc_trait_selection::opaque_types::InferCtxtExt as _;
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 @@ -188,18 +189,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
} else {
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;
}
}
let (arm_span, semi_span) =
self.get_appropriate_arm_semicolon_removal_span(&arms, i, prior_arm_ty, arm_ty);
let (span, code) = match i {
// 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.
Expand Down Expand Up @@ -249,6 +240,28 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
coercion.complete(self)
}

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(
Expand Down Expand Up @@ -521,7 +534,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self,
block: &'tcx hir::Block<'tcx>,
expected_ty: Option<Ty<'tcx>>,
) -> (Span, Option<(Span, bool)>) {
) -> (Span, Option<(Span, StatementAsExpression)>) {
if let Some(expr) = &block.expr {
(expr.span, None)
} else if let Some(stmt) = block.stmts.last() {
Expand Down
46 changes: 22 additions & 24 deletions compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs
View file
Expand Up @@ -33,7 +33,9 @@ use rustc_span::{self, BytePos, MultiSpan, Span};
use rustc_trait_selection::infer::InferCtxtExt as _;
use rustc_trait_selection::opaque_types::InferCtxtExt as _;
use rustc_trait_selection::traits::error_reporting::InferCtxtExt as _;
use rustc_trait_selection::traits::{self, ObligationCauseCode, TraitEngine, TraitEngineExt};
use rustc_trait_selection::traits::{
self, ObligationCauseCode, StatementAsExpression, TraitEngine, TraitEngineExt,
};

use std::collections::hash_map::Entry;
use std::slice;
Expand Down Expand Up @@ -1061,7 +1063,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self,
blk: &'tcx hir::Block<'tcx>,
expected_ty: Ty<'tcx>,
) -> Option<(Span, bool)> {
) -> 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()?;
Expand All @@ -1078,49 +1080,45 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
);
let last_hir_id = self.tcx.hir().local_def_id_to_hir_id(last_def_id.expect_local());
let exp_hir_id = self.tcx.hir().local_def_id_to_hir_id(exp_def_id.expect_local());
if let (
hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds: last_bounds, .. }),
hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds: exp_bounds, .. }),
) = (
match (
&self.tcx.hir().expect_item(last_hir_id).kind,
&self.tcx.hir().expect_item(exp_hir_id).kind,
) {
debug!("{:?} {:?}", last_bounds, exp_bounds);
last_bounds.iter().zip(exp_bounds.iter()).all(|(left, right)| {
(
hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds: last_bounds, .. }),
hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds: exp_bounds, .. }),
) if last_bounds.iter().zip(exp_bounds.iter()).all(|(left, right)| {
match (left, right) {
(
hir::GenericBound::Trait(tl, ml),
hir::GenericBound::Trait(tr, mr),
) => {
tl.trait_ref.trait_def_id() == tr.trait_ref.trait_def_id()
&& ml == 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);
langl == langr
true
}
_ => false,
}
})
} else {
false
}) =>
{
StatementAsExpression::NeedsBoxing
}
_ => StatementAsExpression::CorrectType,
}
}
_ => false,
_ => StatementAsExpression::CorrectType,
};
debug!(
"needs_box {:?} {:?} {:?}",
needs_box,
last_expr_ty.kind(),
self.can_sub(self.param_env, last_expr_ty, expected_ty)
);
if (matches!(last_expr_ty.kind(), ty::Error(_))
|| self.can_sub(self.param_env, last_expr_ty, expected_ty).is_err())
&& !needs_box
&& matches!(needs_box, StatementAsExpression::CorrectType)
{
return None;
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_typeck/src/check/fn_ctxt/checks.rs
View file
Expand Up @@ -20,7 +20,7 @@ use rustc_middle::ty::{self, Ty};
use rustc_session::Session;
use rustc_span::symbol::{sym, Ident};
use rustc_span::{self, MultiSpan, Span};
use rustc_trait_selection::traits::{self, ObligationCauseCode};
use rustc_trait_selection::traits::{self, ObligationCauseCode, StatementAsExpression};

use std::mem::replace;
use std::slice;
Expand Down Expand Up @@ -759,7 +759,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
err: &mut DiagnosticBuilder<'_>,
) {
if let Some((span_semi, boxed)) = self.could_remove_semicolon(blk, expected_ty) {
if boxed {
if let StatementAsExpression::NeedsBoxing = boxed {
err.span_suggestion_verbose(
span_semi,
"consider removing this semicolon and boxing the expression",
Expand Down

0 comments on commit 62ba365

Please sign in to comment.