From ee82d09b6c8516b460432ebe5dd718c3353c3084 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 11 Feb 2019 12:18:40 +0100 Subject: [PATCH] Check user type annotations for range patterns. This commit builds on the fix from #58161 (which fixed miscompilation caused by the introduction of `AscribeUserType` patterns for associated constants) to start checking these patterns are well-formed for ranges (previous fix just ignored them so that miscompilation wouldn't occur). --- src/librustc_mir/build/matches/mod.rs | 18 ++- src/librustc_mir/build/matches/simplify.rs | 10 +- src/librustc_mir/hair/cx/block.rs | 10 +- src/librustc_mir/hair/pattern/_match.rs | 28 ++-- src/librustc_mir/hair/pattern/mod.rs | 145 ++++++++++++--------- src/test/ui/nll/issue-58299.rs | 30 +++++ src/test/ui/nll/issue-58299.stderr | 20 +++ 7 files changed, 176 insertions(+), 85 deletions(-) create mode 100644 src/test/ui/nll/issue-58299.rs create mode 100644 src/test/ui/nll/issue-58299.stderr diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index cf051ba2e0f..b8c1a1a8c45 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -7,7 +7,7 @@ use crate::build::scope::{CachedBlock, DropKind}; use crate::build::ForGuard::{self, OutsideGuard, RefWithinGuard, ValWithinGuard}; use crate::build::{BlockAnd, BlockAndExtension, Builder}; use crate::build::{GuardFrame, GuardFrameLocal, LocalsForNode}; -use crate::hair::*; +use crate::hair::{self, *}; use rustc::mir::*; use rustc::ty::{self, CanonicalUserTypeAnnotation, Ty}; use rustc::ty::layout::VariantIdx; @@ -283,9 +283,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { }, .. }, - user_ty: pat_ascription_ty, - variance: _, - user_ty_span, + ascription: hair::pattern::Ascription { + user_ty: pat_ascription_ty, + variance: _, + user_ty_span, + }, } => { let place = self.storage_live_binding(block, var, irrefutable_pat.span, OutsideGuard); @@ -560,9 +562,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } PatternKind::AscribeUserType { ref subpattern, - ref user_ty, - user_ty_span, - variance: _, + ascription: hair::pattern::Ascription { + ref user_ty, + user_ty_span, + variance: _, + }, } => { // This corresponds to something like // diff --git a/src/librustc_mir/build/matches/simplify.rs b/src/librustc_mir/build/matches/simplify.rs index 6be9ccb2703..b8e38e40b63 100644 --- a/src/librustc_mir/build/matches/simplify.rs +++ b/src/librustc_mir/build/matches/simplify.rs @@ -14,7 +14,7 @@ use crate::build::Builder; use crate::build::matches::{Ascription, Binding, MatchPair, Candidate}; -use crate::hair::*; +use crate::hair::{self, *}; use rustc::ty; use rustc::ty::layout::{Integer, IntegerExt, Size}; use syntax::attr::{SignedInt, UnsignedInt}; @@ -58,9 +58,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { match *match_pair.pattern.kind { PatternKind::AscribeUserType { ref subpattern, - variance, - ref user_ty, - user_ty_span + ascription: hair::pattern::Ascription { + variance, + ref user_ty, + user_ty_span, + }, } => { // Apply the type ascription to the value at `match_pair.place`, which is the // value being matched, taking the variance field into account. diff --git a/src/librustc_mir/hair/cx/block.rs b/src/librustc_mir/hair/cx/block.rs index c24cf956504..7c1dbb449c5 100644 --- a/src/librustc_mir/hair/cx/block.rs +++ b/src/librustc_mir/hair/cx/block.rs @@ -1,4 +1,4 @@ -use crate::hair::*; +use crate::hair::{self, *}; use crate::hair::cx::Cx; use crate::hair::cx::to_ref::ToRef; use rustc::middle::region; @@ -83,10 +83,12 @@ fn mirror_stmts<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>, ty: pattern.ty, span: pattern.span, kind: Box::new(PatternKind::AscribeUserType { - user_ty: PatternTypeProjection::from_user_type(user_ty), - user_ty_span: ty.span, + ascription: hair::pattern::Ascription { + user_ty: PatternTypeProjection::from_user_type(user_ty), + user_ty_span: ty.span, + variance: ty::Variance::Covariant, + }, subpattern: pattern, - variance: ty::Variance::Covariant, }) }; } diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 5779a032acc..3bdc10bcb06 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -871,18 +871,24 @@ impl<'tcx> IntRange<'tcx> { } fn from_pat(tcx: TyCtxt<'_, 'tcx, 'tcx>, - pat: &Pattern<'tcx>) + mut pat: &Pattern<'tcx>) -> Option> { - Self::from_ctor(tcx, &match pat.kind { - box PatternKind::Constant { value } => ConstantValue(value), - box PatternKind::Range(PatternRange { lo, hi, ty, end }) => ConstantRange( - lo.to_bits(tcx, ty::ParamEnv::empty().and(ty)).unwrap(), - hi.to_bits(tcx, ty::ParamEnv::empty().and(ty)).unwrap(), - ty, - end, - ), - _ => return None, - }) + let range = loop { + match pat.kind { + box PatternKind::Constant { value } => break ConstantValue(value), + box PatternKind::Range(PatternRange { lo, hi, ty, end }) => break ConstantRange( + lo.to_bits(tcx, ty::ParamEnv::empty().and(ty)).unwrap(), + hi.to_bits(tcx, ty::ParamEnv::empty().and(ty)).unwrap(), + ty, + end, + ), + box PatternKind::AscribeUserType { ref subpattern, .. } => { + pat = subpattern; + }, + _ => return None, + } + }; + Self::from_ctor(tcx, &range) } // The return value of `signed_bias` should be XORed with an endpoint to encode/decode it. diff --git a/src/librustc_mir/hair/pattern/mod.rs b/src/librustc_mir/hair/pattern/mod.rs index 84d8f32954c..84f1d979b48 100644 --- a/src/librustc_mir/hair/pattern/mod.rs +++ b/src/librustc_mir/hair/pattern/mod.rs @@ -58,7 +58,7 @@ pub struct Pattern<'tcx> { } -#[derive(Clone, Debug)] +#[derive(Copy, Clone, Debug, PartialEq)] pub struct PatternTypeProjection<'tcx> { pub user_ty: CanonicalUserType<'tcx>, } @@ -87,33 +87,38 @@ impl<'tcx> PatternTypeProjection<'tcx> { } } +#[derive(Copy, Clone, Debug, PartialEq)] +pub struct Ascription<'tcx> { + pub user_ty: PatternTypeProjection<'tcx>, + /// Variance to use when relating the type `user_ty` to the **type of the value being + /// matched**. Typically, this is `Variance::Covariant`, since the value being matched must + /// have a type that is some subtype of the ascribed type. + /// + /// Note that this variance does not apply for any bindings within subpatterns. The type + /// assigned to those bindings must be exactly equal to the `user_ty` given here. + /// + /// The only place where this field is not `Covariant` is when matching constants, where + /// we currently use `Contravariant` -- this is because the constant type just needs to + /// be "comparable" to the type of the input value. So, for example: + /// + /// ```text + /// match x { "foo" => .. } + /// ``` + /// + /// requires that `&'static str <: T_x`, where `T_x` is the type of `x`. Really, we should + /// probably be checking for a `PartialEq` impl instead, but this preserves the behavior + /// of the old type-check for now. See #57280 for details. + pub variance: ty::Variance, + pub user_ty_span: Span, +} + #[derive(Clone, Debug)] pub enum PatternKind<'tcx> { Wild, AscribeUserType { - user_ty: PatternTypeProjection<'tcx>, + ascription: Ascription<'tcx>, subpattern: Pattern<'tcx>, - /// Variance to use when relating the type `user_ty` to the **type of the value being - /// matched**. Typically, this is `Variance::Covariant`, since the value being matched must - /// have a type that is some subtype of the ascribed type. - /// - /// Note that this variance does not apply for any bindings within subpatterns. The type - /// assigned to those bindings must be exactly equal to the `user_ty` given here. - /// - /// The only place where this field is not `Covariant` is when matching constants, where - /// we currently use `Contravariant` -- this is because the constant type just needs to - /// be "comparable" to the type of the input value. So, for example: - /// - /// ```text - /// match x { "foo" => .. } - /// ``` - /// - /// requires that `&'static str <: T_x`, where `T_x` is the type of `x`. Really, we should - /// probably be checking for a `PartialEq` impl instead, but this preserves the behavior - /// of the old type-check for now. See #57280 for details. - variance: ty::Variance, - user_ty_span: Span, }, /// x, ref x, x @ P, etc @@ -167,18 +172,7 @@ pub enum PatternKind<'tcx> { }, } -impl<'tcx> PatternKind<'tcx> { - /// If this is a `PatternKind::AscribeUserType` then return the subpattern kind, otherwise - /// return this pattern kind. - fn with_user_type_ascription_subpattern(self) -> Self { - match self { - PatternKind::AscribeUserType { subpattern: Pattern { box kind, .. }, .. } => kind, - kind => kind, - } - } -} - -#[derive(Clone, Copy, Debug, PartialEq)] +#[derive(Copy, Clone, Debug, PartialEq)] pub struct PatternRange<'tcx> { pub lo: ty::Const<'tcx>, pub hi: ty::Const<'tcx>, @@ -405,6 +399,19 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> { ) } + fn lower_range_expr( + &mut self, + expr: &'tcx hir::Expr, + ) -> (PatternKind<'tcx>, Option>) { + match self.lower_lit(expr) { + PatternKind::AscribeUserType { + ascription: lo_ascription, + subpattern: Pattern { kind: box kind, .. }, + } => (kind, Some(lo_ascription)), + kind => (kind, None), + } + } + fn lower_pattern_unadjusted(&mut self, pat: &'tcx hir::Pat) -> Pattern<'tcx> { let mut ty = self.tables.node_id_to_type(pat.hir_id); @@ -414,14 +421,10 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> { PatKind::Lit(ref value) => self.lower_lit(value), PatKind::Range(ref lo_expr, ref hi_expr, end) => { - match ( - // Look for `PatternKind::Constant` patterns inside of any - // `PatternKind::AscribeUserType` patterns. Type ascriptions can be safely - // ignored for the purposes of lowering a range correctly - these are checked - // elsewhere for well-formedness. - self.lower_lit(lo_expr).with_user_type_ascription_subpattern(), - self.lower_lit(hi_expr).with_user_type_ascription_subpattern(), - ) { + let (lo, lo_ascription) = self.lower_range_expr(lo_expr); + let (hi, hi_ascription) = self.lower_range_expr(hi_expr); + + let mut kind = match (lo, hi) { (PatternKind::Constant { value: lo }, PatternKind::Constant { value: hi }) => { use std::cmp::Ordering; let cmp = compare_const_vals( @@ -470,17 +473,33 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> { PatternKind::Wild } } - } + }, ref pats => { self.tcx.sess.delay_span_bug( pat.span, - &format!("found bad range pattern `{:?}` outside of error recovery", - pats), + &format!( + "found bad range pattern `{:?}` outside of error recovery", + pats, + ), ); PatternKind::Wild + }, + }; + + // If we are handling a range with associated constants (e.g. + // `Foo::<'a>::A..=Foo::B`), we need to put the ascriptions for the associated + // constants somewhere. Have them on the range pattern. + for ascription in &[lo_ascription, hi_ascription] { + if let Some(ascription) = ascription { + kind = PatternKind::AscribeUserType { + ascription: *ascription, + subpattern: Pattern { span: pat.span, ty, kind: Box::new(kind), }, + }; } } + + kind } PatKind::Path(ref qpath) => { @@ -756,9 +775,11 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> { ty, kind: Box::new(kind), }, - user_ty: PatternTypeProjection::from_user_type(user_ty), - user_ty_span: span, - variance: ty::Variance::Covariant, + ascription: Ascription { + user_ty: PatternTypeProjection::from_user_type(user_ty), + user_ty_span: span, + variance: ty::Variance::Covariant, + }, }; } @@ -808,11 +829,13 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> { kind: Box::new( PatternKind::AscribeUserType { subpattern: pattern, - /// Note that use `Contravariant` here. See the - /// `variance` field documentation for details. - variance: ty::Variance::Contravariant, - user_ty, - user_ty_span: span, + ascription: Ascription { + /// Note that use `Contravariant` here. See the + /// `variance` field documentation for details. + variance: ty::Variance::Contravariant, + user_ty, + user_ty_span: span, + }, } ), ty: value.ty, @@ -1105,14 +1128,18 @@ impl<'tcx> PatternFoldable<'tcx> for PatternKind<'tcx> { PatternKind::Wild => PatternKind::Wild, PatternKind::AscribeUserType { ref subpattern, - variance, - ref user_ty, - user_ty_span, + ascription: Ascription { + variance, + ref user_ty, + user_ty_span, + }, } => PatternKind::AscribeUserType { subpattern: subpattern.fold_with(folder), - user_ty: user_ty.fold_with(folder), - variance, - user_ty_span, + ascription: Ascription { + user_ty: user_ty.fold_with(folder), + variance, + user_ty_span, + }, }, PatternKind::Binding { mutability, diff --git a/src/test/ui/nll/issue-58299.rs b/src/test/ui/nll/issue-58299.rs new file mode 100644 index 00000000000..9267cac5dd3 --- /dev/null +++ b/src/test/ui/nll/issue-58299.rs @@ -0,0 +1,30 @@ +#![allow(dead_code)] +#![feature(nll)] + +struct A<'a>(&'a ()); + +trait Y { + const X: i32; +} + +impl Y for A<'static> { + const X: i32 = 10; +} + +fn foo<'a>(x: i32) { + match x { + // This uses as Y>::X, but `A<'a>` does not implement `Y`. + A::<'a>::X..=A::<'static>::X => (), //~ ERROR lifetime may not live long enough + _ => (), + } +} + +fn bar<'a>(x: i32) { + match x { + // This uses as Y>::X, but `A<'a>` does not implement `Y`. + A::<'static>::X..=A::<'a>::X => (), //~ ERROR lifetime may not live long enough + _ => (), + } +} + +fn main() {} diff --git a/src/test/ui/nll/issue-58299.stderr b/src/test/ui/nll/issue-58299.stderr new file mode 100644 index 00000000000..b87d0de51a3 --- /dev/null +++ b/src/test/ui/nll/issue-58299.stderr @@ -0,0 +1,20 @@ +error: lifetime may not live long enough + --> $DIR/issue-58299.rs:17:9 + | +LL | fn foo<'a>(x: i32) { + | -- lifetime `'a` defined here +... +LL | A::<'a>::X..=A::<'static>::X => (), //~ ERROR lifetime may not live long enough + | ^^^^^^^^^^ requires that `'a` must outlive `'static` + +error: lifetime may not live long enough + --> $DIR/issue-58299.rs:25:27 + | +LL | fn bar<'a>(x: i32) { + | -- lifetime `'a` defined here +... +LL | A::<'static>::X..=A::<'a>::X => (), //~ ERROR lifetime may not live long enough + | ^^^^^^^^^^ requires that `'a` must outlive `'static` + +error: aborting due to 2 previous errors +