Review comments: use newtype instead of bool

Author: estebank

Diff for: 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 for: 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<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>,
@@ -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>,
 }
 

Diff for: 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<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(
@@ -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() {

Diff for: 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 for: 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",