Skip to content

Commit

Permalink
Rollup merge of rust-lang#67730 - Centril:typeck-pat-cleanup, r=estebank
Browse files Browse the repository at this point in the history
Cleanup pattern type checking, fix diagnostics bugs (+ improvements)

r? @estebank
  • Loading branch information
Centril authored Dec 31, 2019
2 parents bc5963d + 63dc0e4 commit 50fb848
Show file tree
Hide file tree
Showing 50 changed files with 418 additions and 244 deletions.
18 changes: 11 additions & 7 deletions src/librustc/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -581,10 +581,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
exp_found: Option<ty::error::ExpectedFound<Ty<'tcx>>>,
) {
match cause.code {
ObligationCauseCode::MatchExpressionArmPattern { span, ty } => {
ObligationCauseCode::Pattern { origin_expr: true, span: Some(span), root_ty } => {
let ty = self.resolve_vars_if_possible(&root_ty);
if ty.is_suggestable() {
// don't show type `_`
err.span_label(span, format!("this match expression has type `{}`", ty));
err.span_label(span, format!("this expression has type `{}`", ty));
}
if let Some(ty::error::ExpectedFound { found, .. }) = exp_found {
if ty.is_box() && ty.boxed_ty() == found {
Expand All @@ -599,11 +600,14 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}
}
}
ObligationCauseCode::Pattern { origin_expr: false, span: Some(span), .. } => {
err.span_label(span, "expected due to this");
}
ObligationCauseCode::MatchExpressionArm(box MatchExpressionArmCause {
source,
ref prior_arms,
last_ty,
discrim_hir_id,
scrut_hir_id,
..
}) => match source {
hir::MatchSource::IfLetDesugar { .. } => {
Expand All @@ -612,16 +616,16 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}
hir::MatchSource::TryDesugar => {
if let Some(ty::error::ExpectedFound { expected, .. }) = exp_found {
let discrim_expr = self.tcx.hir().expect_expr(discrim_hir_id);
let discrim_ty = if let hir::ExprKind::Call(_, args) = &discrim_expr.kind {
let scrut_expr = self.tcx.hir().expect_expr(scrut_hir_id);
let scrut_ty = if let hir::ExprKind::Call(_, args) = &scrut_expr.kind {
let arg_expr = args.first().expect("try desugaring call w/out arg");
self.in_progress_tables
.and_then(|tables| tables.borrow().expr_ty_opt(arg_expr))
} else {
bug!("try desugaring w/out call expr as discriminant");
bug!("try desugaring w/out call expr as scrutinee");
};

match discrim_ty {
match scrut_ty {
Some(ty) if expected == ty => {
let source_map = self.tcx.sess.source_map();
err.span_suggestion(
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2580,7 +2580,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
match *cause_code {
ObligationCauseCode::ExprAssignable
| ObligationCauseCode::MatchExpressionArm { .. }
| ObligationCauseCode::MatchExpressionArmPattern { .. }
| ObligationCauseCode::Pattern { .. }
| ObligationCauseCode::IfExpression { .. }
| ObligationCauseCode::IfExpressionWithNoElse
| ObligationCauseCode::MainFunctionType
Expand Down
14 changes: 9 additions & 5 deletions src/librustc/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,10 +249,14 @@ pub enum ObligationCauseCode<'tcx> {
/// Computing common supertype in the arms of a match expression
MatchExpressionArm(Box<MatchExpressionArmCause<'tcx>>),

/// Computing common supertype in the pattern guard for the arms of a match expression
MatchExpressionArmPattern {
span: Span,
ty: Ty<'tcx>,
/// Type error arising from type checking a pattern against an expected type.
Pattern {
/// The span of the scrutinee or type expression which caused the `root_ty` type.
span: Option<Span>,
/// The root expected type induced by a scrutinee or type expression.
root_ty: Ty<'tcx>,
/// Whether the `Span` came from an expression or a type expression.
origin_expr: bool,
},

/// Constants in patterns must have `Structural` type.
Expand Down Expand Up @@ -311,7 +315,7 @@ pub struct MatchExpressionArmCause<'tcx> {
pub source: hir::MatchSource,
pub prior_arms: Vec<Span>,
pub last_ty: Ty<'tcx>,
pub discrim_hir_id: hir::HirId,
pub scrut_hir_id: hir::HirId,
}

#[derive(Clone, Debug, PartialEq, Eq, Hash)]
Expand Down
8 changes: 4 additions & 4 deletions src/librustc/traits/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -511,18 +511,18 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> {
source,
ref prior_arms,
last_ty,
discrim_hir_id,
scrut_hir_id,
}) => tcx.lift(&last_ty).map(|last_ty| {
super::MatchExpressionArm(box super::MatchExpressionArmCause {
arm_span,
source,
prior_arms: prior_arms.clone(),
last_ty,
discrim_hir_id,
scrut_hir_id,
})
}),
super::MatchExpressionArmPattern { span, ty } => {
tcx.lift(&ty).map(|ty| super::MatchExpressionArmPattern { span, ty })
super::Pattern { span, root_ty, origin_expr } => {
tcx.lift(&root_ty).map(|root_ty| super::Pattern { span, root_ty, origin_expr })
}
super::IfExpression(box super::IfExpressionCause { then, outer, semicolon }) => {
Some(super::IfExpression(box super::IfExpressionCause { then, outer, semicolon }))
Expand Down
38 changes: 19 additions & 19 deletions src/librustc_typeck/check/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
pub fn check_match(
&self,
expr: &'tcx hir::Expr<'tcx>,
discrim: &'tcx hir::Expr<'tcx>,
scrut: &'tcx hir::Expr<'tcx>,
arms: &'tcx [hir::Arm<'tcx>],
expected: Expectation<'tcx>,
match_src: hir::MatchSource,
Expand All @@ -27,7 +27,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
};

// Type check the descriminant and get its type.
let discrim_ty = if force_scrutinee_bool {
let scrut_ty = if force_scrutinee_bool {
// Here we want to ensure:
//
// 1. That default match bindings are *not* accepted in the condition of an
Expand All @@ -36,9 +36,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// 2. By expecting `bool` for `expr` we get nice diagnostics for e.g. `if x = y { .. }`.
//
// FIXME(60707): Consider removing hack with principled solution.
self.check_expr_has_type_or_error(discrim, self.tcx.types.bool, |_| {})
self.check_expr_has_type_or_error(scrut, self.tcx.types.bool, |_| {})
} else {
self.demand_discriminant_type(arms, discrim)
self.demand_scrutinee_type(arms, scrut)
};

// If there are no arms, that is a diverging match; a special case.
Expand All @@ -51,7 +51,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

// Otherwise, we have to union together the types that the
// arms produce and so forth.
let discrim_diverges = self.diverges.get();
let scrut_diverges = self.diverges.get();
self.diverges.set(Diverges::Maybe);

// rust-lang/rust#55810: Typecheck patterns first (via eager
Expand All @@ -61,7 +61,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.map(|arm| {
let mut all_pats_diverge = Diverges::WarnedAlways;
self.diverges.set(Diverges::Maybe);
self.check_pat_top(&arm.pat, discrim_ty, Some(discrim.span));
self.check_pat_top(&arm.pat, scrut_ty, Some(scrut.span), true);
all_pats_diverge &= self.diverges.get();

// As discussed with @eddyb, this is for disabling unreachable_code
Expand Down Expand Up @@ -157,7 +157,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
source: match_src,
prior_arms: other_arms.clone(),
last_ty: prior_arm_ty.unwrap(),
discrim_hir_id: discrim.hir_id,
scrut_hir_id: scrut.hir_id,
}),
),
};
Expand Down Expand Up @@ -186,8 +186,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
};
}

// We won't diverge unless the discriminant or all arms diverge.
self.diverges.set(discrim_diverges | all_arms_diverge);
// We won't diverge unless the scrutinee or all arms diverge.
self.diverges.set(scrut_diverges | all_arms_diverge);

coercion.complete(self)
}
Expand Down Expand Up @@ -388,14 +388,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
)
}

fn demand_discriminant_type(
fn demand_scrutinee_type(
&self,
arms: &'tcx [hir::Arm<'tcx>],
discrim: &'tcx hir::Expr<'tcx>,
scrut: &'tcx hir::Expr<'tcx>,
) -> Ty<'tcx> {
// Not entirely obvious: if matches may create ref bindings, we want to
// use the *precise* type of the discriminant, *not* some supertype, as
// the "discriminant type" (issue #23116).
// use the *precise* type of the scrutinee, *not* some supertype, as
// the "scrutinee type" (issue #23116).
//
// arielb1 [writes here in this comment thread][c] that there
// is certainly *some* potential danger, e.g., for an example
Expand Down Expand Up @@ -454,17 +454,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
});

if let Some(m) = contains_ref_bindings {
self.check_expr_with_needs(discrim, Needs::maybe_mut_place(m))
self.check_expr_with_needs(scrut, Needs::maybe_mut_place(m))
} else {
// ...but otherwise we want to use any supertype of the
// discriminant. This is sort of a workaround, see note (*) in
// scrutinee. This is sort of a workaround, see note (*) in
// `check_pat` for some details.
let discrim_ty = self.next_ty_var(TypeVariableOrigin {
let scrut_ty = self.next_ty_var(TypeVariableOrigin {
kind: TypeVariableOriginKind::TypeInference,
span: discrim.span,
span: scrut.span,
});
self.check_expr_has_type_or_error(discrim, discrim_ty, |_| {});
discrim_ty
self.check_expr_has_type_or_error(scrut, scrut_ty, |_| {});
scrut_ty
}
}
}
31 changes: 1 addition & 30 deletions src/librustc_typeck/check/demand.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::check::FnCtxt;
use rustc::infer::InferOk;
use rustc::traits::{self, ObligationCause, ObligationCauseCode};
use rustc::traits::{self, ObligationCause};

use errors::{Applicability, DiagnosticBuilder};
use rustc::hir::{self, is_range_literal, print, Node};
Expand Down Expand Up @@ -79,35 +79,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

pub fn demand_eqtype_pat_diag(
&self,
cause_span: Span,
expected: Ty<'tcx>,
actual: Ty<'tcx>,
match_expr_span: Option<Span>,
) -> Option<DiagnosticBuilder<'tcx>> {
let cause = if let Some(span) = match_expr_span {
self.cause(
cause_span,
ObligationCauseCode::MatchExpressionArmPattern { span, ty: expected },
)
} else {
self.misc(cause_span)
};
self.demand_eqtype_with_origin(&cause, expected, actual)
}

pub fn demand_eqtype_pat(
&self,
cause_span: Span,
expected: Ty<'tcx>,
actual: Ty<'tcx>,
match_expr_span: Option<Span>,
) {
self.demand_eqtype_pat_diag(cause_span, expected, actual, match_expr_span)
.map(|mut err| err.emit());
}

pub fn demand_coerce(
&self,
expr: &hir::Expr<'_>,
Expand Down
Loading

0 comments on commit 50fb848

Please sign in to comment.