Skip to content

[NLL] Check user types are well-formed #54942

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

Merged
merged 2 commits into from
Oct 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ pub struct LocalDecl<'tcx> {
/// e.g. via `let x: T`, then we carry that type here. The MIR
/// borrow checker needs this information since it can affect
/// region inference.
pub user_ty: Option<CanonicalTy<'tcx>>,
pub user_ty: Option<(CanonicalTy<'tcx>, Span)>,

/// Name of the local, used in debuginfo and pretty-printing.
///
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,7 @@ macro_rules! make_mir_visitor {
local,
source_info: *source_info,
});
if let Some(user_ty) = user_ty {
if let Some((user_ty, _)) = user_ty {
self.visit_user_ty(user_ty);
}
self.visit_source_info(source_info);
Expand Down
14 changes: 10 additions & 4 deletions src/librustc_mir/borrow_check/nll/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,12 +310,12 @@ impl<'a, 'b, 'gcx, 'tcx> Visitor<'tcx> for TypeVerifier<'a, 'b, 'gcx, 'tcx> {
self.super_local_decl(local, local_decl);
self.sanitize_type(local_decl, local_decl.ty);

if let Some(user_ty) = local_decl.user_ty {
if let Some((user_ty, span)) = local_decl.user_ty {
if let Err(terr) = self.cx.relate_type_and_user_type(
local_decl.ty,
ty::Variance::Invariant,
user_ty,
Locations::All(local_decl.source_info.span),
Locations::All(span),
ConstraintCategory::TypeAnnotation,
) {
span_mirbug!(
Expand Down Expand Up @@ -970,15 +970,21 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
locations: Locations,
category: ConstraintCategory,
) -> Fallible<()> {
relate_tys::relate_type_and_user_type(
let ty = relate_tys::relate_type_and_user_type(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is okay to have a variable name the same as the module name ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

self.infcx,
a,
v,
b,
locations,
category,
self.borrowck_context.as_mut().map(|x| &mut **x),
)
)?;
self.prove_predicate(
ty::Predicate::WellFormed(ty),
locations,
category,
);
Ok(())
}

fn eq_opaque_type_and_type(
Expand Down
21 changes: 16 additions & 5 deletions src/librustc_mir/borrow_check/nll/type_check/relate_tys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

use borrow_check::nll::constraints::OutlivesConstraint;
use borrow_check::nll::type_check::{BorrowCheckContext, Locations};
use rustc::infer::canonical::{Canonical, CanonicalVarInfos};
use rustc::infer::canonical::{Canonical, CanonicalVarInfos, CanonicalVarValues};
use rustc::infer::{InferCtxt, NLLRegionVariableOrigin};
use rustc::mir::ConstraintCategory;
use rustc::traits::query::Fallible;
Expand Down Expand Up @@ -70,7 +70,7 @@ pub(super) fn relate_type_and_user_type<'tcx>(
locations: Locations,
category: ConstraintCategory,
borrowck_context: Option<&mut BorrowCheckContext<'_, 'tcx>>,
) -> Fallible<()> {
) -> Fallible<Ty<'tcx>> {
debug!(
"sub_type_and_user_type(a={:?}, b={:?}, locations={:?})",
a, b, locations
Expand All @@ -85,13 +85,24 @@ pub(super) fn relate_type_and_user_type<'tcx>(
// variance to get the right relationship.
let v1 = ty::Contravariant.xform(v);

TypeRelating::new(
let mut type_relating = TypeRelating::new(
infcx.tcx,
NllTypeRelatingDelegate::new(infcx, borrowck_context, locations, category),
v1,
b_variables,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

b_variables
Can you explain to me how this magic happens ? b.variables is written as b_variables ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

b is destructured just above this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind pointing me to the line please ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line 79

).relate(&b_value, &a)?;
Ok(())
);
type_relating.relate(&b_value, &a)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type_relating.relate(&b_value, &a)?;

Can you explain to me what this line does ?

Copy link
Contributor Author

@matthewjasper matthewjasper Oct 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It checks that a and b_value are the same type apart from lifetimes, if they're not it returns an error to the caller. Otherwise, depending on v1, it adds any outlives constraints that are required for a being a subtype of b, or b being a subtype of a or b being the same type as a to the borrowck_context (if it's Some, which it will be during the NLL type check). Finally it works out what any type variables in b should be based on a.

So if b is &mut '_1 &'_2 _, a is &mut '_3 &'_4 i32 and v1is Covariant, the constraints added are'_1: '_3, '_2: '_4and'_4: '_2(the last constraint comes from&mut Tbeing invariant inT), and the type variable is inferred to be i32`.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matthewjasper Thanks a lot for the explanation. Really appreciate it.


Ok(b.substitute(
infcx.tcx,
&CanonicalVarValues {
var_values: type_relating
.canonical_var_values
.into_iter()
.map(|x| x.expect("unsubstituted canonical variable"))
.collect(),
},
))
}

struct TypeRelating<'me, 'gcx: 'tcx, 'tcx: 'me, D>
Expand Down
37 changes: 19 additions & 18 deletions src/librustc_mir/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,30 +292,32 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
..
},
user_ty: ascription_user_ty,
user_ty_span,
} => {
let place =
self.storage_live_binding(block, var, irrefutable_pat.span, OutsideGuard);
unpack!(block = self.into(&place, block, initializer));

let source_info = self.source_info(irrefutable_pat.span);
// Inject a fake read, see comments on `FakeReadCause::ForLet`.
let pattern_source_info = self.source_info(irrefutable_pat.span);
self.cfg.push(
block,
Statement {
source_info,
kind: StatementKind::AscribeUserType(
place.clone(),
ty::Variance::Invariant,
ascription_user_ty,
),
source_info: pattern_source_info,
kind: StatementKind::FakeRead(FakeReadCause::ForLet, place.clone()),
},
);

// Inject a fake read, see comments on `FakeReadCause::ForLet`.
let ty_source_info = self.source_info(user_ty_span);
self.cfg.push(
block,
Statement {
source_info,
kind: StatementKind::FakeRead(FakeReadCause::ForLet, place.clone()),
source_info: ty_source_info,
kind: StatementKind::AscribeUserType(
place.clone(),
ty::Variance::Invariant,
ascription_user_ty,
),
},
);

Expand Down Expand Up @@ -489,7 +491,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
pub fn visit_bindings(
&mut self,
pattern: &Pattern<'tcx>,
mut pattern_user_ty: Option<CanonicalTy<'tcx>>,
mut pattern_user_ty: Option<(CanonicalTy<'tcx>, Span)>,
f: &mut impl FnMut(
&mut Self,
Mutability,
Expand All @@ -498,7 +500,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
NodeId,
Span,
Ty<'tcx>,
Option<CanonicalTy<'tcx>>,
Option<(CanonicalTy<'tcx>, Span)>,
),
) {
match *pattern.kind {
Expand Down Expand Up @@ -549,16 +551,15 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
// FIXME(#47184): extract or handle `pattern_user_ty` somehow
self.visit_bindings(subpattern, None, f);
}
PatternKind::AscribeUserType { ref subpattern, user_ty } => {
PatternKind::AscribeUserType { ref subpattern, user_ty, user_ty_span } => {
// This corresponds to something like
//
// ```
// let (p1: T1): T2 = ...;
// let A::<'a>(_): A<'static> = ...;
// ```
//
// Not presently possible, though maybe someday.
assert!(pattern_user_ty.is_none());
self.visit_bindings(subpattern, Some(user_ty), f)
// FIXME(#47184): handle `pattern_user_ty` somehow
self.visit_bindings(subpattern, Some((user_ty, user_ty_span)), f)
}
PatternKind::Leaf { ref subpatterns }
| PatternKind::Variant {
Expand Down Expand Up @@ -1469,7 +1470,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
num_patterns: usize,
var_id: NodeId,
var_ty: Ty<'tcx>,
user_var_ty: Option<CanonicalTy<'tcx>>,
user_var_ty: Option<(CanonicalTy<'tcx>, Span)>,
has_guard: ArmHasGuard,
opt_match_place: Option<(Option<Place<'tcx>>, Span)>,
pat_span: Span,
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/build/matches/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
candidate: &mut Candidate<'pat, 'tcx>)
-> Result<(), MatchPair<'pat, 'tcx>> {
match *match_pair.pattern.kind {
PatternKind::AscribeUserType { ref subpattern, user_ty } => {
PatternKind::AscribeUserType { ref subpattern, user_ty, user_ty_span } => {
candidate.ascriptions.push(Ascription {
span: match_pair.pattern.span,
span: user_ty_span,
user_ty,
source: match_pair.place.clone(),
});
Expand Down
1 change: 1 addition & 0 deletions src/librustc_mir/hair/cx/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ fn mirror_stmts<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
span: pattern.span,
kind: Box::new(PatternKind::AscribeUserType {
user_ty: *user_ty,
user_ty_span: ty.span,
subpattern: pattern
})
};
Expand Down
4 changes: 4 additions & 0 deletions src/librustc_mir/hair/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ pub enum PatternKind<'tcx> {
AscribeUserType {
user_ty: CanonicalTy<'tcx>,
subpattern: Pattern<'tcx>,
user_ty_span: Span,
},

/// x, ref x, x @ P, etc
Expand Down Expand Up @@ -692,6 +693,7 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> {
kind = PatternKind::AscribeUserType {
subpattern,
user_ty,
user_ty_span: span,
};
}

Expand Down Expand Up @@ -1015,9 +1017,11 @@ impl<'tcx> PatternFoldable<'tcx> for PatternKind<'tcx> {
PatternKind::AscribeUserType {
ref subpattern,
user_ty,
user_ty_span,
} => PatternKind::AscribeUserType {
subpattern: subpattern.fold_with(folder),
user_ty: user_ty.fold_with(folder),
user_ty_span,
},
PatternKind::Binding {
mutability,
Expand Down
2 changes: 1 addition & 1 deletion src/test/mir-opt/basic_assignment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ fn main() {
// StorageDead(_3);
// StorageLive(_4);
// _4 = std::option::Option<std::boxed::Box<u32>>::None;
// AscribeUserType(_4, o, Canonical { variables: [], value: std::option::Option<std::boxed::Box<u32>> });
// FakeRead(ForLet, _4);
// AscribeUserType(_4, o, Canonical { variables: [], value: std::option::Option<std::boxed::Box<u32>> });
// StorageLive(_5);
// StorageLive(_6);
// _6 = move _4;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
error: unsatisfied lifetime constraints
--> $DIR/associated-types-subtyping-1.rs:36:13
|
LL | fn method2<'a,'b,T>(x: &'a T, y: &'b T)
| -- -- lifetime `'b` defined here
| |
| lifetime `'a` defined here
...
LL | let _c: <T as Trait<'b>>::Type = a; //~ ERROR E0623
| ^^^^^^^^^^^^^^^^^^^^^^ type annotation requires that `'b` must outlive `'a`

error: unsatisfied lifetime constraints
--> $DIR/associated-types-subtyping-1.rs:44:12
|
LL | fn method3<'a,'b,T>(x: &'a T, y: &'b T)
| -- -- lifetime `'b` defined here
| |
| lifetime `'a` defined here
...
LL | let b: <T as Trait<'b>>::Type = make_any();
| ^^^^^^^^^^^^^^^^^^^^^^ type annotation requires that `'b` must outlive `'a`

error: aborting due to 2 previous errors

20 changes: 10 additions & 10 deletions src/test/ui/associated-types/associated-types-subtyping-1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// ignore-compare-mode-nll

#![allow(unused_variables)]

fn make_any<T>() -> T { loop {} }

trait Trait<'a> {
type Type;

Expand All @@ -22,35 +22,35 @@ fn method1<'a,'b,T>(x: &'a T, y: &'b T)
where T : for<'z> Trait<'z>, 'a : 'b
{
// Note that &'static T <: &'a T.
let a: <T as Trait<'a>>::Type = loop { };
let b: <T as Trait<'b>>::Type = loop { };
let a: <T as Trait<'a>>::Type = make_any();
let b: <T as Trait<'b>>::Type = make_any();
let _c: <T as Trait<'a>>::Type = a;
}

fn method2<'a,'b,T>(x: &'a T, y: &'b T)
where T : for<'z> Trait<'z>, 'a : 'b
{
// Note that &'static T <: &'a T.
let a: <T as Trait<'a>>::Type = loop { };
let b: <T as Trait<'b>>::Type = loop { };
let a: <T as Trait<'a>>::Type = make_any();
let b: <T as Trait<'b>>::Type = make_any();
let _c: <T as Trait<'b>>::Type = a; //~ ERROR E0623
}

fn method3<'a,'b,T>(x: &'a T, y: &'b T)
where T : for<'z> Trait<'z>, 'a : 'b
{
// Note that &'static T <: &'a T.
let a: <T as Trait<'a>>::Type = loop { };
let b: <T as Trait<'b>>::Type = loop { };
let a: <T as Trait<'a>>::Type = make_any();
let b: <T as Trait<'b>>::Type = make_any();
let _c: <T as Trait<'a>>::Type = b; //~ ERROR E0623
}

fn method4<'a,'b,T>(x: &'a T, y: &'b T)
where T : for<'z> Trait<'z>, 'a : 'b
{
// Note that &'static T <: &'a T.
let a: <T as Trait<'a>>::Type = loop { };
let b: <T as Trait<'b>>::Type = loop { };
let a: <T as Trait<'a>>::Type = make_any();
let b: <T as Trait<'b>>::Type = make_any();
let _c: <T as Trait<'b>>::Type = b;
}

Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/nll/relate_tys/hr-fn-aaa-as-aba.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ LL | let a: for<'a, 'b> fn(&'a u32, &'b u32) -> &'a u32 = make_it();
| ^^^^^^^^^

error: higher-ranked subtype error
--> $DIR/hr-fn-aaa-as-aba.rs:32:9
--> $DIR/hr-fn-aaa-as-aba.rs:32:12
|
LL | let _: for<'a, 'b> fn(&'a u32, &'b u32) -> &'a u32 = make_it();
| ^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors

4 changes: 2 additions & 2 deletions src/test/ui/nll/user-annotations/patterns.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,12 @@ LL | y //~ ERROR
| ^ returning this value requires that `'a` must outlive `'static`

error: unsatisfied lifetime constraints
--> $DIR/patterns.rs:117:9
--> $DIR/patterns.rs:117:18
|
LL | fn a_to_static_then_static<'a>(x: &'a u32) -> &'static u32 {
| -- lifetime `'a` defined here
LL | let (y, _z): (&'static u32, u32) = (x, 44); //~ ERROR
| ^^^^^^^ type annotation requires that `'a` must outlive `'static`
| ^^^^^^^^^^^^^^^^^^^ type annotation requires that `'a` must outlive `'static`

error: aborting due to 14 previous errors

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
error[E0621]: explicit lifetime required in the type of `v`
--> $DIR/region-object-lifetime-in-coercion.rs:18:9
--> $DIR/region-object-lifetime-in-coercion.rs:18:12
|
LL | fn a(v: &[u8]) -> Box<Foo + 'static> {
| ----- help: add explicit lifetime `'static` to the type of `v`: `&'static [u8]`
LL | let x: Box<Foo + 'static> = Box::new(v);
| ^ lifetime `'static` required
| ^^^^^^^^^^^^^^^^^^ lifetime `'static` required

error[E0621]: explicit lifetime required in the type of `v`
--> $DIR/region-object-lifetime-in-coercion.rs:24:5
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/regions/regions-addr-of-self.nll.stderr
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
error: unsatisfied lifetime constraints
--> $DIR/regions-addr-of-self.rs:17:13
--> $DIR/regions-addr-of-self.rs:17:16
|
LL | pub fn chase_cat(&mut self) {
| - let's call the lifetime of this reference `'1`
LL | let p: &'static mut usize = &mut self.cats_chased; //~ ERROR cannot infer
| ^ type annotation requires that `'1` must outlive `'static`
| ^^^^^^^^^^^^^^^^^^ type annotation requires that `'1` must outlive `'static`

error: aborting due to previous error

Loading