From d6772cb972a5ca531ecbe45ad779bafe33895caa Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Tue, 28 Nov 2017 00:39:38 -0300 Subject: [PATCH 01/22] Check Repeat Rvalue --- src/librustc/infer/mod.rs | 4 ++ src/librustc_mir/transform/type_check.rs | 53 +++++++++++++++++-- .../nll/where_clauses_in_repeat_rvalue.rs | 39 ++++++++++++++ 3 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 src/test/compile-fail/nll/where_clauses_in_repeat_rvalue.rs diff --git a/src/librustc/infer/mod.rs b/src/librustc/infer/mod.rs index f5595d07340..9788ec2a5e4 100644 --- a/src/librustc/infer/mod.rs +++ b/src/librustc/infer/mod.rs @@ -1201,6 +1201,10 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { /// translate them into the form that the NLL solver /// understands. See the NLL module for mode details. pub fn take_and_reset_region_constraints(&self) -> RegionConstraintData<'tcx> { + assert!(self.region_obligations.borrow().is_empty(), + "region_obligations not empty: {:#?}", + self.region_obligations.borrow()); + self.borrow_region_constraints().take_and_reset_data() } diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index 1a74f327001..3599a3174f7 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -17,7 +17,7 @@ use rustc::infer::region_constraints::RegionConstraintData; use rustc::traits::{self, FulfillmentContext}; use rustc::ty::error::TypeError; use rustc::ty::fold::TypeFoldable; -use rustc::ty::{self, Ty, TyCtxt, TypeVariants}; +use rustc::ty::{self, Ty, TyCtxt, TypeVariants, ToPolyTraitRef}; use rustc::middle::const_val::ConstVal; use rustc::mir::*; use rustc::mir::tcx::PlaceTy; @@ -545,6 +545,13 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { span_mirbug!(self, "", "errors selecting obligation: {:?}", e); } + self.infcx.process_registered_region_obligations( + &[], + None, + self.param_env, + self.body_id, + ); + let data = self.infcx.take_and_reset_region_constraints(); if !data.is_empty() { self.constraints @@ -1110,13 +1117,28 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { } fn check_rvalue(&mut self, mir: &Mir<'tcx>, rvalue: &Rvalue<'tcx>, location: Location) { + let tcx = self.tcx(); + match rvalue { Rvalue::Aggregate(ak, ops) => { self.check_aggregate_rvalue(mir, rvalue, ak, ops, location) } + + Rvalue::Repeat(operand, const_usize) => { + if const_usize.as_u64() > 1 { + let operand_ty = operand.ty(mir, tcx); + + let trait_ref = ty::TraitRef { + def_id: tcx.lang_items().copy_trait().unwrap(), + substs: tcx.mk_substs_trait(operand_ty, &[]), + }; + + self.prove_trait_ref(trait_ref, location); + } + } + // FIXME: These other cases have to be implemented in future PRs Rvalue::Use(..) | - Rvalue::Repeat(..) | Rvalue::Ref(..) | Rvalue::Len(..) | Rvalue::Cast(..) | @@ -1205,6 +1227,31 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { } } + fn prove_trait_ref(&mut self, trait_ref: ty::TraitRef<'tcx>, location: Location) { + self.prove_predicates( + &[ + ty::Predicate::Trait(trait_ref.to_poly_trait_ref().to_poly_trait_predicate()), + ], + location, + ); + } + + fn prove_predicates(&mut self, predicates: &[ty::Predicate<'tcx>], location: Location) { + self.fully_perform_op(location.at_self(), |this| { + let cause = this.misc(this.last_span); + let obligations = predicates + .iter() + .map(|&p| { + traits::Obligation::new(cause.clone(), this.param_env, p) + }) + .collect(); + Ok(InferOk { + value: (), + obligations, + }) + }).unwrap() + } + fn typeck_mir(&mut self, mir: &Mir<'tcx>) { self.last_span = mir.span; debug!("run_on_mir: {:?}", mir.span); @@ -1237,7 +1284,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { { self.fully_perform_op(location.at_self(), |this| { let mut selcx = traits::SelectionContext::new(this.infcx); - let cause = traits::ObligationCause::misc(this.last_span, ast::CRATE_NODE_ID); + let cause = this.misc(this.last_span); let traits::Normalized { value, obligations } = traits::normalize(&mut selcx, this.param_env, cause, value); Ok(InferOk { value, obligations }) diff --git a/src/test/compile-fail/nll/where_clauses_in_repeat_rvalue.rs b/src/test/compile-fail/nll/where_clauses_in_repeat_rvalue.rs new file mode 100644 index 00000000000..aae0cd3fdb0 --- /dev/null +++ b/src/test/compile-fail/nll/where_clauses_in_repeat_rvalue.rs @@ -0,0 +1,39 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -Z borrowck=mir -Z nll + +#![allow(warnings)] + +struct Foo { + t: T, +} + +impl Copy for Foo {} +impl Clone for Foo { + fn clone(&self) -> Self { + *self + } +} + +fn main() { + let mut x = 22; + + { + let p = &x; //~ ERROR borrowed value does not live long enough + let w = Foo { t: p }; + + let v = [w; 22]; + } + + x += 1; + //~^ ERROR cannot assign to `x` because it is borrowed [E0506] +} +//~^ ERROR borrowed value does not live long enough [E0597] From 7f20b9142d278c0ce68e7ce48e3ef92201c61ab4 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 26 Nov 2017 20:05:18 -0500 Subject: [PATCH 02/22] fix universal regions to handle constant expressions like `[T; 22]` --- .../borrow_check/nll/universal_regions.rs | 32 +++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/borrow_check/nll/universal_regions.rs b/src/librustc_mir/borrow_check/nll/universal_regions.rs index 857a620cead..c8b5a258a70 100644 --- a/src/librustc_mir/borrow_check/nll/universal_regions.rs +++ b/src/librustc_mir/borrow_check/nll/universal_regions.rs @@ -493,7 +493,27 @@ impl<'cx, 'gcx, 'tcx> UniversalRegionsBuilder<'cx, 'gcx, 'tcx> { substs.substs } ty::TyFnDef(_, substs) => substs, - _ => bug!(), + + // FIXME. When we encounter other sorts of constant + // expressions, such as the `22` in `[foo; 22]`, we can + // get the type `usize` here. For now, just return an + // empty vector of substs in this case, since there are no + // generics in scope in such expressions right now. + // + // Eventually I imagine we could get a wider range of + // types. What is the best way to handle this? Should we + // be checking something other than the type of the def-id + // to figure out what to do (e.g. the def-key?). + ty::TyUint(..) => { + assert!(identity_substs.is_empty()); + identity_substs + } + + _ => span_bug!( + tcx.def_span(self.mir_def_id), + "unknown defining type: {:?}", + defining_ty + ), }; let global_mapping = iter::once((gcx.types.re_static, fr_static)); @@ -551,7 +571,15 @@ impl<'cx, 'gcx, 'tcx> UniversalRegionsBuilder<'cx, 'gcx, 'tcx> { ty::TyFnDef(def_id, _) => { let sig = tcx.fn_sig(def_id); let sig = indices.fold_to_region_vids(tcx, &sig); - return sig.inputs_and_output(); + sig.inputs_and_output() + } + + // FIXME: as above, this happens on things like `[foo; + // 22]`. For now, no inputs, one output, but it seems like + // we need a more general way to handle this category of + // MIR. + ty::TyUint(..) => { + ty::Binder::dummy(tcx.mk_type_list(iter::once(defining_ty))) } _ => span_bug!( From c9159262d161cd64d828f7821dca5e07926e34f2 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Mon, 20 Nov 2017 11:09:45 -0300 Subject: [PATCH 03/22] Check NullaryOp Rvalue --- src/librustc_mir/transform/type_check.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index 3599a3174f7..6c6ae3d02da 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -1137,6 +1137,15 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { } } + Rvalue::NullaryOp(_, ty) => { + let trait_ref = ty::TraitRef { + def_id: tcx.lang_items().sized_trait().unwrap(), + substs: tcx.mk_substs_trait(ty, &[]), + }; + + self.prove_trait_ref(trait_ref, location); + } + // FIXME: These other cases have to be implemented in future PRs Rvalue::Use(..) | Rvalue::Ref(..) | @@ -1145,8 +1154,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { Rvalue::BinaryOp(..) | Rvalue::CheckedBinaryOp(..) | Rvalue::UnaryOp(..) | - Rvalue::Discriminant(..) | - Rvalue::NullaryOp(..) => {} + Rvalue::Discriminant(..) => {} } } From 5010496677ee5eb2bdfcbf104eaaa19130d0dc17 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sun, 26 Nov 2017 17:28:04 -0300 Subject: [PATCH 04/22] Check Aggregate predicates --- src/librustc_mir/transform/type_check.rs | 27 ++++++++++++++++++ .../nll/where_clauses_in_structs.rs | 28 +++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 src/test/compile-fail/nll/where_clauses_in_structs.rs diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index 6c6ae3d02da..19c22d7017d 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -1168,6 +1168,8 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { ) { let tcx = self.tcx(); + self.prove_aggregate_predicates(aggregate_kind, location); + match aggregate_kind { // tuple rvalue field type is always the type of the op. Nothing to check here. AggregateKind::Tuple => return, @@ -1235,6 +1237,31 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { } } + fn prove_aggregate_predicates( + &mut self, + aggregate_kind: &AggregateKind<'tcx>, + location: Location, + ) { + let tcx = self.tcx(); + + let instantiated_predicates = match aggregate_kind { + AggregateKind::Adt(def, _, substs, _) => { + tcx.predicates_of(def.did).instantiate(tcx, substs) + } + + AggregateKind::Closure(def_id, substs) | + AggregateKind::Generator(def_id, substs, _) => { + tcx.predicates_of(*def_id).instantiate(tcx, substs.substs) + } + + AggregateKind::Array(_) | + AggregateKind::Tuple => ty::InstantiatedPredicates::empty(), + }; + + let predicates = self.normalize(&instantiated_predicates.predicates, location); + self.prove_predicates(&predicates, location); + } + fn prove_trait_ref(&mut self, trait_ref: ty::TraitRef<'tcx>, location: Location) { self.prove_predicates( &[ diff --git a/src/test/compile-fail/nll/where_clauses_in_structs.rs b/src/test/compile-fail/nll/where_clauses_in_structs.rs new file mode 100644 index 00000000000..0c4fd5dead3 --- /dev/null +++ b/src/test/compile-fail/nll/where_clauses_in_structs.rs @@ -0,0 +1,28 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -Z borrowck=mir -Z nll + +#![allow(dead_code)] + +use std::cell::Cell; + +struct Foo<'a: 'b, 'b> { + x: Cell<&'a u32>, + y: Cell<&'b u32>, +} + +fn bar<'a, 'b>(x: Cell<&'a u32>, y: Cell<&'b u32>) { + Foo { x, y }; + //~^ ERROR free region `'_#1r` does not outlive free region `'_#2r` + //~| WARNING not reporting region error due to -Znll +} + +fn main() {} From 688ab5af8198f4da45e90f2cf4a2d37ce97d5e8d Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Mon, 27 Nov 2017 12:25:01 -0300 Subject: [PATCH 05/22] Check functions predicates --- src/librustc_mir/transform/type_check.rs | 44 ++++++++++++++++++- .../nll/where_clauses_in_functions.rs | 28 ++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) create mode 100644 src/test/compile-fail/nll/where_clauses_in_functions.rs diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index 19c22d7017d..e722e583ad2 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -59,7 +59,10 @@ pub fn type_check<'a, 'gcx, 'tcx>( } fn mirbug(tcx: TyCtxt, span: Span, msg: &str) { - tcx.sess.diagnostic().span_bug(span, msg); + // We sometimes see MIR failures (notably predicate failures) due to + // the fact that we check rvalue sized predicates here. So use `delay_span_bug` + // to avoid reporting bugs in those cases. + tcx.sess.diagnostic().delay_span_bug(span, msg); } macro_rules! span_mirbug { @@ -171,7 +174,44 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> { ); let expected_ty = match constant.literal { - Literal::Value { value } => value.ty, + Literal::Value { value } => { + if let ConstVal::Function(def_id, ..) = value.val { + let tcx = self.tcx(); + let type_checker = &mut self.cx; + + // FIXME -- For now, use the substitutions from + // `value.ty` rather than `value.val`. The + // renumberer will rewrite them to independent + // sets of regions; in principle, we ought to + // derive the type of the `value.val` from "first + // principles" and equate with value.ty, but as we + // are transitioning to the miri-based system, we + // don't have a handy function for that, so for + // now we just ignore `value.val` regions. + let substs = match value.ty.sty { + ty::TyFnDef(ty_def_id, substs) => { + assert_eq!(def_id, ty_def_id); + substs + } + _ => { + span_bug!( + self.last_span, + "unexpected type for constant function: {:?}", + value.ty + ) + } + }; + + let instantiated_predicates = + tcx.predicates_of(def_id).instantiate(tcx, substs); + let predicates = + type_checker.normalize(&instantiated_predicates.predicates, location); + type_checker.prove_predicates(&predicates, location); + } + + value.ty + } + Literal::Promoted { .. } => { // FIXME -- promoted MIR return types reference // various "free regions" (e.g., scopes and things) diff --git a/src/test/compile-fail/nll/where_clauses_in_functions.rs b/src/test/compile-fail/nll/where_clauses_in_functions.rs new file mode 100644 index 00000000000..a13360aeca7 --- /dev/null +++ b/src/test/compile-fail/nll/where_clauses_in_functions.rs @@ -0,0 +1,28 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -Z borrowck=mir -Z nll + +#![allow(dead_code)] + +fn foo<'a, 'b>(x: &'a u32, y: &'b u32) -> (&'a u32, &'b u32) +where + 'a: 'b, +{ + (x, y) +} + +fn bar<'a, 'b>(x: &'a u32, y: &'b u32) -> (&'a u32, &'b u32) { + foo(x, y) + //~^ ERROR free region `'_#1r` does not outlive free region `'_#2r` + //~| WARNING not reporting region error due to -Znll +} + +fn main() {} From 4449240d1ec360e8712c1e17e0b01c6ab5d02845 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Tue, 28 Nov 2017 00:54:50 -0300 Subject: [PATCH 06/22] Add more debug logs --- src/librustc/infer/outlives/obligations.rs | 16 +++++++++++++++- src/librustc_mir/transform/type_check.rs | 12 ++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/librustc/infer/outlives/obligations.rs b/src/librustc/infer/outlives/obligations.rs index 07eacde0aab..eda2e1f7b4e 100644 --- a/src/librustc/infer/outlives/obligations.rs +++ b/src/librustc/infer/outlives/obligations.rs @@ -88,7 +88,12 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> { body_id: ast::NodeId, obligation: RegionObligation<'tcx>, ) { - debug!("register_region_obligation({:?}, {:?})", body_id, obligation); + debug!( + "register_region_obligation(body_id={:?}, obligation={:?})", + body_id, + obligation + ); + self.region_obligations .borrow_mut() .push((body_id, obligation)); @@ -139,6 +144,8 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> { "cannot process registered region obligations in a snapshot" ); + debug!("process_registered_region_obligations()"); + // pull out the region obligations with the given `body_id` (leaving the rest) let mut my_region_obligations = Vec::with_capacity(self.region_obligations.borrow().len()); { @@ -157,6 +164,13 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> { cause, } in my_region_obligations { + debug!( + "process_registered_region_obligations: sup_type={:?} sub_region={:?} cause={:?}", + sup_type, + sub_region, + cause + ); + let origin = SubregionOrigin::from_obligation_cause( &cause, || infer::RelateParamBound(cause.span, sup_type), diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index e722e583ad2..13e517c9c79 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -1284,6 +1284,12 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { ) { let tcx = self.tcx(); + debug!( + "prove_aggregate_predicates(aggregate_kind={:?}, location={:?})", + aggregate_kind, + location + ); + let instantiated_predicates = match aggregate_kind { AggregateKind::Adt(def, _, substs, _) => { tcx.predicates_of(def.did).instantiate(tcx, substs) @@ -1299,6 +1305,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { }; let predicates = self.normalize(&instantiated_predicates.predicates, location); + debug!("prove_aggregate_predicates: predicates={:?}", predicates); self.prove_predicates(&predicates, location); } @@ -1312,6 +1319,11 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { } fn prove_predicates(&mut self, predicates: &[ty::Predicate<'tcx>], location: Location) { + debug!( + "prove_predicates(predicates={:?}, location={:?})", + predicates, + location + ); self.fully_perform_op(location.at_self(), |this| { let cause = this.misc(this.last_span); let obligations = predicates From 86355480bd965c519cab418794fdc0f5e8202888 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Tue, 28 Nov 2017 14:54:17 -0300 Subject: [PATCH 07/22] Restructure a bit check_aggregate_rvalue code --- src/librustc_mir/transform/type_check.rs | 70 ++++++++++++------------ 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index 13e517c9c79..07b14abc4c3 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -1210,41 +1210,9 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { self.prove_aggregate_predicates(aggregate_kind, location); - match aggregate_kind { + if *aggregate_kind == AggregateKind::Tuple { // tuple rvalue field type is always the type of the op. Nothing to check here. - AggregateKind::Tuple => return, - - // For closures, we have some **extra requirements** we - // have to check. In particular, in their upvars and - // signatures, closures often reference various regions - // from the surrounding function -- we call those the - // closure's free regions. When we borrow-check (and hence - // region-check) closures, we may find that the closure - // requires certain relationships between those free - // regions. However, because those free regions refer to - // portions of the CFG of their caller, the closure is not - // in a position to verify those relationships. In that - // case, the requirements get "propagated" to us, and so - // we have to solve them here where we instantiate the - // closure. - // - // Despite the opacity of the previous parapgrah, this is - // actually relatively easy to understand in terms of the - // desugaring. A closure gets desugared to a struct, and - // these extra requirements are basically like where - // clauses on the struct. - AggregateKind::Closure(def_id, substs) => { - if let Some(closure_region_requirements) = tcx.mir_borrowck(*def_id) { - closure_region_requirements.apply_requirements( - self.infcx, - location, - *def_id, - *substs, - ); - } - } - - _ => {} + return; } for (i, operand) in operands.iter().enumerate() { @@ -1295,7 +1263,39 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { tcx.predicates_of(def.did).instantiate(tcx, substs) } - AggregateKind::Closure(def_id, substs) | + // For closures, we have some **extra requirements** we + // + // have to check. In particular, in their upvars and + // signatures, closures often reference various regions + // from the surrounding function -- we call those the + // closure's free regions. When we borrow-check (and hence + // region-check) closures, we may find that the closure + // requires certain relationships between those free + // regions. However, because those free regions refer to + // portions of the CFG of their caller, the closure is not + // in a position to verify those relationships. In that + // case, the requirements get "propagated" to us, and so + // we have to solve them here where we instantiate the + // closure. + // + // Despite the opacity of the previous parapgrah, this is + // actually relatively easy to understand in terms of the + // desugaring. A closure gets desugared to a struct, and + // these extra requirements are basically like where + // clauses on the struct. + AggregateKind::Closure(def_id, substs) => { + if let Some(closure_region_requirements) = tcx.mir_borrowck(*def_id) { + closure_region_requirements.apply_requirements( + self.infcx, + location, + *def_id, + *substs, + ); + } + + tcx.predicates_of(*def_id).instantiate(tcx, substs.substs) + } + AggregateKind::Generator(def_id, substs, _) => { tcx.predicates_of(*def_id).instantiate(tcx, substs.substs) } From 7d56131e831baf6053b5ae3021d11f3b622c3862 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Wed, 29 Nov 2017 08:38:07 -0300 Subject: [PATCH 08/22] Mir typeck Cast for ReifyFnPtr value --- src/librustc_mir/transform/type_check.rs | 19 ++++++- src/test/compile-fail/mir_check_cast_reify.rs | 52 +++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 src/test/compile-fail/mir_check_cast_reify.rs diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index 07b14abc4c3..dcd97f2a7be 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -1186,11 +1186,28 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { self.prove_trait_ref(trait_ref, location); } + Rvalue::Cast(cast_kind, op, ty) => { + match cast_kind { + CastKind::ReifyFnPointer => { + let ty_fn_ptr_from = tcx.mk_fn_ptr(op.ty(mir, tcx).fn_sig(tcx)); + + if let Err(terr) = self.eq_types(ty_fn_ptr_from, ty, location.at_self()) { + span_mirbug!(self, "", "casting {:?}", terr); + } + } + + CastKind::ClosureFnPointer | + CastKind::UnsafeFnPointer | + CastKind::Misc | + CastKind::Unsize => {} + + } + } + // FIXME: These other cases have to be implemented in future PRs Rvalue::Use(..) | Rvalue::Ref(..) | Rvalue::Len(..) | - Rvalue::Cast(..) | Rvalue::BinaryOp(..) | Rvalue::CheckedBinaryOp(..) | Rvalue::UnaryOp(..) | diff --git a/src/test/compile-fail/mir_check_cast_reify.rs b/src/test/compile-fail/mir_check_cast_reify.rs new file mode 100644 index 00000000000..a253243bad7 --- /dev/null +++ b/src/test/compile-fail/mir_check_cast_reify.rs @@ -0,0 +1,52 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -Z borrowck=mir -Z nll + +#![allow(dead_code)] + +// Test that we relate the type of the fn type to the type of the fn +// ptr when doing a `ReifyFnPointer` cast. +// +// This test is a bit tortured, let me explain: +// + +// The `where 'a: 'a` clause here ensures that `'a` is early bound, +// which is needed below to ensure that this test hits the path we are +// concerned with. +fn foo<'a>(x: &'a u32) -> &'a u32 +where + 'a: 'a, +{ + panic!() +} + +fn bar<'a>(x: &'a u32) -> &'static u32 { + // Here, the type of `foo` is `typeof(foo::<'x>)` for some fresh variable `'x`. + // During NLL region analysis, this will get renumbered to `typeof(foo::<'?0>)` + // where `'?0` is a new region variable. + // + // (Note that if `'a` on `foo` were early-bound, the type would be + // `typeof(foo)`, which would interact differently with because + // the renumbering later.) + // + // This type is then coerced to a fn type `fn(&'?1 u32) -> &'?2 + // u32`. Here, the `'?1` and `'?2` will have been created during + // the NLL region renumbering. + // + // The MIR type checker must therefore relate `'?0` to `'?1` and `'?2` + // as part of checking the `ReifyFnPointer`. + let f: fn(_) -> _ = foo; + //~^ WARNING not reporting region error due to -Znll + f(x) + //~^ ERROR free region `'_#1r` does not outlive free region `'static` +} + +fn main() {} From 900d4d5bda06eb87b5a8a36713720095ef6951e1 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 30 Nov 2017 11:07:04 -0300 Subject: [PATCH 09/22] Mir typeck Cast for UnsafeFnPtr value --- src/librustc_mir/transform/type_check.rs | 10 ++++++-- .../compile-fail/mir_check_cast_unsafe_fn.rs | 24 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 src/test/compile-fail/mir_check_cast_unsafe_fn.rs diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index dcd97f2a7be..0c481f964a7 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -1196,11 +1196,17 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { } } + CastKind::UnsafeFnPointer => { + let ty_fn_ptr_from = tcx.safe_to_unsafe_fn_ty(op.ty(mir, tcx).fn_sig(tcx)); + + if let Err(terr) = self.eq_types(ty_fn_ptr_from, ty, location.at_self()) { + span_mirbug!(self, "", "casting {:?}", terr); + } + } + CastKind::ClosureFnPointer | - CastKind::UnsafeFnPointer | CastKind::Misc | CastKind::Unsize => {} - } } diff --git a/src/test/compile-fail/mir_check_cast_unsafe_fn.rs b/src/test/compile-fail/mir_check_cast_unsafe_fn.rs new file mode 100644 index 00000000000..701a7c6b056 --- /dev/null +++ b/src/test/compile-fail/mir_check_cast_unsafe_fn.rs @@ -0,0 +1,24 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -Z borrowck=mir -Z nll + +#![allow(dead_code)] + +fn bar<'a>(input: &'a u32, f: fn(&'a u32) -> &'a u32) -> &'static u32 { + // Here the NLL checker must relate the types in `f` to the types + // in `g`. These are related via the `UnsafeFnPointer` cast. + let g: unsafe fn(_) -> _ = f; + //~^ WARNING not reporting region error due to -Znll + unsafe { g(input) } + //~^ ERROR free region `'_#1r` does not outlive free region `'static` +} + +fn main() {} From ae035cb7311ab734630ba40d1b1409d76ac2105a Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 30 Nov 2017 12:22:11 -0300 Subject: [PATCH 10/22] Extract coerce_closure_fn_ty function --- src/librustc/ty/context.rs | 21 +++++++++++++++++++++ src/librustc_typeck/check/coercion.rs | 18 +----------------- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 4315d59f771..441dd792ee1 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -1717,6 +1717,27 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { })) } + /// Create an unsafe fn ty based on a safe fn ty. + pub fn coerce_closure_fn_ty(self, sig: PolyFnSig<'tcx>) -> Ty<'tcx> { + let converted_sig = sig.map_bound(|s| { + let params_iter = match s.inputs()[0].sty { + ty::TyTuple(params, _) => { + params.into_iter().cloned() + } + _ => bug!(), + }; + self.mk_fn_sig( + params_iter, + s.output(), + s.variadic, + hir::Unsafety::Normal, + abi::Abi::Rust + ) + }); + + self.mk_fn_ptr(converted_sig) + } + // Interns a type/name combination, stores the resulting box in cx.interners, // and returns the box as cast to an unsafe ptr (see comments for Ty above). pub fn mk_ty(self, st: TypeVariants<'tcx>) -> Ty<'tcx> { diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index dc5d3141d4c..079dc2964a3 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -75,7 +75,6 @@ use rustc::ty::fold::TypeFoldable; use rustc::ty::error::TypeError; use rustc::ty::relate::RelateResult; use errors::DiagnosticBuilder; -use syntax::abi; use syntax::feature_gate; use syntax::ptr::P; use syntax_pos; @@ -670,22 +669,7 @@ impl<'f, 'gcx, 'tcx> Coerce<'f, 'gcx, 'tcx> { // to // `fn(arg0,arg1,...) -> _` let sig = self.closure_sig(def_id_a, substs_a); - let converted_sig = sig.map_bound(|s| { - let params_iter = match s.inputs()[0].sty { - ty::TyTuple(params, _) => { - params.into_iter().cloned() - } - _ => bug!(), - }; - self.tcx.mk_fn_sig( - params_iter, - s.output(), - s.variadic, - hir::Unsafety::Normal, - abi::Abi::Rust - ) - }); - let pointer_ty = self.tcx.mk_fn_ptr(converted_sig); + let pointer_ty = self.tcx.coerce_closure_fn_ty(sig); debug!("coerce_closure_to_fn(a={:?}, b={:?}, pty={:?})", a, b, pointer_ty); self.unify_and(pointer_ty, b, simple(Adjust::ClosureFnPointer)) From 14700e58b430ddddef037715778e452db8763ae9 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 30 Nov 2017 12:43:32 -0300 Subject: [PATCH 11/22] Mir typeck Cast for ClosureFnPtr value --- src/librustc_mir/transform/type_check.rs | 18 ++++++++++++--- .../compile-fail/mir_check_cast_closure.rs | 22 +++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 src/test/compile-fail/mir_check_cast_closure.rs diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index 0c481f964a7..00e60bd1910 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -1196,6 +1196,20 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { } } + CastKind::ClosureFnPointer => { + let sig = match op.ty(mir, tcx).sty { + ty::TyClosure(def_id, substs) => { + substs.closure_sig_ty(def_id, tcx).fn_sig(tcx) + } + _ => bug!(), + }; + let ty_fn_ptr_from = tcx.coerce_closure_fn_ty(sig); + + if let Err(terr) = self.eq_types(ty_fn_ptr_from, ty, location.at_self()) { + span_mirbug!(self, "", "casting {:?}", terr); + } + } + CastKind::UnsafeFnPointer => { let ty_fn_ptr_from = tcx.safe_to_unsafe_fn_ty(op.ty(mir, tcx).fn_sig(tcx)); @@ -1204,9 +1218,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { } } - CastKind::ClosureFnPointer | - CastKind::Misc | - CastKind::Unsize => {} + CastKind::Misc | CastKind::Unsize => {} } } diff --git a/src/test/compile-fail/mir_check_cast_closure.rs b/src/test/compile-fail/mir_check_cast_closure.rs new file mode 100644 index 00000000000..be0d4b13741 --- /dev/null +++ b/src/test/compile-fail/mir_check_cast_closure.rs @@ -0,0 +1,22 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -Z borrowck=mir -Z nll + +#![allow(dead_code)] + +fn bar<'a, 'b>() -> fn(&'a u32, &'b u32) -> &'a u32 { + let g: fn(_, _) -> _ = |_x, y| y; + g + //~^ WARNING not reporting region error due to -Znll + //~| ERROR free region `'b` does not outlive free region `'a` +} + +fn main() {} From 0c26d8fcd1b171beaf467d1e6ea0157b88bc5a2a Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 30 Nov 2017 16:09:27 -0300 Subject: [PATCH 12/22] Mir typeck Cast for Unsize value --- src/librustc_mir/transform/type_check.rs | 11 ++++++++- .../compile-fail/mir_check_cast_unsize.rs | 24 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 src/test/compile-fail/mir_check_cast_unsize.rs diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index 00e60bd1910..0de258f6ed7 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -1218,7 +1218,16 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { } } - CastKind::Misc | CastKind::Unsize => {} + CastKind::Unsize => { + let trait_ref = ty::TraitRef { + def_id: tcx.lang_items().coerce_unsized_trait().unwrap(), + substs: tcx.mk_substs_trait(op.ty(mir, tcx), &[ty]), + }; + + self.prove_trait_ref(trait_ref, location); + } + + CastKind::Misc => {} } } diff --git a/src/test/compile-fail/mir_check_cast_unsize.rs b/src/test/compile-fail/mir_check_cast_unsize.rs new file mode 100644 index 00000000000..bc867047ab5 --- /dev/null +++ b/src/test/compile-fail/mir_check_cast_unsize.rs @@ -0,0 +1,24 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -Z borrowck=mir -Z nll + +#![allow(dead_code)] +#![feature(dyn_trait)] + +use std::fmt::Debug; + +fn bar<'a>(x: &'a u32) -> &'static dyn Debug { + //~^ ERROR free region `'_#1r` does not outlive free region `'static` + x + //~^ WARNING not reporting region error due to -Znll +} + +fn main() {} From d5cff0740fa761aea4aa22d0e3b1637730dda460 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 1 Dec 2017 13:55:27 -0500 Subject: [PATCH 13/22] normalize fn sig as part of reification --- src/librustc_mir/transform/type_check.rs | 162 ++++++++++-------- .../run-pass/mir-typeck-normalize-fn-sig.rs | 38 ++++ 2 files changed, 133 insertions(+), 67 deletions(-) create mode 100644 src/test/run-pass/mir-typeck-normalize-fn-sig.rs diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index 0de258f6ed7..0fa24b3478d 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -17,7 +17,7 @@ use rustc::infer::region_constraints::RegionConstraintData; use rustc::traits::{self, FulfillmentContext}; use rustc::ty::error::TypeError; use rustc::ty::fold::TypeFoldable; -use rustc::ty::{self, Ty, TyCtxt, TypeVariants, ToPolyTraitRef}; +use rustc::ty::{self, ToPolyTraitRef, Ty, TyCtxt, TypeVariants}; use rustc::middle::const_val::ConstVal; use rustc::mir::*; use rustc::mir::tcx::PlaceTy; @@ -193,13 +193,11 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> { assert_eq!(def_id, ty_def_id); substs } - _ => { - span_bug!( - self.last_span, - "unexpected type for constant function: {:?}", - value.ty - ) - } + _ => span_bug!( + self.last_span, + "unexpected type for constant function: {:?}", + value.ty + ), }; let instantiated_predicates = @@ -585,12 +583,8 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { span_mirbug!(self, "", "errors selecting obligation: {:?}", e); } - self.infcx.process_registered_region_obligations( - &[], - None, - self.param_env, - self.body_id, - ); + self.infcx + .process_registered_region_obligations(&[], None, self.param_env, self.body_id); let data = self.infcx.take_and_reset_region_constraints(); if !data.is_empty() { @@ -1164,18 +1158,16 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { self.check_aggregate_rvalue(mir, rvalue, ak, ops, location) } - Rvalue::Repeat(operand, const_usize) => { - if const_usize.as_u64() > 1 { - let operand_ty = operand.ty(mir, tcx); + Rvalue::Repeat(operand, const_usize) => if const_usize.as_u64() > 1 { + let operand_ty = operand.ty(mir, tcx); - let trait_ref = ty::TraitRef { - def_id: tcx.lang_items().copy_trait().unwrap(), - substs: tcx.mk_substs_trait(operand_ty, &[]), - }; + let trait_ref = ty::TraitRef { + def_id: tcx.lang_items().copy_trait().unwrap(), + substs: tcx.mk_substs_trait(operand_ty, &[]), + }; - self.prove_trait_ref(trait_ref, location); - } - } + self.prove_trait_ref(trait_ref, location); + }, Rvalue::NullaryOp(_, ty) => { let trait_ref = ty::TraitRef { @@ -1186,50 +1178,87 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { self.prove_trait_ref(trait_ref, location); } - Rvalue::Cast(cast_kind, op, ty) => { - match cast_kind { - CastKind::ReifyFnPointer => { - let ty_fn_ptr_from = tcx.mk_fn_ptr(op.ty(mir, tcx).fn_sig(tcx)); + Rvalue::Cast(cast_kind, op, ty) => match cast_kind { + CastKind::ReifyFnPointer => { + let fn_sig = op.ty(mir, tcx).fn_sig(tcx); - if let Err(terr) = self.eq_types(ty_fn_ptr_from, ty, location.at_self()) { - span_mirbug!(self, "", "casting {:?}", terr); - } + // The type that we see in the fcx is like + // `foo::<'a, 'b>`, where `foo` is the path to a + // function definition. When we extract the + // signature, it comes from the `fn_sig` query, + // and hence may contain unnormalized results. + let fn_sig = self.normalize(&fn_sig, location); + + let ty_fn_ptr_from = tcx.mk_fn_ptr(fn_sig); + + if let Err(terr) = self.eq_types(ty_fn_ptr_from, ty, location.at_self()) { + span_mirbug!( + self, + rvalue, + "equating {:?} with {:?} yields {:?}", + ty_fn_ptr_from, + ty, + terr + ); } - - CastKind::ClosureFnPointer => { - let sig = match op.ty(mir, tcx).sty { - ty::TyClosure(def_id, substs) => { - substs.closure_sig_ty(def_id, tcx).fn_sig(tcx) - } - _ => bug!(), - }; - let ty_fn_ptr_from = tcx.coerce_closure_fn_ty(sig); - - if let Err(terr) = self.eq_types(ty_fn_ptr_from, ty, location.at_self()) { - span_mirbug!(self, "", "casting {:?}", terr); - } - } - - CastKind::UnsafeFnPointer => { - let ty_fn_ptr_from = tcx.safe_to_unsafe_fn_ty(op.ty(mir, tcx).fn_sig(tcx)); - - if let Err(terr) = self.eq_types(ty_fn_ptr_from, ty, location.at_self()) { - span_mirbug!(self, "", "casting {:?}", terr); - } - } - - CastKind::Unsize => { - let trait_ref = ty::TraitRef { - def_id: tcx.lang_items().coerce_unsized_trait().unwrap(), - substs: tcx.mk_substs_trait(op.ty(mir, tcx), &[ty]), - }; - - self.prove_trait_ref(trait_ref, location); - } - - CastKind::Misc => {} } - } + + CastKind::ClosureFnPointer => { + let sig = match op.ty(mir, tcx).sty { + ty::TyClosure(def_id, substs) => { + substs.closure_sig_ty(def_id, tcx).fn_sig(tcx) + } + _ => bug!(), + }; + let ty_fn_ptr_from = tcx.coerce_closure_fn_ty(sig); + + if let Err(terr) = self.eq_types(ty_fn_ptr_from, ty, location.at_self()) { + span_mirbug!( + self, + rvalue, + "equating {:?} with {:?} yields {:?}", + ty_fn_ptr_from, + ty, + terr + ); + } + } + + CastKind::UnsafeFnPointer => { + let fn_sig = op.ty(mir, tcx).fn_sig(tcx); + + // The type that we see in the fcx is like + // `foo::<'a, 'b>`, where `foo` is the path to a + // function definition. When we extract the + // signature, it comes from the `fn_sig` query, + // and hence may contain unnormalized results. + let fn_sig = self.normalize(&fn_sig, location); + + let ty_fn_ptr_from = tcx.safe_to_unsafe_fn_ty(fn_sig); + + if let Err(terr) = self.eq_types(ty_fn_ptr_from, ty, location.at_self()) { + span_mirbug!( + self, + rvalue, + "equating {:?} with {:?} yields {:?}", + ty_fn_ptr_from, + ty, + terr + ); + } + } + + CastKind::Unsize => { + let trait_ref = ty::TraitRef { + def_id: tcx.lang_items().coerce_unsized_trait().unwrap(), + substs: tcx.mk_substs_trait(op.ty(mir, tcx), &[ty]), + }; + + self.prove_trait_ref(trait_ref, location); + } + + CastKind::Misc => {} + }, // FIXME: These other cases have to be implemented in future PRs Rvalue::Use(..) | @@ -1344,8 +1373,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { tcx.predicates_of(*def_id).instantiate(tcx, substs.substs) } - AggregateKind::Array(_) | - AggregateKind::Tuple => ty::InstantiatedPredicates::empty(), + AggregateKind::Array(_) | AggregateKind::Tuple => ty::InstantiatedPredicates::empty(), }; let predicates = self.normalize(&instantiated_predicates.predicates, location); diff --git a/src/test/run-pass/mir-typeck-normalize-fn-sig.rs b/src/test/run-pass/mir-typeck-normalize-fn-sig.rs new file mode 100644 index 00000000000..11918d3739d --- /dev/null +++ b/src/test/run-pass/mir-typeck-normalize-fn-sig.rs @@ -0,0 +1,38 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// This code was creating an ICE in the MIR type checker. The reason +// is that we are reifying a reference to a function (`foo::<'x>`), +// which involves extracting its signature, but we were not +// normalizing the signature afterwards. As a result, we sometimes got +// errors around the `>::Value`, which can be +// normalized to `f64`. + +#![allow(dead_code)] + +trait Foo<'x> { + type Value; +} + +impl<'x> Foo<'x> for u32 { + type Value = f64; +} + +struct Providers<'x> { + foo: for<'y> fn(x: &'x u32, y: &'y u32) -> >::Value, +} + +fn foo<'y, 'x: 'x>(x: &'x u32, y: &'y u32) -> >::Value { + *x as f64 +} + +fn main() { + Providers { foo }; +} From decbbb3fc03875e7b188d8ed30ded23a87d81696 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 1 Dec 2017 21:11:25 -0500 Subject: [PATCH 14/22] when reifying a safe fn as an unsafe fn ptr, insert two casts Otherwise, `run-pass/typeck-fn-to-unsafe-fn-ptr.rs` fails the MIR type checker. --- src/librustc_typeck/check/coercion.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index 079dc2964a3..edda557c98c 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -638,9 +638,18 @@ impl<'f, 'gcx, 'tcx> Coerce<'f, 'gcx, 'tcx> { self.normalize_associated_types_in_as_infer_ok(self.cause.span, &a_sig); let a_fn_pointer = self.tcx.mk_fn_ptr(a_sig); - let InferOk { value, obligations: o2 } = - self.coerce_from_safe_fn(a_fn_pointer, a_sig, b, - simple(Adjust::ReifyFnPointer), simple(Adjust::ReifyFnPointer))?; + let InferOk { value, obligations: o2 } = self.coerce_from_safe_fn( + a_fn_pointer, + a_sig, + b, + |unsafe_ty| { + vec![ + Adjustment { kind: Adjust::ReifyFnPointer, target: a_fn_pointer }, + Adjustment { kind: Adjust::UnsafeFnPointer, target: unsafe_ty }, + ] + }, + simple(Adjust::ReifyFnPointer) + )?; obligations.extend(o2); Ok(InferOk { value, obligations }) From a30e2259daacb0075cb5d0ecdca63afd8016b607 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 7 Dec 2017 06:25:59 -0500 Subject: [PATCH 15/22] fix closure tests now that MIR typeck works properly These tests had FIXMEs for errors that were not previously being reported. --- .../nll/where_clauses_in_repeat_rvalue.rs | 4 +- src/test/compile-fail/regions-static-bound.rs | 5 +- .../propagate-approximated-ref.rs | 10 ++-- .../propagate-approximated-ref.stderr | 41 ++++++++++------ ...horter-to-static-comparing-against-free.rs | 4 +- ...er-to-static-comparing-against-free.stderr | 27 +++++++---- ...approximated-shorter-to-static-no-bound.rs | 5 +- ...oximated-shorter-to-static-no-bound.stderr | 47 +++++++++++++------ ...roximated-shorter-to-static-wrong-bound.rs | 3 +- ...mated-shorter-to-static-wrong-bound.stderr | 35 ++++++++++---- .../propagate-approximated-val.rs | 5 +- .../propagate-approximated-val.stderr | 41 ++++++++++------ 12 files changed, 145 insertions(+), 82 deletions(-) diff --git a/src/test/compile-fail/nll/where_clauses_in_repeat_rvalue.rs b/src/test/compile-fail/nll/where_clauses_in_repeat_rvalue.rs index aae0cd3fdb0..93c774e8996 100644 --- a/src/test/compile-fail/nll/where_clauses_in_repeat_rvalue.rs +++ b/src/test/compile-fail/nll/where_clauses_in_repeat_rvalue.rs @@ -27,7 +27,8 @@ fn main() { let mut x = 22; { - let p = &x; //~ ERROR borrowed value does not live long enough + let p = &x; + //~^ ERROR `x` does not live long enough let w = Foo { t: p }; let v = [w; 22]; @@ -36,4 +37,3 @@ fn main() { x += 1; //~^ ERROR cannot assign to `x` because it is borrowed [E0506] } -//~^ ERROR borrowed value does not live long enough [E0597] diff --git a/src/test/compile-fail/regions-static-bound.rs b/src/test/compile-fail/regions-static-bound.rs index 9de8ca196f8..678da45fce4 100644 --- a/src/test/compile-fail/regions-static-bound.rs +++ b/src/test/compile-fail/regions-static-bound.rs @@ -24,11 +24,10 @@ fn static_id_wrong_way<'a>(t: &'a ()) -> &'static () where 'static: 'a { fn error(u: &(), v: &()) { static_id(&u); //[ll]~ ERROR cannot infer an appropriate lifetime //[nll]~^ WARNING not reporting region error due to -Znll + //[nll]~| ERROR free region `'_#1r` does not outlive free region `'static` static_id_indirect(&v); //[ll]~ ERROR cannot infer an appropriate lifetime //[nll]~^ WARNING not reporting region error due to -Znll - - // FIXME(#45827) -- MIR type checker shortcomings mean we don't - // see these errors (yet) in nll mode. + //[nll]~| ERROR free region `'_#2r` does not outlive free region `'static` } fn main() {} diff --git a/src/test/ui/nll/closure-requirements/propagate-approximated-ref.rs b/src/test/ui/nll/closure-requirements/propagate-approximated-ref.rs index 006bdaf4c60..80a40581b89 100644 --- a/src/test/ui/nll/closure-requirements/propagate-approximated-ref.rs +++ b/src/test/ui/nll/closure-requirements/propagate-approximated-ref.rs @@ -21,14 +21,8 @@ // // Note: the use of `Cell` here is to introduce invariance. One less // variable. -// -// FIXME(#45827): The `supply` function *ought* to generate an error, but it -// currently does not. This is I believe a shortcoming of the MIR type -// checker: the closure inference is expressing the correct -// requirement, as you can see from the `#[rustc_regions]` output. // compile-flags:-Znll -Zborrowck=mir -Zverbose -// must-compile-successfully #![feature(rustc_attrs)] @@ -57,8 +51,10 @@ fn demand_y<'x, 'y>(_cell_x: &Cell<&'x u32>, _cell_y: &Cell<&'y u32>, _y: &'y u3 #[rustc_regions] fn supply<'a, 'b>(cell_a: Cell<&'a u32>, cell_b: Cell<&'b u32>) { establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y| { + //~^ ERROR free region `'_#1r` does not outlive free region `'_#2r` + // Only works if 'x: 'y: - demand_y(x, y, x.get()) + demand_y(x, y, x.get()) //~ WARNING not reporting region error due to -Znll }); } diff --git a/src/test/ui/nll/closure-requirements/propagate-approximated-ref.stderr b/src/test/ui/nll/closure-requirements/propagate-approximated-ref.stderr index 81eb90020cc..717cf481a01 100644 --- a/src/test/ui/nll/closure-requirements/propagate-approximated-ref.stderr +++ b/src/test/ui/nll/closure-requirements/propagate-approximated-ref.stderr @@ -1,17 +1,19 @@ warning: not reporting region error due to -Znll - --> $DIR/propagate-approximated-ref.rs:61:9 + --> $DIR/propagate-approximated-ref.rs:57:9 | -61 | demand_y(x, y, x.get()) +57 | demand_y(x, y, x.get()) //~ WARNING not reporting region error due to -Znll | ^^^^^^^^^^^^^^^^^^^^^^^ note: External requirements - --> $DIR/propagate-approximated-ref.rs:59:47 + --> $DIR/propagate-approximated-ref.rs:53:47 | -59 | establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y| { +53 | establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y| { | _______________________________________________^ -60 | | // Only works if 'x: 'y: -61 | | demand_y(x, y, x.get()) -62 | | }); +54 | | //~^ ERROR free region `'_#1r` does not outlive free region `'_#2r` +55 | | +56 | | // Only works if 'x: 'y: +57 | | demand_y(x, y, x.get()) //~ WARNING not reporting region error due to -Znll +58 | | }); | |_____^ | = note: defining type: DefId(0/1:18 ~ propagate_approximated_ref[317d]::supply[0]::{{closure}}[0]) with closure substs [ @@ -21,16 +23,25 @@ note: External requirements = note: number of external vids: 3 = note: where '_#1r: '_#2r -note: No external requirements - --> $DIR/propagate-approximated-ref.rs:58:1 +error: free region `'_#1r` does not outlive free region `'_#2r` + --> $DIR/propagate-approximated-ref.rs:53:38 | -58 | / fn supply<'a, 'b>(cell_a: Cell<&'a u32>, cell_b: Cell<&'b u32>) { -59 | | establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y| { -60 | | // Only works if 'x: 'y: -61 | | demand_y(x, y, x.get()) -62 | | }); -63 | | } +53 | establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y| { + | ^^^^^^^ + +note: No external requirements + --> $DIR/propagate-approximated-ref.rs:52:1 + | +52 | / fn supply<'a, 'b>(cell_a: Cell<&'a u32>, cell_b: Cell<&'b u32>) { +53 | | establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y| { +54 | | //~^ ERROR free region `'_#1r` does not outlive free region `'_#2r` +55 | | +... | +58 | | }); +59 | | } | |_^ | = note: defining type: DefId(0/0:6 ~ propagate_approximated_ref[317d]::supply[0]) with substs [] +error: aborting due to previous error + diff --git a/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-comparing-against-free.rs b/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-comparing-against-free.rs index 0a47ee80256..244929d71db 100644 --- a/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-comparing-against-free.rs +++ b/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-comparing-against-free.rs @@ -39,12 +39,10 @@ fn case1() { fn case2() { let a = 0; let cell = Cell::new(&a); + //~^ ERROR `a` does not live long enough // As you can see in the stderr output, this closure propoagates a // requirement that `'a: 'static'. - // - // FIXME(#45827) However, because of shortcomings in the MIR type - // checker, this does not result in errors later on (yet). foo(cell, |cell_a, cell_x| { cell_x.set(cell_a.get()); // forces 'a: 'x, implies 'a = 'static -> borrow error }) diff --git a/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-comparing-against-free.stderr b/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-comparing-against-free.stderr index e2de72ffe93..b93c69dc13f 100644 --- a/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-comparing-against-free.stderr +++ b/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-comparing-against-free.stderr @@ -42,12 +42,12 @@ note: No external requirements = note: defining type: DefId(0/0:5 ~ propagate_approximated_shorter_to_static_comparing_against_free[317d]::case1[0]) with substs [] note: External requirements - --> $DIR/propagate-approximated-shorter-to-static-comparing-against-free.rs:48:15 + --> $DIR/propagate-approximated-shorter-to-static-comparing-against-free.rs:46:15 | -48 | foo(cell, |cell_a, cell_x| { +46 | foo(cell, |cell_a, cell_x| { | _______________^ -49 | | cell_x.set(cell_a.get()); // forces 'a: 'x, implies 'a = 'static -> borrow error -50 | | }) +47 | | cell_x.set(cell_a.get()); // forces 'a: 'x, implies 'a = 'static -> borrow error +48 | | }) | |_____^ | = note: defining type: DefId(0/1:13 ~ propagate_approximated_shorter_to_static_comparing_against_free[317d]::case2[0]::{{closure}}[0]) with closure substs [ @@ -63,13 +63,24 @@ note: No external requirements 39 | / fn case2() { 40 | | let a = 0; 41 | | let cell = Cell::new(&a); -42 | | +42 | | //~^ ERROR `a` does not live long enough ... | -50 | | }) -51 | | } +48 | | }) +49 | | } | |_^ | = note: defining type: DefId(0/0:6 ~ propagate_approximated_shorter_to_static_comparing_against_free[317d]::case2[0]) with substs [] -error: aborting due to previous error +error[E0597]: `a` does not live long enough + --> $DIR/propagate-approximated-shorter-to-static-comparing-against-free.rs:41:26 + | +41 | let cell = Cell::new(&a); + | ^^ does not live long enough +... +49 | } + | - borrowed value only lives until here + | + = note: borrowed value must be valid for lifetime '_#1r... + +error: aborting due to 2 previous errors diff --git a/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-no-bound.rs b/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-no-bound.rs index 5bd170cc304..54007f0191d 100644 --- a/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-no-bound.rs +++ b/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-no-bound.rs @@ -17,7 +17,6 @@ // these errors are not (yet) reported. // compile-flags:-Znll -Zborrowck=mir -Zverbose -// must-compile-successfully #![feature(rustc_attrs)] @@ -44,8 +43,10 @@ fn demand_y<'x, 'y>(_cell_x: &Cell<&'x u32>, _cell_y: &Cell<&'y u32>, _y: &'y u3 #[rustc_regions] fn supply<'a, 'b>(cell_a: Cell<&'a u32>, cell_b: Cell<&'b u32>) { establish_relationships(&cell_a, &cell_b, |_outlives, x, y| { + //~^ ERROR free region `'_#1r` does not outlive free region `ReStatic` + // Only works if 'x: 'y: - demand_y(x, y, x.get()) + demand_y(x, y, x.get()) //~ WARNING not reporting region error due to -Znll }); } diff --git a/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-no-bound.stderr b/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-no-bound.stderr index 8fd6acbbe5d..86b9fecb80e 100644 --- a/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-no-bound.stderr +++ b/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-no-bound.stderr @@ -1,17 +1,19 @@ warning: not reporting region error due to -Znll - --> $DIR/propagate-approximated-shorter-to-static-no-bound.rs:48:9 + --> $DIR/propagate-approximated-shorter-to-static-no-bound.rs:49:9 | -48 | demand_y(x, y, x.get()) +49 | demand_y(x, y, x.get()) //~ WARNING not reporting region error due to -Znll | ^^^^^^^^^^^^^^^^^^^^^^^ note: External requirements - --> $DIR/propagate-approximated-shorter-to-static-no-bound.rs:46:47 + --> $DIR/propagate-approximated-shorter-to-static-no-bound.rs:45:47 | -46 | establish_relationships(&cell_a, &cell_b, |_outlives, x, y| { +45 | establish_relationships(&cell_a, &cell_b, |_outlives, x, y| { | _______________________________________________^ -47 | | // Only works if 'x: 'y: -48 | | demand_y(x, y, x.get()) -49 | | }); +46 | | //~^ ERROR free region `'_#1r` does not outlive free region `ReStatic` +47 | | +48 | | // Only works if 'x: 'y: +49 | | demand_y(x, y, x.get()) //~ WARNING not reporting region error due to -Znll +50 | | }); | |_____^ | = note: defining type: DefId(0/1:18 ~ propagate_approximated_shorter_to_static_no_bound[317d]::supply[0]::{{closure}}[0]) with closure substs [ @@ -21,16 +23,31 @@ note: External requirements = note: number of external vids: 2 = note: where '_#1r: '_#0r -note: No external requirements - --> $DIR/propagate-approximated-shorter-to-static-no-bound.rs:45:1 +error: free region `'_#1r` does not outlive free region `ReStatic` + --> $DIR/propagate-approximated-shorter-to-static-no-bound.rs:45:47 | -45 | / fn supply<'a, 'b>(cell_a: Cell<&'a u32>, cell_b: Cell<&'b u32>) { -46 | | establish_relationships(&cell_a, &cell_b, |_outlives, x, y| { -47 | | // Only works if 'x: 'y: -48 | | demand_y(x, y, x.get()) -49 | | }); -50 | | } +45 | establish_relationships(&cell_a, &cell_b, |_outlives, x, y| { + | _______________________________________________^ +46 | | //~^ ERROR free region `'_#1r` does not outlive free region `ReStatic` +47 | | +48 | | // Only works if 'x: 'y: +49 | | demand_y(x, y, x.get()) //~ WARNING not reporting region error due to -Znll +50 | | }); + | |_____^ + +note: No external requirements + --> $DIR/propagate-approximated-shorter-to-static-no-bound.rs:44:1 + | +44 | / fn supply<'a, 'b>(cell_a: Cell<&'a u32>, cell_b: Cell<&'b u32>) { +45 | | establish_relationships(&cell_a, &cell_b, |_outlives, x, y| { +46 | | //~^ ERROR free region `'_#1r` does not outlive free region `ReStatic` +47 | | +... | +50 | | }); +51 | | } | |_^ | = note: defining type: DefId(0/0:6 ~ propagate_approximated_shorter_to_static_no_bound[317d]::supply[0]) with substs [] +error: aborting due to previous error + diff --git a/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-wrong-bound.rs b/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-wrong-bound.rs index f4fdd4a8c17..68d51e2b7d1 100644 --- a/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-wrong-bound.rs +++ b/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-wrong-bound.rs @@ -18,7 +18,6 @@ // these errors are not (yet) reported. // compile-flags:-Znll -Zborrowck=mir -Zverbose -// must-compile-successfully #![feature(rustc_attrs)] @@ -47,8 +46,10 @@ fn demand_y<'x, 'y>(_cell_x: &Cell<&'x u32>, _cell_y: &Cell<&'y u32>, _y: &'y u3 #[rustc_regions] fn supply<'a, 'b>(cell_a: Cell<&'a u32>, cell_b: Cell<&'b u32>) { establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y| { + //~^ ERROR free region `'_#1r` does not outlive free region `ReStatic` // Only works if 'x: 'y: demand_y(x, y, x.get()) + //~^ WARNING not reporting region error due to -Znll }); } diff --git a/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-wrong-bound.stderr b/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-wrong-bound.stderr index f189967244a..adc6b1ac595 100644 --- a/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-wrong-bound.stderr +++ b/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-wrong-bound.stderr @@ -5,13 +5,15 @@ warning: not reporting region error due to -Znll | ^^^^^^^^^^^^^^^^^^^^^^^ note: External requirements - --> $DIR/propagate-approximated-shorter-to-static-wrong-bound.rs:49:47 + --> $DIR/propagate-approximated-shorter-to-static-wrong-bound.rs:48:47 | -49 | establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y| { +48 | establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y| { | _______________________________________________^ +49 | | //~^ ERROR free region `'_#1r` does not outlive free region `ReStatic` 50 | | // Only works if 'x: 'y: 51 | | demand_y(x, y, x.get()) -52 | | }); +52 | | //~^ WARNING not reporting region error due to -Znll +53 | | }); | |_____^ | = note: defining type: DefId(0/1:18 ~ propagate_approximated_shorter_to_static_wrong_bound[317d]::supply[0]::{{closure}}[0]) with closure substs [ @@ -21,16 +23,31 @@ note: External requirements = note: number of external vids: 3 = note: where '_#1r: '_#0r -note: No external requirements - --> $DIR/propagate-approximated-shorter-to-static-wrong-bound.rs:48:1 +error: free region `'_#1r` does not outlive free region `ReStatic` + --> $DIR/propagate-approximated-shorter-to-static-wrong-bound.rs:48:47 | -48 | / fn supply<'a, 'b>(cell_a: Cell<&'a u32>, cell_b: Cell<&'b u32>) { -49 | | establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y| { +48 | establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y| { + | _______________________________________________^ +49 | | //~^ ERROR free region `'_#1r` does not outlive free region `ReStatic` 50 | | // Only works if 'x: 'y: 51 | | demand_y(x, y, x.get()) -52 | | }); -53 | | } +52 | | //~^ WARNING not reporting region error due to -Znll +53 | | }); + | |_____^ + +note: No external requirements + --> $DIR/propagate-approximated-shorter-to-static-wrong-bound.rs:47:1 + | +47 | / fn supply<'a, 'b>(cell_a: Cell<&'a u32>, cell_b: Cell<&'b u32>) { +48 | | establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y| { +49 | | //~^ ERROR free region `'_#1r` does not outlive free region `ReStatic` +50 | | // Only works if 'x: 'y: +... | +53 | | }); +54 | | } | |_^ | = note: defining type: DefId(0/0:6 ~ propagate_approximated_shorter_to_static_wrong_bound[317d]::supply[0]) with substs [] +error: aborting due to previous error + diff --git a/src/test/ui/nll/closure-requirements/propagate-approximated-val.rs b/src/test/ui/nll/closure-requirements/propagate-approximated-val.rs index 97a15a36fdd..b4a759d5e70 100644 --- a/src/test/ui/nll/closure-requirements/propagate-approximated-val.rs +++ b/src/test/ui/nll/closure-requirements/propagate-approximated-val.rs @@ -16,7 +16,6 @@ // anonymous regions as well. // compile-flags:-Znll -Zborrowck=mir -Zverbose -// must-compile-successfully #![feature(rustc_attrs)] @@ -45,8 +44,10 @@ fn demand_y<'x, 'y>(_outlives1: Cell<&&'x u32>, _outlives2: Cell<&'y &u32>, _y: #[rustc_regions] fn test<'a, 'b>(cell_a: Cell<&'a u32>, cell_b: Cell<&'b u32>) { establish_relationships(cell_a, cell_b, |outlives1, outlives2, x, y| { + //~^ ERROR free region `'_#1r` does not outlive free region `'_#2r` + // Only works if 'x: 'y: - demand_y(outlives1, outlives2, x.get()) + demand_y(outlives1, outlives2, x.get()) //~ WARNING not reporting region error due to -Znll }); } diff --git a/src/test/ui/nll/closure-requirements/propagate-approximated-val.stderr b/src/test/ui/nll/closure-requirements/propagate-approximated-val.stderr index 1577e34e0de..43464bfb2b9 100644 --- a/src/test/ui/nll/closure-requirements/propagate-approximated-val.stderr +++ b/src/test/ui/nll/closure-requirements/propagate-approximated-val.stderr @@ -1,17 +1,19 @@ warning: not reporting region error due to -Znll - --> $DIR/propagate-approximated-val.rs:49:9 + --> $DIR/propagate-approximated-val.rs:50:9 | -49 | demand_y(outlives1, outlives2, x.get()) +50 | demand_y(outlives1, outlives2, x.get()) //~ WARNING not reporting region error due to -Znll | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: External requirements - --> $DIR/propagate-approximated-val.rs:47:45 + --> $DIR/propagate-approximated-val.rs:46:45 | -47 | establish_relationships(cell_a, cell_b, |outlives1, outlives2, x, y| { +46 | establish_relationships(cell_a, cell_b, |outlives1, outlives2, x, y| { | _____________________________________________^ -48 | | // Only works if 'x: 'y: -49 | | demand_y(outlives1, outlives2, x.get()) -50 | | }); +47 | | //~^ ERROR free region `'_#1r` does not outlive free region `'_#2r` +48 | | +49 | | // Only works if 'x: 'y: +50 | | demand_y(outlives1, outlives2, x.get()) //~ WARNING not reporting region error due to -Znll +51 | | }); | |_____^ | = note: defining type: DefId(0/1:18 ~ propagate_approximated_val[317d]::test[0]::{{closure}}[0]) with closure substs [ @@ -21,16 +23,25 @@ note: External requirements = note: number of external vids: 3 = note: where '_#1r: '_#2r -note: No external requirements - --> $DIR/propagate-approximated-val.rs:46:1 +error: free region `'_#1r` does not outlive free region `'_#2r` + --> $DIR/propagate-approximated-val.rs:46:37 | -46 | / fn test<'a, 'b>(cell_a: Cell<&'a u32>, cell_b: Cell<&'b u32>) { -47 | | establish_relationships(cell_a, cell_b, |outlives1, outlives2, x, y| { -48 | | // Only works if 'x: 'y: -49 | | demand_y(outlives1, outlives2, x.get()) -50 | | }); -51 | | } +46 | establish_relationships(cell_a, cell_b, |outlives1, outlives2, x, y| { + | ^^^^^^ + +note: No external requirements + --> $DIR/propagate-approximated-val.rs:45:1 + | +45 | / fn test<'a, 'b>(cell_a: Cell<&'a u32>, cell_b: Cell<&'b u32>) { +46 | | establish_relationships(cell_a, cell_b, |outlives1, outlives2, x, y| { +47 | | //~^ ERROR free region `'_#1r` does not outlive free region `'_#2r` +48 | | +... | +51 | | }); +52 | | } | |_^ | = note: defining type: DefId(0/0:6 ~ propagate_approximated_val[317d]::test[0]) with substs [] +error: aborting due to previous error + From 77663a677da13757e7aa0b4d1a2bc77612000ab9 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 1 Dec 2017 11:07:53 -0500 Subject: [PATCH 16/22] refactor region value bitmatrix --- .../borrow_check/nll/region_infer/dump_mir.rs | 2 +- .../borrow_check/nll/region_infer/mod.rs | 143 +++-------- .../borrow_check/nll/region_infer/values.rs | 227 ++++++++++++++++++ src/test/mir-opt/nll/named-lifetimes-basic.rs | 8 +- 4 files changed, 270 insertions(+), 110 deletions(-) create mode 100644 src/librustc_mir/borrow_check/nll/region_infer/values.rs diff --git a/src/librustc_mir/borrow_check/nll/region_infer/dump_mir.rs b/src/librustc_mir/borrow_check/nll/region_infer/dump_mir.rs index 5477308bde9..69ecafa66ae 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/dump_mir.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/dump_mir.rs @@ -70,7 +70,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { with_msg: &mut FnMut(&str) -> io::Result<()>, ) -> io::Result<()> { for region in self.definitions.indices() { - let value = self.region_value_str_from_matrix(&self.liveness_constraints, region); + let value = self.liveness_constraints.region_value_str(region); if value != "{}" { with_msg(&format!("{:?} live at {}", region, value))?; } diff --git a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs index b2e2ccc5d0b..047b78dd55c 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs @@ -19,15 +19,15 @@ use rustc::mir::{ClosureOutlivesRequirement, ClosureRegionRequirements, Location use rustc::ty::{self, RegionVid}; use rustc_data_structures::indexed_vec::IndexVec; use rustc_data_structures::fx::FxHashSet; -use rustc_data_structures::bitvec::BitMatrix; -use rustc_data_structures::indexed_vec::Idx; -use std::collections::BTreeMap; use std::fmt; +use std::rc::Rc; use syntax_pos::Span; mod annotation; mod dump_mir; mod graphviz; +mod values; +use self::values::{RegionValueElements, RegionValues}; pub struct RegionInferenceContext<'tcx> { /// Contains the definition for every region variable. Region @@ -36,27 +36,22 @@ pub struct RegionInferenceContext<'tcx> { /// from as well as its final inferred value. definitions: IndexVec>, + /// Maps from points/universal-regions to a `RegionElementIndex`. + elements: Rc, + /// The liveness constraints added to each region. For most /// regions, these start out empty and steadily grow, though for /// each universally quantified region R they start out containing /// the entire CFG and `end(R)`. - /// - /// In this `BitMatrix` representation, the rows are the region - /// variables and the columns are the free regions and MIR locations. - liveness_constraints: BitMatrix, + liveness_constraints: RegionValues, /// The final inferred values of the inference variables; `None` /// until `solve` is invoked. - inferred_values: Option, + inferred_values: Option, /// The constraints we have accumulated and used during solving. constraints: Vec, - /// A map from each MIR Location to its column index in - /// `liveness_constraints`/`inferred_values`. (The first N columns are - /// the free regions.) - point_indices: BTreeMap, - /// Information about the universally quantified regions in scope /// on this function and their (known) relations to one another. universal_regions: UniversalRegions<'tcx>, @@ -112,19 +107,16 @@ impl<'tcx> RegionInferenceContext<'tcx> { let num_region_variables = var_origins.len(); let num_universal_regions = universal_regions.len(); - let mut num_points = 0; - let mut point_indices = BTreeMap::new(); - + let mut points = Vec::new(); for (block, block_data) in mir.basic_blocks().iter_enumerated() { for statement_index in 0..block_data.statements.len() + 1 { - let location = Location { + points.push(Location { block, statement_index, - }; - point_indices.insert(location, num_universal_regions + num_points); - num_points += 1; + }); } } + let elements = &Rc::new(RegionValueElements::new(points, num_universal_regions)); // Create a RegionDefinition for each inference variable. let definitions = var_origins @@ -134,13 +126,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { let mut result = Self { definitions, - liveness_constraints: BitMatrix::new( - num_region_variables, - num_universal_regions + num_points, - ), + elements: elements.clone(), + liveness_constraints: RegionValues::new(elements, num_region_variables), inferred_values: None, constraints: Vec::new(), - point_indices, universal_regions, }; @@ -186,14 +175,12 @@ impl<'tcx> RegionInferenceContext<'tcx> { self.definitions[variable].is_universal = true; // Add all nodes in the CFG to liveness constraints - for (_location, point_index) in &self.point_indices { - self.liveness_constraints - .add(variable.index(), *point_index); + for point_index in self.elements.all_point_indices() { + self.liveness_constraints.add(variable, point_index); } // Add `end(X)` into the set for X. - self.liveness_constraints - .add(variable.index(), variable.index()); + self.liveness_constraints.add(variable, variable); } } @@ -217,32 +204,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { let inferred_values = self.inferred_values .as_ref() .expect("region values not yet inferred"); - self.region_contains_point_in_matrix(inferred_values, r, p) - } - - /// True if given region `r` contains the point `p`, when - /// evaluated in the set of region values `matrix`. - fn region_contains_point_in_matrix( - &self, - matrix: &BitMatrix, - r: RegionVid, - p: Location, - ) -> bool { - let point_index = self.point_indices - .get(&p) - .expect("point index should be known"); - matrix.contains(r.index(), *point_index) - } - - /// True if given region `r` contains the `end(s)`, when - /// evaluated in the set of region values `matrix`. - fn region_contains_region_in_matrix( - &self, - matrix: &BitMatrix, - r: RegionVid, - s: RegionVid, - ) -> bool { - matrix.contains(r.index(), s.index()) + inferred_values.contains(r, p) } /// Returns access to the value of `r` for debugging purposes. @@ -251,43 +213,21 @@ impl<'tcx> RegionInferenceContext<'tcx> { .as_ref() .expect("region values not yet inferred"); - self.region_value_str_from_matrix(inferred_values, r) - } - - fn region_value_str_from_matrix(&self, - matrix: &BitMatrix, - r: RegionVid) -> String { - let mut result = String::new(); - result.push_str("{"); - let mut sep = ""; - - for &point in self.point_indices.keys() { - if self.region_contains_point_in_matrix(matrix, r, point) { - result.push_str(&format!("{}{:?}", sep, point)); - sep = ", "; - } - } - - for fr in (0..self.universal_regions.len()).map(RegionVid::new) { - if self.region_contains_region_in_matrix(matrix, r, fr) { - result.push_str(&format!("{}{:?}", sep, fr)); - sep = ", "; - } - } - - result.push_str("}"); - - result + inferred_values.region_value_str(r) } /// Indicates that the region variable `v` is live at the point `point`. pub(super) fn add_live_point(&mut self, v: RegionVid, point: Location) -> bool { debug!("add_live_point({:?}, {:?})", v, point); assert!(self.inferred_values.is_none(), "values already inferred"); - let point_index = self.point_indices - .get(&point) - .expect("point index should be known"); - self.liveness_constraints.add(v.index(), *point_index) + debug!("add_live_point: @{:?}", point); + + let element = self.elements.index(point); + if self.liveness_constraints.add(v, element) { + true + } else { + false + } } /// Indicates that the region variable `sup` must outlive `sub` is live at the point `point`. @@ -386,16 +326,12 @@ impl<'tcx> RegionInferenceContext<'tcx> { outlives_requirements: &mut Vec, ) { let inferred_values = self.inferred_values.as_ref().unwrap(); - let longer_value = inferred_values.iter(longer_fr.index()); debug!("check_universal_region(fr={:?})", longer_fr); // Find every region `o` such that `fr: o` // (because `fr` includes `end(o)`). - let shorter_frs = longer_value - .take_while(|&i| i < self.universal_regions.len()) - .map(RegionVid::new); - for shorter_fr in shorter_frs { + for shorter_fr in inferred_values.universal_regions_outlived_by(longer_fr) { // If it is known that `fr: o`, carry on. if self.universal_regions.outlives(longer_fr, shorter_fr) { continue; @@ -512,20 +448,22 @@ impl<'tcx> RegionInferenceContext<'tcx> { fn copy( &self, - inferred_values: &mut BitMatrix, + inferred_values: &mut RegionValues, mir: &Mir<'tcx>, from_region: RegionVid, to_region: RegionVid, - start_point: Location, + constraint_point: Location, ) -> bool { let mut changed = false; let mut stack = vec![]; let mut visited = FxHashSet(); - stack.push(start_point); + stack.push(constraint_point); while let Some(p) = stack.pop() { - if !self.region_contains_point_in_matrix(inferred_values, from_region, p) { + let point_index = self.elements.index(p); + + if !inferred_values.contains(from_region, point_index) { debug!(" not in from-region"); continue; } @@ -535,8 +473,8 @@ impl<'tcx> RegionInferenceContext<'tcx> { continue; } - let point_index = self.point_indices.get(&p).unwrap(); - changed |= inferred_values.add(to_region.index(), *point_index); + let new = inferred_values.add(to_region, point_index); + changed |= new; let block_data = &mir[p.block]; let successor_points = if p.statement_index < block_data.statements.len() { @@ -564,13 +502,8 @@ impl<'tcx> RegionInferenceContext<'tcx> { // If we reach the END point in the graph, then copy // over any skolemized end points in the `from_region` // and make sure they are included in the `to_region`. - let universal_region_indices = inferred_values - .iter(from_region.index()) - .take_while(|&i| i < self.universal_regions.len()) - .collect::>(); - for fr in &universal_region_indices { - changed |= inferred_values.add(to_region.index(), *fr); - } + changed |= + inferred_values.add_universal_regions_outlived_by(from_region, to_region); } else { stack.extend(successor_points); } diff --git a/src/librustc_mir/borrow_check/nll/region_infer/values.rs b/src/librustc_mir/borrow_check/nll/region_infer/values.rs new file mode 100644 index 00000000000..cda199859e4 --- /dev/null +++ b/src/librustc_mir/borrow_check/nll/region_infer/values.rs @@ -0,0 +1,227 @@ +use std::rc::Rc; +use rustc_data_structures::bitvec::BitMatrix; +use rustc_data_structures::indexed_vec::Idx; +use rustc::mir::Location; +use rustc::ty::RegionVid; + +/// Maps between the various kinds of elements of a region value to +/// the internal indices that w use. +pub(super) struct RegionValueElements { + points: Vec, + num_universal_regions: usize, +} + +impl RegionValueElements { + pub(super) fn new(points: Vec, num_universal_regions: usize) -> Self { + Self { + points, + num_universal_regions, + } + } + + /// Converts an element of a region value into a `RegionElementIndex`. + pub(super) fn index(&self, elem: T) -> RegionElementIndex { + elem.to_element_index(self) + } + + /// Iterates over the `RegionElementIndex` for all points in the CFG. + pub(super) fn all_point_indices<'a>(&'a self) -> impl Iterator + 'a { + (0..self.points.len()).map(move |i| RegionElementIndex::new(i + self.num_universal_regions)) + } + + /// Iterates over the `RegionElementIndex` for all points in the CFG. + pub(super) fn all_universal_region_indices(&self) -> impl Iterator { + (0..self.num_universal_regions).map(move |i| RegionElementIndex::new(i)) + } + + /// Converts a particular `RegionElementIndex` to the `RegionElement` it represents. + pub(super) fn to_element(&self, i: RegionElementIndex) -> RegionElement { + if let Some(r) = self.to_universal_region(i) { + RegionElement::UniversalRegion(r) + } else { + RegionElement::Location(self.points[i.index() - self.num_universal_regions]) + } + } + + /// Converts a particular `RegionElementIndex` to a universal + /// region, if that is what it represents. Returns `None` + /// otherwise. + pub(super) fn to_universal_region(&self, i: RegionElementIndex) -> Option { + if i.index() < self.num_universal_regions { + Some(RegionVid::new(i.index())) + } else { + None + } + } +} + +/// A newtype for the integers that represent one of the possible +/// elements in a region. These are the rows in the `BitMatrix` that +/// is used to store the values of all regions. They have the following +/// convention: +/// +/// - The first N indices represent free regions (where N = universal_regions.len()). +/// - The remainder represent the points in the CFG (see `point_indices` map). +/// +/// You can convert a `RegionElementIndex` into a `RegionElement` +/// using the `to_region_elem` method. +newtype_index!(RegionElementIndex { DEBUG_FORMAT = "RegionElementIndex({})" }); + +/// An individual element in a region value -- the value of a +/// particular region variable consists of a set of these elements. +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] +pub(super) enum RegionElement { + /// A point in the control-flow graph. + Location(Location), + + /// An in-scope, universally quantified region (e.g., a liftime parameter). + UniversalRegion(RegionVid), +} + + +pub(super) trait ToElementIndex { + fn to_element_index(self, elements: &RegionValueElements) -> RegionElementIndex; +} + +impl ToElementIndex for Location { + fn to_element_index(self, elements: &RegionValueElements) -> RegionElementIndex { + let index = elements.points.binary_search(&self).unwrap(); + RegionElementIndex::new(index + elements.num_universal_regions) + } +} + +impl ToElementIndex for RegionVid { + fn to_element_index(self, elements: &RegionValueElements) -> RegionElementIndex { + assert!(self.index() < elements.num_universal_regions); + RegionElementIndex::new(self.index()) + } +} + +impl ToElementIndex for RegionElementIndex { + fn to_element_index(self, _elements: &RegionValueElements) -> RegionElementIndex { + self + } +} + +/// Stores the values for a set of regions. These are stored in a +/// compact `BitMatrix` representation, with one row per region +/// variable. The columns consist of either universal regions or +/// points in the CFG. +#[derive(Clone)] +pub(super) struct RegionValues { + elements: Rc, + matrix: BitMatrix, +} + +impl RegionValues { + pub(super) fn new(elements: &Rc, num_region_variables: usize) -> Self { + assert!( + elements.num_universal_regions <= num_region_variables, + "universal regions are a subset of the region variables" + ); + + let num_columns = elements.points.len() + elements.num_universal_regions; + + Self { + elements: elements.clone(), + matrix: BitMatrix::new(num_region_variables, num_columns), + } + } + + /// Adds the given element to the value for the given region. Returns true if + /// the element is newly added (i.e., was not already present). + pub(super) fn add(&mut self, r: RegionVid, elem: E) -> bool { + let i = self.elements.index(elem); + if self.matrix.add(r.index(), i.index()) { + debug!("add(r={:?}, i={:?})", r, self.elements.to_element(i)); + true + } else { + false + } + } + + /// Adds all the universal regions outlived by `from_region` to + /// `to_region`. + pub(super) fn add_universal_regions_outlived_by( + &mut self, + from_region: RegionVid, + to_region: RegionVid, + ) -> bool { + // FIXME. We could optimize this by improving + // `BitMatrix::merge` so it does not always merge an entire + // row. + debug!("add_universal_regions_outlived_by(from_region={:?}, to_region={:?})", + from_region, to_region); + let mut changed = false; + for elem in self.elements.all_universal_region_indices() { + if self.contains(from_region, elem) { + changed |= self.add(to_region, elem); + } + } + changed + } + + /// True if the region `r` contains the given element. + pub(super) fn contains(&self, r: RegionVid, elem: E) -> bool { + let i = self.elements.index(elem); + self.matrix.contains(r.index(), i.index()) + } + + /// Iterate over the value of the region `r`, yielding up element + /// indices. You may prefer `universal_regions_outlived_by` or + /// `elements_contained_in`. + pub(super) fn element_indices_contained_in<'a>( + &'a self, + r: RegionVid, + ) -> impl Iterator + 'a { + self.matrix + .iter(r.index()) + .map(move |i| RegionElementIndex::new(i)) + } + + /// Returns just the universal regions that are contained in a given region's value. + pub(super) fn universal_regions_outlived_by<'a>( + &'a self, + r: RegionVid, + ) -> impl Iterator + 'a { + self.element_indices_contained_in(r) + .map(move |i| self.elements.to_universal_region(i)) + .take_while(move |v| v.is_some()) // universal regions are a prefix + .map(move |v| v.unwrap()) + } + + /// Returns all the elements contained in a given region's value. + pub(super) fn elements_contained_in<'a>( + &'a self, + r: RegionVid, + ) -> impl Iterator + 'a { + self.element_indices_contained_in(r) + .map(move |r| self.elements.to_element(r)) + } + + /// Returns a "pretty" string value of the region. Meant for debugging. + pub(super) fn region_value_str(&self, r: RegionVid) -> String { + let mut result = String::new(); + result.push_str("{"); + + for (index, element) in self.elements_contained_in(r).enumerate() { + if index > 0 { + result.push_str(", "); + } + + match element { + RegionElement::Location(l) => { + result.push_str(&format!("{:?}", l)); + } + + RegionElement::UniversalRegion(fr) => { + result.push_str(&format!("{:?}", fr)); + } + } + } + + result.push_str("}"); + + result + } +} diff --git a/src/test/mir-opt/nll/named-lifetimes-basic.rs b/src/test/mir-opt/nll/named-lifetimes-basic.rs index 0c42585a528..f3a57c08840 100644 --- a/src/test/mir-opt/nll/named-lifetimes-basic.rs +++ b/src/test/mir-opt/nll/named-lifetimes-basic.rs @@ -33,10 +33,10 @@ fn main() { // | '_#3r | Local | ['_#3r] // | // | Inferred Region Values -// | '_#0r | {bb0[0], bb0[1], '_#0r} -// | '_#1r | {bb0[0], bb0[1], '_#1r} -// | '_#2r | {bb0[0], bb0[1], '_#2r} -// | '_#3r | {bb0[0], bb0[1], '_#3r} +// | '_#0r | {'_#0r, bb0[0], bb0[1]} +// | '_#1r | {'_#1r, bb0[0], bb0[1]} +// | '_#2r | {'_#2r, bb0[0], bb0[1]} +// | '_#3r | {'_#3r, bb0[0], bb0[1]} // | // ... // fn use_x(_1: &'_#1r mut i32, _2: &'_#2r u32, _3: &'_#1r u32, _4: &'_#3r u32) -> bool { From 7a20a3f1619db092ac935a247ff06c6e03f20255 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 1 Dec 2017 14:20:48 -0500 Subject: [PATCH 17/22] change to use an O(1) data structure for looking up point indices Converting a `RegionElementIndex` to a `Location` is O(n) though could trivially be O(log n), but we don't do it that much anyhow -- just on error and debugging. --- .../borrow_check/nll/region_infer/mod.rs | 11 +-- .../borrow_check/nll/region_infer/values.rs | 87 ++++++++++++++++--- 2 files changed, 77 insertions(+), 21 deletions(-) diff --git a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs index 047b78dd55c..c1518b28dab 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs @@ -107,16 +107,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { let num_region_variables = var_origins.len(); let num_universal_regions = universal_regions.len(); - let mut points = Vec::new(); - for (block, block_data) in mir.basic_blocks().iter_enumerated() { - for statement_index in 0..block_data.statements.len() + 1 { - points.push(Location { - block, - statement_index, - }); - } - } - let elements = &Rc::new(RegionValueElements::new(points, num_universal_regions)); + let elements = &Rc::new(RegionValueElements::new(mir, num_universal_regions)); // Create a RegionDefinition for each inference variable. let definitions = var_origins diff --git a/src/librustc_mir/borrow_check/nll/region_infer/values.rs b/src/librustc_mir/borrow_check/nll/region_infer/values.rs index cda199859e4..849ccd3259a 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/values.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/values.rs @@ -1,24 +1,58 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + use std::rc::Rc; use rustc_data_structures::bitvec::BitMatrix; use rustc_data_structures::indexed_vec::Idx; -use rustc::mir::Location; +use rustc_data_structures::indexed_vec::IndexVec; +use rustc::mir::{BasicBlock, Location, Mir}; use rustc::ty::RegionVid; /// Maps between the various kinds of elements of a region value to /// the internal indices that w use. pub(super) struct RegionValueElements { - points: Vec, + /// For each basic block, how many points are contained within? + statements_before_block: IndexVec, + num_points: usize, num_universal_regions: usize, } impl RegionValueElements { - pub(super) fn new(points: Vec, num_universal_regions: usize) -> Self { + pub(super) fn new(mir: &Mir<'_>, num_universal_regions: usize) -> Self { + let mut num_points = 0; + let statements_before_block = + mir.basic_blocks() + .iter() + .map(|block_data| { + let v = num_points; + num_points += block_data.statements.len() + 1; + v + }) + .collect(); + + debug!("RegionValueElements(num_universal_regions={:?})", num_universal_regions); + debug!("RegionValueElements: statements_before_block={:#?}", statements_before_block); + debug!("RegionValueElements: num_points={:#?}", num_points); + Self { - points, + statements_before_block, num_universal_regions, + num_points, } } + /// Total number of element indices that exist. + pub(super) fn num_elements(&self) -> usize { + self.num_points + self.num_universal_regions + } + /// Converts an element of a region value into a `RegionElementIndex`. pub(super) fn index(&self, elem: T) -> RegionElementIndex { elem.to_element_index(self) @@ -26,7 +60,7 @@ impl RegionValueElements { /// Iterates over the `RegionElementIndex` for all points in the CFG. pub(super) fn all_point_indices<'a>(&'a self) -> impl Iterator + 'a { - (0..self.points.len()).map(move |i| RegionElementIndex::new(i + self.num_universal_regions)) + (0..self.num_points).map(move |i| RegionElementIndex::new(i + self.num_universal_regions)) } /// Iterates over the `RegionElementIndex` for all points in the CFG. @@ -36,10 +70,42 @@ impl RegionValueElements { /// Converts a particular `RegionElementIndex` to the `RegionElement` it represents. pub(super) fn to_element(&self, i: RegionElementIndex) -> RegionElement { + debug!("to_element(i={:?})", i); + if let Some(r) = self.to_universal_region(i) { RegionElement::UniversalRegion(r) } else { - RegionElement::Location(self.points[i.index() - self.num_universal_regions]) + let point_index = i.index() - self.num_universal_regions; + + // Find the basic block. We have a vector with the + // starting index of the statement in each block. Imagine + // we have statement #22, and we have a vector like: + // + // [0, 10, 20] + // + // In that case, this represents point_index 2 of + // basic block BB2. We know this because BB0 accounts for + // 0..10, BB1 accounts for 11..20, and BB2 accounts for + // 20... + // + // To compute this, we could do a binary search, but + // because I am lazy we instead iterate through to find + // the last point where the "first index" (0, 10, or 20) + // was less than the statement index (22). In our case, this will + // be (BB2, 20). + // + // Nit: we could do a binary search here but I'm too lazy. + let (block, &first_index) = + self.statements_before_block + .iter_enumerated() + .filter(|(_, first_index)| **first_index <= point_index) + .last() + .unwrap(); + + RegionElement::Location(Location { + block, + statement_index: point_index - first_index, + }) } } @@ -85,8 +151,9 @@ pub(super) trait ToElementIndex { impl ToElementIndex for Location { fn to_element_index(self, elements: &RegionValueElements) -> RegionElementIndex { - let index = elements.points.binary_search(&self).unwrap(); - RegionElementIndex::new(index + elements.num_universal_regions) + let Location { block, statement_index } = self; + let start_index = elements.statements_before_block[block]; + RegionElementIndex::new(elements.num_universal_regions + start_index + statement_index) } } @@ -120,11 +187,9 @@ impl RegionValues { "universal regions are a subset of the region variables" ); - let num_columns = elements.points.len() + elements.num_universal_regions; - Self { elements: elements.clone(), - matrix: BitMatrix::new(num_region_variables, num_columns), + matrix: BitMatrix::new(num_region_variables, elements.num_elements()), } } From f6723a9592e69419b1325faf5ffbbad2f8a6291a Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 12 Dec 2017 14:21:10 -0500 Subject: [PATCH 18/22] improve comments on `safe_to_unsafe_fn_ty` and `coerce_closure_fn_ty` --- src/librustc/ty/context.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 441dd792ee1..1d538e1c167 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -1708,7 +1708,9 @@ slice_interners!( ); impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { - /// Create an unsafe fn ty based on a safe fn ty. + /// Given a `fn` type, returns an equivalent `unsafe fn` type; + /// that is, a `fn` type that is equivalent in every way for being + /// unsafe. pub fn safe_to_unsafe_fn_ty(self, sig: PolyFnSig<'tcx>) -> Ty<'tcx> { assert_eq!(sig.unsafety(), hir::Unsafety::Normal); self.mk_fn_ptr(sig.map_bound(|sig| ty::FnSig { @@ -1717,7 +1719,10 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { })) } - /// Create an unsafe fn ty based on a safe fn ty. + /// Given a closure signature `sig`, returns an equivalent `fn` + /// type with the same signature. Detuples and so forth -- so + /// e.g. if we have a sig with `Fn<(u32, i32)>` then you would get + /// a `fn(u32, i32)`. pub fn coerce_closure_fn_ty(self, sig: PolyFnSig<'tcx>) -> Ty<'tcx> { let converted_sig = sig.map_bound(|s| { let params_iter = match s.inputs()[0].sty { @@ -1731,8 +1736,8 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { s.output(), s.variadic, hir::Unsafety::Normal, - abi::Abi::Rust - ) + abi::Abi::Rust, + ) }); self.mk_fn_ptr(converted_sig) From 75ac071cd6e14c887fbb7526da8958b53aaf7198 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 12 Dec 2017 14:21:37 -0500 Subject: [PATCH 19/22] document return value of `add_live_point` --- src/librustc_mir/borrow_check/nll/region_infer/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs index c1518b28dab..c926c7432bb 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs @@ -208,6 +208,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { } /// Indicates that the region variable `v` is live at the point `point`. + /// + /// Returns `true` if this constraint is new and `false` is the + /// constraint was already present. pub(super) fn add_live_point(&mut self, v: RegionVid, point: Location) -> bool { debug!("add_live_point({:?}, {:?})", v, point); assert!(self.inferred_values.is_none(), "values already inferred"); From abd6d0d76ec279378fa1ebf92c7c6b5dc6c28906 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 12 Dec 2017 19:46:36 -0500 Subject: [PATCH 20/22] comments for `defining_ty` and `compute_indices` Plus an extra assertion. --- .../borrow_check/nll/universal_regions.rs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/librustc_mir/borrow_check/nll/universal_regions.rs b/src/librustc_mir/borrow_check/nll/universal_regions.rs index c8b5a258a70..5336bd271f5 100644 --- a/src/librustc_mir/borrow_check/nll/universal_regions.rs +++ b/src/librustc_mir/borrow_check/nll/universal_regions.rs @@ -456,6 +456,21 @@ impl<'cx, 'gcx, 'tcx> UniversalRegionsBuilder<'cx, 'gcx, 'tcx> { } } + /// Returns the "defining type" of the current MIR: + /// + /// - for functions, this is the `TyFnDef`; + /// - for closures, this is the `TyClosure`; + /// - for generators, this is the `TyGenerator`; + /// - for constants, this is the type of value that gets produced. + /// - FIXME. Constants are handled somewhat inelegantly; this gets + /// patched in a later PR that has already landed on nll-master. + /// + /// The key feature of the "defining type" is that it contains the + /// information needed to derive all the universal regions that + /// are in scope as well as the types of the inputs/output from + /// the MIR. In general, early-bound universal regions appear free + /// in the defining type and late-bound regions appear bound in + /// the signature. fn defining_ty(&self) -> ty::Ty<'tcx> { let tcx = self.infcx.tcx; let closure_base_def_id = tcx.closure_base_def_id(self.mir_def_id); @@ -471,6 +486,10 @@ impl<'cx, 'gcx, 'tcx> UniversalRegionsBuilder<'cx, 'gcx, 'tcx> { .replace_free_regions_with_nll_infer_vars(FR, &defining_ty) } + /// Builds a hashmap that maps from the universal regions that are + /// in scope (as a `ty::Region<'tcx>`) to their indices (as a + /// `RegionVid`). The map returned by this function contains only + /// the early-bound regions. fn compute_indices( &self, fr_static: RegionVid, @@ -490,6 +509,7 @@ impl<'cx, 'gcx, 'tcx> UniversalRegionsBuilder<'cx, 'gcx, 'tcx> { // that correspond to early-bound regions declared on // the `closure_base_def_id`. assert!(substs.substs.len() >= identity_substs.len()); + assert_eq!(substs.substs.regions().count(), identity_substs.regions().count()); substs.substs } ty::TyFnDef(_, substs) => substs, From 51847a1b185b86974637d6dd08c2c02516271cc3 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 12 Dec 2017 19:48:16 -0500 Subject: [PATCH 21/22] add FIXME related to constant well-formedness --- src/librustc_mir/transform/type_check.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index 0fa24b3478d..fae911780a1 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -175,6 +175,14 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> { let expected_ty = match constant.literal { Literal::Value { value } => { + // FIXME(#46702) -- We need some way to get the predicates + // associated with the "pre-evaluated" form of the + // constant. For example, consider that the constant + // may have associated constant projections (`>::SOME_CONST`) that impose + // constraints on `'a` and `'b`. These constraints + // would be lost if we just look at the normalized + // value. if let ConstVal::Function(def_id, ..) = value.val { let tcx = self.tcx(); let type_checker = &mut self.cx; From 237dd4121157943ed54a2e3f75b5849fadd8c7b4 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 12 Dec 2017 19:48:29 -0500 Subject: [PATCH 22/22] correct comment in test --- src/test/compile-fail/mir_check_cast_reify.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/compile-fail/mir_check_cast_reify.rs b/src/test/compile-fail/mir_check_cast_reify.rs index a253243bad7..091e0b71b2d 100644 --- a/src/test/compile-fail/mir_check_cast_reify.rs +++ b/src/test/compile-fail/mir_check_cast_reify.rs @@ -33,7 +33,7 @@ fn bar<'a>(x: &'a u32) -> &'static u32 { // During NLL region analysis, this will get renumbered to `typeof(foo::<'?0>)` // where `'?0` is a new region variable. // - // (Note that if `'a` on `foo` were early-bound, the type would be + // (Note that if `'a` on `foo` were late-bound, the type would be // `typeof(foo)`, which would interact differently with because // the renumbering later.) //