Skip to content

Commit

Permalink
Check user type annotations for range patterns.
Browse files Browse the repository at this point in the history
This commit builds on the fix from rust-lang#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).
  • Loading branch information
davidtwco committed Feb 12, 2019
1 parent 2e08bb1 commit ee82d09
Show file tree
Hide file tree
Showing 7 changed files with 176 additions and 85 deletions.
18 changes: 11 additions & 7 deletions src/librustc_mir/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
//
Expand Down
10 changes: 6 additions & 4 deletions src/librustc_mir/build/matches/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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.
Expand Down
10 changes: 6 additions & 4 deletions src/librustc_mir/hair/cx/block.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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,
})
};
}
Expand Down
28 changes: 17 additions & 11 deletions src/librustc_mir/hair/pattern/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -871,18 +871,24 @@ impl<'tcx> IntRange<'tcx> {
}

fn from_pat(tcx: TyCtxt<'_, 'tcx, 'tcx>,
pat: &Pattern<'tcx>)
mut pat: &Pattern<'tcx>)
-> Option<IntRange<'tcx>> {
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.
Expand Down
145 changes: 86 additions & 59 deletions src/librustc_mir/hair/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -405,6 +399,19 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> {
)
}

fn lower_range_expr(
&mut self,
expr: &'tcx hir::Expr,
) -> (PatternKind<'tcx>, Option<Ascription<'tcx>>) {
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);

Expand All @@ -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(
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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,
},
};
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
30 changes: 30 additions & 0 deletions src/test/ui/nll/issue-58299.rs
Original file line number Diff line number Diff line change
@@ -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 <A<'a> 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 <A<'a> 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() {}
20 changes: 20 additions & 0 deletions src/test/ui/nll/issue-58299.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit ee82d09

Please sign in to comment.