Skip to content

Commit

Permalink
Auto merge of #57304 - davidtwco:issue-57280, r=nikomatsakis
Browse files Browse the repository at this point in the history
NLL: Fix bug in associated constant type annotations.

Fixes #57280.

This PR 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.

r? @nikomatsakis
  • Loading branch information
bors committed Jan 7, 2019
2 parents 1f7c44c + 4933793 commit 21ac19d
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 21ac19d

Please sign in to comment.