bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Review comments: use newtype instead of bool

Browse source
This commit is contained in:
Esteban Küber 2020-10-22 10:27:40 -07:00
parent 671d7c4afb
commit 62ba365195
5 changed files with 68 additions and 43 deletions
Download patch file Download diff file Expand all files Collapse all files

5
compiler/rustc_infer/src/infer/error_reporting/mod.rs
View file

@ -54,6 +54,7 @@ use crate::infer::OriginalQueryValues;
use crate::traits::error_reporting::report_object_safety_error; use crate::traits::error_reporting::report_object_safety_error;
use crate::traits::{ use crate::traits::{
IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode, IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode,
StatementAsExpression,
}; };
use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::fx::{FxHashMap, FxHashSet};
@ -689,7 +690,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
let msg = "`match` arms have incompatible types"; let msg = "`match` arms have incompatible types";
err.span_label(outer_error_span, msg); err.span_label(outer_error_span, msg);
if let Some((sp, boxed)) = semi_span { if let Some((sp, boxed)) = semi_span {
if boxed { if matches!(boxed, StatementAsExpression::NeedsBoxing) {
err.span_suggestion_verbose( err.span_suggestion_verbose(
sp, sp,
"consider removing this semicolon and boxing the expression", "consider removing this semicolon and boxing the expression",
@ -727,7 +728,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
err.span_label(sp, "`if` and `else` have incompatible types"); err.span_label(sp, "`if` and `else` have incompatible types");
} }
if let Some((sp, boxed)) = semicolon { if let Some((sp, boxed)) = semicolon {
if boxed { if matches!(boxed, StatementAsExpression::NeedsBoxing) {
err.span_suggestion_verbose( err.span_suggestion_verbose(
sp, sp,
"consider removing this semicolon and boxing the expression", "consider removing this semicolon and boxing the expression",

17
compiler/rustc_middle/src/traits/mod.rs
View file

@ -340,11 +340,24 @@ impl ObligationCauseCode<'_> {
#[cfg(target_arch = "x86_64")] #[cfg(target_arch = "x86_64")]
static_assert_size!(ObligationCauseCode<'_>, 32); 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)] #[derive(Clone, Debug, PartialEq, Eq, Hash, Lift)]
pub struct MatchExpressionArmCause<'tcx> { pub struct MatchExpressionArmCause<'tcx> {
pub arm_span: Span, pub arm_span: Span,
pub scrut_span: Span, pub scrut_span: Span,
pub semi_span: Option<(Span, bool)>, pub semi_span: Option<(Span, StatementAsExpression)>,
pub source: hir::MatchSource, pub source: hir::MatchSource,
pub prior_arms: Vec<Span>, pub prior_arms: Vec<Span>,
pub last_ty: Ty<'tcx>, pub last_ty: Ty<'tcx>,
@ -357,7 +370,7 @@ pub struct IfExpressionCause {
pub then: Span, pub then: Span,
pub else_sp: Span, pub else_sp: Span,
pub outer: Option<Span>, pub outer: Option<Span>,
pub semicolon: Option<(Span, bool)>, pub semicolon: Option<(Span, StatementAsExpression)>,
pub opt_suggest_box_span: Option<Span>, pub opt_suggest_box_span: Option<Span>,
} }

39
compiler/rustc_typeck/src/check/_match.rs
View file

@ -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::query::evaluate_obligation::InferCtxtExt;
use rustc_trait_selection::traits::{ use rustc_trait_selection::traits::{
IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode, IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode,
StatementAsExpression,
}; };
impl<'a, 'tcx> FnCtxt<'a, 'tcx> { impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
@ -188,18 +189,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
} }
} }
} else { } else {
let (arm_span, mut semi_span) = if let hir::ExprKind::Block(blk, _) = &arm.body.kind let (arm_span, semi_span) =
{ self.get_appropriate_arm_semicolon_removal_span(&arms, i, prior_arm_ty, arm_ty);
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 (span, code) = match i { let (span, code) = match i {
// The reason for the first arm to fail is not that the match arms diverge, // 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. // but rather that there's a prior obligation that doesn't hold.
@ -249,6 +240,28 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
coercion.complete(self) 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, /// When the previously checked expression (the scrutinee) diverges,
/// warn the user about the match arms being unreachable. /// warn the user about the match arms being unreachable.
fn warn_arms_when_scrutinee_diverges( fn warn_arms_when_scrutinee_diverges(
@ -521,7 +534,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self, &self,
block: &'tcx hir::Block<'tcx>, block: &'tcx hir::Block<'tcx>,
expected_ty: Option<Ty<'tcx>>, expected_ty: Option<Ty<'tcx>>,
) -> (Span, Option<(Span, bool)>) { ) -> (Span, Option<(Span, StatementAsExpression)>) {
if let Some(expr) = &block.expr { if let Some(expr) = &block.expr {
(expr.span, None) (expr.span, None)
} else if let Some(stmt) = block.stmts.last() { } else if let Some(stmt) = block.stmts.last() {

46
compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs
View file

@ -33,7 +33,9 @@ use rustc_span::{self, BytePos, MultiSpan, Span};
use rustc_trait_selection::infer::InferCtxtExt as _; use rustc_trait_selection::infer::InferCtxtExt as _;
use rustc_trait_selection::opaque_types::InferCtxtExt as _; use rustc_trait_selection::opaque_types::InferCtxtExt as _;
use rustc_trait_selection::traits::error_reporting::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::collections::hash_map::Entry;
use std::slice; use std::slice;
@ -1061,7 +1063,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self, &self,
blk: &'tcx hir::Block<'tcx>, blk: &'tcx hir::Block<'tcx>,
expected_ty: Ty<'tcx>, expected_ty: Ty<'tcx>,
) -> Option<(Span, bool)> { ) -> Option<(Span, StatementAsExpression)> {
// Be helpful when the user wrote `{... expr;}` and // Be helpful when the user wrote `{... expr;}` and
// taking the `;` off is enough to fix the error. // taking the `;` off is enough to fix the error.
let last_stmt = blk.stmts.last()?; let last_stmt = blk.stmts.last()?;
@ -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 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()); let exp_hir_id = self.tcx.hir().local_def_id_to_hir_id(exp_def_id.expect_local());
if let ( match (
hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds: last_bounds, .. }),
hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds: exp_bounds, .. }),
) = (
&self.tcx.hir().expect_item(last_hir_id).kind, &self.tcx.hir().expect_item(last_hir_id).kind,
&self.tcx.hir().expect_item(exp_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) { match (left, right) {
( (
hir::GenericBound::Trait(tl, ml), hir::GenericBound::Trait(tl, ml),
hir::GenericBound::Trait(tr, mr), hir::GenericBound::Trait(tr, mr),
) => { ) if tl.trait_ref.trait_def_id() == tr.trait_ref.trait_def_id()
tl.trait_ref.trait_def_id() == tr.trait_ref.trait_def_id() && ml == mr =>
&& ml == mr {
true
} }
( (
hir::GenericBound::LangItemTrait(langl, _, _, argsl), hir::GenericBound::LangItemTrait(langl, _, _, argsl),
hir::GenericBound::LangItemTrait(langr, _, _, argsr), hir::GenericBound::LangItemTrait(langr, _, _, argsr),
) => { ) if langl == langr => {
// FIXME: consider the bounds! // FIXME: consider the bounds!
debug!("{:?} {:?}", argsl, argsr); debug!("{:?} {:?}", argsl, argsr);
langl == langr true
} }
_ => false, _ => 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(_)) if (matches!(last_expr_ty.kind(), ty::Error(_))
|| self.can_sub(self.param_env, last_expr_ty, expected_ty).is_err()) || self.can_sub(self.param_env, last_expr_ty, expected_ty).is_err())
&& !needs_box && matches!(needs_box, StatementAsExpression::CorrectType)
{ {
return None; return None;
} }

4
compiler/rustc_typeck/src/check/fn_ctxt/checks.rs
View file

@ -20,7 +20,7 @@ use rustc_middle::ty::{self, Ty};
use rustc_session::Session; use rustc_session::Session;
use rustc_span::symbol::{sym, Ident}; use rustc_span::symbol::{sym, Ident};
use rustc_span::{self, MultiSpan, Span}; 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::mem::replace;
use std::slice; use std::slice;
@ -759,7 +759,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
err: &mut DiagnosticBuilder<'_>, err: &mut DiagnosticBuilder<'_>,
) { ) {
if let Some((span_semi, boxed)) = self.could_remove_semicolon(blk, expected_ty) { if let Some((span_semi, boxed)) = self.could_remove_semicolon(blk, expected_ty) {
if boxed { if let StatementAsExpression::NeedsBoxing = boxed {
err.span_suggestion_verbose( err.span_suggestion_verbose(
span_semi, span_semi,
"consider removing this semicolon and boxing the expression", "consider removing this semicolon and boxing the expression",