Skip to content

Commit

Permalink
Fix bug in associated constant type annotations.
Browse files Browse the repository at this point in the history
This commit reverses the variance used when relating types from the type
annotation of an associated constant - this matches the behaviour of the
lexical borrow checker and fixes a bug whereby matching a `&'a str`
against a `&'static str` would produce an error.
  • Loading branch information
davidtwco committed Jan 3, 2019
1 parent ec19464 commit 4933793
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 4 deletions.
28 changes: 26 additions & 2 deletions src/librustc_mir/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
..
},
user_ty: pat_ascription_ty,
variance: _,
user_ty_span,
} => {
let place =
Expand All @@ -310,6 +311,20 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
source_info: ty_source_info,
kind: StatementKind::AscribeUserType(
place,
// We always use invariant as the variance here. This is because the
// variance field from the ascription refers to the variance to use
// when applying the type to the value being matched, but this
// ascription applies rather to the type of the binding. e.g., in this
// example:
//
// ```
// let x: T = <expr>
// ```
//
// We are creating an ascription that defines the type of `x` to be
// exactly `T` (i.e., with invariance). The variance field, in
// contrast, is intended to be used to relate `T` to the type of
// `<expr>`.
ty::Variance::Invariant,
user_ty,
),
Expand Down Expand Up @@ -541,12 +556,20 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
PatternKind::Deref { ref subpattern } => {
self.visit_bindings(subpattern, pattern_user_ty.deref(), f);
}
PatternKind::AscribeUserType { ref subpattern, ref user_ty, user_ty_span } => {
PatternKind::AscribeUserType {
ref subpattern,
ref user_ty,
user_ty_span,
variance: _,
} => {
// This corresponds to something like
//
// ```
// let A::<'a>(_): A<'static> = ...;
// ```
//
// Note that the variance doesn't apply here, as we are tracking the effect
// of `user_ty` on any bindings contained with subpattern.
let annotation = (user_ty_span, user_ty.base);
let projection = UserTypeProjection {
base: self.canonical_user_type_annotations.push(annotation),
Expand Down Expand Up @@ -628,6 +651,7 @@ struct Ascription<'tcx> {
span: Span,
source: Place<'tcx>,
user_ty: PatternTypeProjection<'tcx>,
variance: ty::Variance,
}

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -1321,7 +1345,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
source_info,
kind: StatementKind::AscribeUserType(
ascription.source.clone(),
ty::Variance::Covariant,
ascription.variance,
user_ty,
),
},
Expand Down
10 changes: 9 additions & 1 deletion src/librustc_mir/build/matches/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,19 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
-> Result<(), MatchPair<'pat, 'tcx>> {
let tcx = self.hir.tcx();
match *match_pair.pattern.kind {
PatternKind::AscribeUserType { ref subpattern, ref user_ty, user_ty_span } => {
PatternKind::AscribeUserType {
ref subpattern,
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.
candidate.ascriptions.push(Ascription {
span: user_ty_span,
user_ty: user_ty.clone(),
source: match_pair.place.clone(),
variance,
});

candidate.match_pairs.push(MatchPair::new(match_pair.place, subpattern));
Expand Down
4 changes: 3 additions & 1 deletion src/librustc_mir/hair/cx/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use hair::cx::Cx;
use hair::cx::to_ref::ToRef;
use rustc::middle::region;
use rustc::hir;
use rustc::ty;

use rustc_data_structures::indexed_vec::Idx;

Expand Down Expand Up @@ -86,7 +87,8 @@ fn mirror_stmts<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
kind: Box::new(PatternKind::AscribeUserType {
user_ty: PatternTypeProjection::from_user_type(user_ty),
user_ty_span: ty.span,
subpattern: pattern
subpattern: pattern,
variance: ty::Variance::Covariant,
})
};
}
Expand Down
25 changes: 25 additions & 0 deletions src/librustc_mir/hair/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,25 @@ pub enum PatternKind<'tcx> {
AscribeUserType {
user_ty: PatternTypeProjection<'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,
},

Expand Down Expand Up @@ -714,6 +733,7 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> {
},
user_ty: PatternTypeProjection::from_user_type(user_ty),
user_ty_span: span,
variance: ty::Variance::Covariant,
};
}

Expand Down Expand Up @@ -763,6 +783,9 @@ 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,
}
Expand Down Expand Up @@ -1057,11 +1080,13 @@ impl<'tcx> PatternFoldable<'tcx> for PatternKind<'tcx> {
PatternKind::Wild => PatternKind::Wild,
PatternKind::AscribeUserType {
ref subpattern,
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,
},
PatternKind::Binding {
Expand Down
21 changes: 21 additions & 0 deletions src/test/ui/nll/issue-57280-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#![feature(nll)]

// compile-pass

trait Foo<'a> {
const C: &'a u32;
}

impl<'a, T> Foo<'a> for T {
const C: &'a u32 = &22;
}

fn foo() {
let a = 22;
match &a {
<() as Foo<'static>>::C => { }
&_ => { }
}
}

fn main() {}
22 changes: 22 additions & 0 deletions src/test/ui/nll/issue-57280.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#![feature(nll)]

// compile-pass

trait Foo {
const BLAH: &'static str;
}

struct Placeholder;

impl Foo for Placeholder {
const BLAH: &'static str = "hi";
}

fn foo(x: &str) {
match x {
<Placeholder as Foo>::BLAH => { }
_ => { }
}
}

fn main() {}

0 comments on commit 4933793

Please sign in to comment.