From 62ba365195e37b0508dc35f73b55243cb1aef7f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 22 Oct 2020 10:27:40 -0700 Subject: [PATCH] Review comments: use newtype instead of `bool` --- .../src/infer/error_reporting/mod.rs | 5 +- compiler/rustc_middle/src/traits/mod.rs | 17 ++++++- compiler/rustc_typeck/src/check/_match.rs | 39 ++++++++++------ .../rustc_typeck/src/check/fn_ctxt/_impl.rs | 46 +++++++++---------- .../rustc_typeck/src/check/fn_ctxt/checks.rs | 4 +- 5 files changed, 68 insertions(+), 43 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index f6aa3c3d5ba..9e93cfc27d0 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -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}; @@ -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", @@ -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", diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index daa3010abb5..4deb7225dcb 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -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 { + 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, pub last_ty: Ty<'tcx>, @@ -357,7 +370,7 @@ pub struct IfExpressionCause { pub then: Span, pub else_sp: Span, pub outer: Option, - pub semicolon: Option<(Span, bool)>, + pub semicolon: Option<(Span, StatementAsExpression)>, pub opt_suggest_box_span: Option, } diff --git a/compiler/rustc_typeck/src/check/_match.rs b/compiler/rustc_typeck/src/check/_match.rs index 27c85317e82..e8eea65137f 100644 --- a/compiler/rustc_typeck/src/check/_match.rs +++ b/compiler/rustc_typeck/src/check/_match.rs @@ -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> { @@ -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. @@ -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>, + 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( @@ -521,7 +534,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &self, block: &'tcx hir::Block<'tcx>, expected_ty: Option>, - ) -> (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() { diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs index f26a168bcb2..f87e6b607d4 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs @@ -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; @@ -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()?; @@ -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; } diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs index a124ad16612..a820661d843 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs @@ -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; @@ -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",