Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup pattern type checking, fix diagnostics bugs (+ improvements) #67730

Merged
merged 10 commits into from
Dec 31, 2019
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