From 99674dc52b45a22b49f13242be9d931009b4f276 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 24 Jul 2012 16:23:23 -0700 Subject: [PATCH] avoid capture of bound regions when infering types for closure expressions. cc #2981 --- src/rustc/metadata/tyencode.rs | 6 ++++ src/rustc/middle/ty.rs | 26 +++++++++++--- src/rustc/middle/typeck/check.rs | 36 +++++++++++++++---- src/rustc/util/ppaux.rs | 21 ++++++++--- src/test/compile-fail/borrowck-call-sendfn.rs | 8 +++++ src/test/compile-fail/borrowck-lend-args.rs | 2 +- .../compile-fail/borrowck-mut-deref-comp.rs | 2 +- .../borrowck-pure-scope-in-call.rs | 2 +- .../compile-fail/borrowck-uniq-via-box.rs | 18 +++++----- .../compile-fail/borrowck-uniq-via-ref.rs | 18 +++++----- .../regions-escape-via-iface-or-not.rs | 24 +++++++++++++ .../compile-fail/regions-ret-borrowed-1.rs | 17 +++++++++ src/test/compile-fail/regions-ret-borrowed.rs | 20 +++++++++++ src/test/run-pass/borrowck-mut-uniq.rs | 26 ++++++++++++++ 14 files changed, 189 insertions(+), 37 deletions(-) create mode 100644 src/test/compile-fail/borrowck-call-sendfn.rs create mode 100644 src/test/compile-fail/regions-escape-via-iface-or-not.rs create mode 100644 src/test/compile-fail/regions-ret-borrowed-1.rs create mode 100644 src/test/compile-fail/regions-ret-borrowed.rs create mode 100644 src/test/run-pass/borrowck-mut-uniq.rs diff --git a/src/rustc/metadata/tyencode.rs b/src/rustc/metadata/tyencode.rs index 9547db818dbb..1471bbd1acbe 100644 --- a/src/rustc/metadata/tyencode.rs +++ b/src/rustc/metadata/tyencode.rs @@ -160,6 +160,12 @@ fn enc_bound_region(w: io::writer, br: ty::bound_region) { w.write_str(*s); w.write_char(']') } + ty::br_cap_avoid(id, br) { + w.write_char('c'); + w.write_int(id); + w.write_char('|'); + enc_bound_region(w, *br); + } } } diff --git a/src/rustc/middle/ty.rs b/src/rustc/middle/ty.rs index af4974347367..bb175be3903b 100644 --- a/src/rustc/middle/ty.rs +++ b/src/rustc/middle/ty.rs @@ -100,7 +100,7 @@ export ty_self, mk_self, type_has_self; export ty_class; export region, bound_region, encl_region; export re_bound, re_free, re_scope, re_static, re_var; -export br_self, br_anon, br_named; +export br_self, br_anon, br_named, br_cap_avoid; export get, type_has_params, type_needs_infer, type_has_regions; export type_has_resources, type_id; export tbox_has_flag; @@ -351,9 +351,24 @@ enum region { } enum bound_region { - br_self, // The self region for classes, impls - br_anon, // The anonymous region parameter for a given function. - br_named(ast::ident) // A named region parameter. + /// The self region for classes, impls (&T in a type defn or &self/T) + br_self, + + /// Anonymous region parameter for a given fn (&T) + br_anon, + + /// Named region parameters for functions (a in &a/T) + br_named(ast::ident), + + /// Handles capture-avoiding substitution in a rather subtle case. If you + /// have a closure whose argument types are being inferred based on the + /// expected type, and the expected type includes bound regions, then we + /// will wrap those bound regions in a br_cap_avoid() with the id of the + /// fn expression. This ensures that the names are not "captured" by the + /// enclosing scope, which may define the same names. For an example of + /// where this comes up, see src/test/compile-fail/regions-ret-borrowed.rs + /// and regions-ret-borrowed-1.rs. + br_cap_avoid(ast::node_id, @bound_region), } type opt_region = option; @@ -1665,7 +1680,7 @@ fn type_kind(cx: ctxt, ty: t) -> kind { ret result; } -// True if instantiating an instance of `ty` requires an instance of `r_ty`. +// True if instantiating an instance of `r_ty` requires an instance of `r_ty`. fn is_instantiable(cx: ctxt, r_ty: t) -> bool { fn type_requires(cx: ctxt, seen: @mut ~[def_id], @@ -2001,6 +2016,7 @@ fn hash_bound_region(br: bound_region) -> uint { ty::br_self { 0u } ty::br_anon { 1u } ty::br_named(str) { str::hash(*str) } + ty::br_cap_avoid(id, br) { id as uint | hash_bound_region(*br) } } } diff --git a/src/rustc/middle/typeck/check.rs b/src/rustc/middle/typeck/check.rs index b5743e525b19..599bfc18b136 100644 --- a/src/rustc/middle/typeck/check.rs +++ b/src/rustc/middle/typeck/check.rs @@ -1084,11 +1084,21 @@ fn check_expr_with_unifier(fcx: @fn_ctxt, expected: option) { let tcx = fcx.ccx.tcx; + // Find the expected input/output types (if any). Careful to + // avoid capture of bound regions in the expected type. See + // def'n of br_cap_avoid() for a more lengthy explanation of + // what's going on here. let expected_tys = do unpack_expected(fcx, expected) |sty| { alt sty { - ty::ty_fn(fn_ty) {some({inputs:fn_ty.inputs, - output:fn_ty.output})} - _ {none} + ty::ty_fn(fn_ty) => { + let {fn_ty, _} = + replace_bound_regions_in_fn_ty( + tcx, @nil, none, fn_ty, + |br| ty::re_bound(ty::br_cap_avoid(expr.id, @br))); + some({inputs:fn_ty.inputs, + output:fn_ty.output}) + } + _ => {none} } }; @@ -1984,15 +1994,26 @@ fn check_const(ccx: @crate_ctxt, _sp: span, e: @ast::expr, id: ast::node_id) { writeback::resolve_type_vars_in_expr(fcx, e); } +/// Checks whether a type can be created without an instance of itself. +/// This is similar but different from the question of whether a type +/// can be represented. For example, the following type: +/// +/// enum foo { none, some(foo) } +/// +/// is instantiable but is not representable. Similarly, the type +/// +/// enum foo { some(@foo) } +/// +/// is representable, but not instantiable. fn check_instantiable(tcx: ty::ctxt, sp: span, item_id: ast::node_id) { - let rty = ty::node_id_to_type(tcx, item_id); - if !ty::is_instantiable(tcx, rty) { + let item_ty = ty::node_id_to_type(tcx, item_id); + if !ty::is_instantiable(tcx, item_ty) { tcx.sess.span_err(sp, #fmt["this type cannot be instantiated \ without an instance of itself; \ consider using `option<%s>`", - ty_to_str(tcx, rty)]); + ty_to_str(tcx, item_ty)]); } } @@ -2063,6 +2084,9 @@ fn check_enum_variants(ccx: @crate_ctxt, } // Check that it is possible to instantiate this enum: + // + // This *sounds* like the same that as representable, but it's + // not. See def'n of `check_instantiable()` for details. check_instantiable(ccx.tcx, sp, id); } diff --git a/src/rustc/util/ppaux.rs b/src/rustc/util/ppaux.rs index 778884ccaf95..b114e659c4c0 100644 --- a/src/rustc/util/ppaux.rs +++ b/src/rustc/util/ppaux.rs @@ -1,6 +1,7 @@ import std::map::hashmap; import middle::ty; -import middle::ty::{arg, bound_region, br_anon, br_named, canon_mode}; +import middle::ty::{arg, canon_mode}; +import middle::ty::{bound_region, br_anon, br_named, br_self, br_cap_avoid}; import middle::ty::{ck_block, ck_box, ck_uniq, ctxt, field, method}; import middle::ty::{mt, re_bound, re_free, re_scope, re_var, region, t}; import middle::ty::{ty_bool, ty_bot, ty_box, ty_class, ty_enum}; @@ -20,10 +21,20 @@ import driver::session::session; fn bound_region_to_str(cx: ctxt, br: bound_region) -> ~str { alt br { - br_anon { ~"&" } - br_named(str) { #fmt["&%s", *str] } - br_self if cx.sess.ppregions() { ~"&" } - br_self { ~"&self" } + br_anon => { ~"&" } + br_named(str) => { #fmt["&%s", *str] } + br_self if cx.sess.ppregions() => { ~"&" } + br_self => { ~"&self" } + + // FIXME(#3011) -- even if this arm is removed, exhaustiveness checking + // does not fail + br_cap_avoid(id, br) => { + if cx.sess.ppregions() { + #fmt["br_cap_avoid(%?, %s)", id, bound_region_to_str(cx, *br)] + } else { + bound_region_to_str(cx, *br) + } + } } } diff --git a/src/test/compile-fail/borrowck-call-sendfn.rs b/src/test/compile-fail/borrowck-call-sendfn.rs new file mode 100644 index 000000000000..fa100d12437c --- /dev/null +++ b/src/test/compile-fail/borrowck-call-sendfn.rs @@ -0,0 +1,8 @@ +// xfail-test #2978 + +fn call(x: @{mut f: fn~()}) { + x.f(); //~ ERROR foo + //~^ NOTE bar +} + +fn main() {} diff --git a/src/test/compile-fail/borrowck-lend-args.rs b/src/test/compile-fail/borrowck-lend-args.rs index 4fbc51183ce5..7978ffc9550b 100644 --- a/src/test/compile-fail/borrowck-lend-args.rs +++ b/src/test/compile-fail/borrowck-lend-args.rs @@ -5,7 +5,7 @@ fn borrow_from_arg_imm_ref(&&v: ~int) { } fn borrow_from_arg_mut_ref(&v: ~int) { - borrow(v); //~ ERROR illegal borrow unless pure: unique value in aliasable, mutable location + borrow(v); //~ ERROR illegal borrow unless pure: creating immutable alias to aliasable, mutable memory //~^ NOTE impure due to access to impure function } diff --git a/src/test/compile-fail/borrowck-mut-deref-comp.rs b/src/test/compile-fail/borrowck-mut-deref-comp.rs index b97471e6042d..005689a98976 100644 --- a/src/test/compile-fail/borrowck-mut-deref-comp.rs +++ b/src/test/compile-fail/borrowck-mut-deref-comp.rs @@ -1,7 +1,7 @@ enum foo = ~int; fn borrow(x: @mut foo) { - let y = &***x; //~ ERROR illegal borrow unless pure: unique value in aliasable, mutable location + let _y = &***x; //~ ERROR illegal borrow unless pure: creating immutable alias to aliasable, mutable memory *x = foo(~4); //~ NOTE impure due to assigning to dereference of mutable @ pointer } diff --git a/src/test/compile-fail/borrowck-pure-scope-in-call.rs b/src/test/compile-fail/borrowck-pure-scope-in-call.rs index 71a3988b2eee..b7cb1202c3d5 100644 --- a/src/test/compile-fail/borrowck-pure-scope-in-call.rs +++ b/src/test/compile-fail/borrowck-pure-scope-in-call.rs @@ -4,7 +4,7 @@ fn test1(x: @mut ~int) { // Here, evaluating the second argument actually invalidates the // first borrow, even though it occurs outside of the scope of the // borrow! - pure_borrow(*x, *x = ~5); //~ ERROR illegal borrow unless pure: unique value in aliasable, mutable location + pure_borrow(*x, *x = ~5); //~ ERROR illegal borrow unless pure: creating immutable alias to aliasable, mutable memory //~^ NOTE impure due to assigning to dereference of mutable @ pointer } diff --git a/src/test/compile-fail/borrowck-uniq-via-box.rs b/src/test/compile-fail/borrowck-uniq-via-box.rs index 6fcdf369aba3..130a6f6bd85c 100644 --- a/src/test/compile-fail/borrowck-uniq-via-box.rs +++ b/src/test/compile-fail/borrowck-uniq-via-box.rs @@ -1,22 +1,22 @@ fn borrow(_v: &int) {} fn box_mut(v: @mut ~int) { - borrow(*v); //~ ERROR illegal borrow unless pure: unique value in aliasable, mutable location + borrow(*v); //~ ERROR illegal borrow unless pure: creating immutable alias to aliasable, mutable memory //~^ NOTE impure due to access to impure function } fn box_rec_mut(v: @{mut f: ~int}) { - borrow(v.f); //~ ERROR illegal borrow unless pure: unique value in aliasable, mutable location + borrow(v.f); //~ ERROR illegal borrow unless pure: creating immutable alias to aliasable, mutable memory //~^ NOTE impure due to access to impure function } fn box_mut_rec(v: @mut {f: ~int}) { - borrow(v.f); //~ ERROR illegal borrow unless pure: unique value in aliasable, mutable location + borrow(v.f); //~ ERROR illegal borrow unless pure: creating immutable alias to aliasable, mutable memory //~^ NOTE impure due to access to impure function } fn box_mut_recs(v: @mut {f: {g: {h: ~int}}}) { - borrow(v.f.g.h); //~ ERROR illegal borrow unless pure: unique value in aliasable, mutable location + borrow(v.f.g.h); //~ ERROR illegal borrow unless pure: creating immutable alias to aliasable, mutable memory //~^ NOTE impure due to access to impure function } @@ -33,27 +33,27 @@ fn box_imm_recs(v: @{f: {g: {h: ~int}}}) { } fn box_const(v: @const ~int) { - borrow(*v); //~ ERROR illegal borrow unless pure: unique value in aliasable, mutable location + borrow(*v); //~ ERROR illegal borrow unless pure: creating immutable alias to aliasable, const memory //~^ NOTE impure due to access to impure function } fn box_rec_const(v: @{const f: ~int}) { - borrow(v.f); //~ ERROR illegal borrow unless pure: unique value in aliasable, mutable location + borrow(v.f); //~ ERROR illegal borrow unless pure: creating immutable alias to aliasable, const memory //~^ NOTE impure due to access to impure function } fn box_recs_const(v: @{f: {g: {const h: ~int}}}) { - borrow(v.f.g.h); //~ ERROR illegal borrow unless pure: unique value in aliasable, mutable location + borrow(v.f.g.h); //~ ERROR illegal borrow unless pure: creating immutable alias to aliasable, const memory //~^ NOTE impure due to access to impure function } fn box_const_rec(v: @const {f: ~int}) { - borrow(v.f); //~ ERROR illegal borrow unless pure: unique value in aliasable, mutable location + borrow(v.f); //~ ERROR illegal borrow unless pure: creating immutable alias to aliasable, const memory //~^ NOTE impure due to access to impure function } fn box_const_recs(v: @const {f: {g: {h: ~int}}}) { - borrow(v.f.g.h); //~ ERROR illegal borrow unless pure: unique value in aliasable, mutable location + borrow(v.f.g.h); //~ ERROR illegal borrow unless pure: creating immutable alias to aliasable, const memory //~^ NOTE impure due to access to impure function } diff --git a/src/test/compile-fail/borrowck-uniq-via-ref.rs b/src/test/compile-fail/borrowck-uniq-via-ref.rs index fe825165183a..e52b3c7807d2 100644 --- a/src/test/compile-fail/borrowck-uniq-via-ref.rs +++ b/src/test/compile-fail/borrowck-uniq-via-ref.rs @@ -1,22 +1,22 @@ fn borrow(_v: &int) {} fn box_mut(v: &mut ~int) { - borrow(*v); //~ ERROR illegal borrow unless pure: unique value in aliasable, mutable location + borrow(*v); //~ ERROR illegal borrow unless pure: creating immutable alias to aliasable, mutable memory //~^ NOTE impure due to access to impure function } fn box_rec_mut(v: &{mut f: ~int}) { - borrow(v.f); //~ ERROR illegal borrow unless pure: unique value in aliasable, mutable location + borrow(v.f); //~ ERROR illegal borrow unless pure: creating immutable alias to aliasable, mutable memory //~^ NOTE impure due to access to impure function } fn box_mut_rec(v: &mut {f: ~int}) { - borrow(v.f); //~ ERROR illegal borrow unless pure: unique value in aliasable, mutable location + borrow(v.f); //~ ERROR illegal borrow unless pure: creating immutable alias to aliasable, mutable memory //~^ NOTE impure due to access to impure function } fn box_mut_recs(v: &mut {f: {g: {h: ~int}}}) { - borrow(v.f.g.h); //~ ERROR illegal borrow unless pure: unique value in aliasable, mutable location + borrow(v.f.g.h); //~ ERROR illegal borrow unless pure: creating immutable alias to aliasable, mutable memory //~^ NOTE impure due to access to impure function } @@ -33,27 +33,27 @@ fn box_imm_recs(v: &{f: {g: {h: ~int}}}) { } fn box_const(v: &const ~int) { - borrow(*v); //~ ERROR illegal borrow unless pure: unique value in aliasable, mutable location + borrow(*v); //~ ERROR illegal borrow unless pure: creating immutable alias to aliasable, const memory //~^ NOTE impure due to access to impure function } fn box_rec_const(v: &{const f: ~int}) { - borrow(v.f); //~ ERROR illegal borrow unless pure: unique value in aliasable, mutable location + borrow(v.f); //~ ERROR illegal borrow unless pure: creating immutable alias to aliasable, const memory //~^ NOTE impure due to access to impure function } fn box_recs_const(v: &{f: {g: {const h: ~int}}}) { - borrow(v.f.g.h); //~ ERROR illegal borrow unless pure: unique value in aliasable, mutable location + borrow(v.f.g.h); //~ ERROR illegal borrow unless pure: creating immutable alias to aliasable, const memory //~^ NOTE impure due to access to impure function } fn box_const_rec(v: &const {f: ~int}) { - borrow(v.f); //~ ERROR illegal borrow unless pure: unique value in aliasable, mutable location + borrow(v.f); //~ ERROR illegal borrow unless pure: creating immutable alias to aliasable, const memory //~^ NOTE impure due to access to impure function } fn box_const_recs(v: &const {f: {g: {h: ~int}}}) { - borrow(v.f.g.h); //~ ERROR illegal borrow unless pure: unique value in aliasable, mutable location + borrow(v.f.g.h); //~ ERROR illegal borrow unless pure: creating immutable alias to aliasable, const memory //~^ NOTE impure due to access to impure function } diff --git a/src/test/compile-fail/regions-escape-via-iface-or-not.rs b/src/test/compile-fail/regions-escape-via-iface-or-not.rs new file mode 100644 index 000000000000..415e66fbda2f --- /dev/null +++ b/src/test/compile-fail/regions-escape-via-iface-or-not.rs @@ -0,0 +1,24 @@ +iface deref { + fn get() -> int; +} + +impl of deref for &int { + fn get() -> int { + *self + } +} + +fn with(f: fn(x: &int) -> R) -> int { + f(&3).get() +} + +fn return_it() -> int { + with(|o| o) + //~^ ERROR reference is not valid outside of its lifetime, & + //~^^ ERROR reference is not valid outside of its lifetime, & +} + +fn main() { + let x = return_it(); + #debug["foo=%d", x]; +} diff --git a/src/test/compile-fail/regions-ret-borrowed-1.rs b/src/test/compile-fail/regions-ret-borrowed-1.rs new file mode 100644 index 000000000000..86210b25a101 --- /dev/null +++ b/src/test/compile-fail/regions-ret-borrowed-1.rs @@ -0,0 +1,17 @@ +// Similar to regions-ret-borrowed.rs, but using a named lifetime. At +// some point regions-ret-borrowed reported an error but this file did +// not, due to special hardcoding around the anonymous region. + +fn with(f: fn(x: &a/int) -> R) -> R { + f(&3) +} + +fn return_it() -> &a/int { + with(|o| o) //~ ERROR mismatched types + //~^ ERROR reference is not valid outside of its lifetime +} + +fn main() { + let x = return_it(); + #debug["foo=%d", *x]; +} diff --git a/src/test/compile-fail/regions-ret-borrowed.rs b/src/test/compile-fail/regions-ret-borrowed.rs new file mode 100644 index 000000000000..3bd3deefcf5f --- /dev/null +++ b/src/test/compile-fail/regions-ret-borrowed.rs @@ -0,0 +1,20 @@ +// Ensure that you cannot use generic types to return a region outside +// of its bound. Here, in the `return_it()` fn, we call with() but +// with R bound to &int from the return_it. Meanwhile, with() +// provides a value that is only good within its own stack frame. This +// used to successfully compile because we failed to account for the +// fact that fn(x: &int) rebound the region &. + +fn with(f: fn(x: &int) -> R) -> R { + f(&3) +} + +fn return_it() -> &int { + with(|o| o) //~ ERROR mismatched types + //~^ ERROR reference is not valid outside of its lifetime +} + +fn main() { + let x = return_it(); + #debug["foo=%d", *x]; +} diff --git a/src/test/run-pass/borrowck-mut-uniq.rs b/src/test/run-pass/borrowck-mut-uniq.rs new file mode 100644 index 000000000000..c01a9feab7dd --- /dev/null +++ b/src/test/run-pass/borrowck-mut-uniq.rs @@ -0,0 +1,26 @@ +type ints = {sum: ~int, values: ~[int]}; + +fn add_int(x: &mut ints, v: int) { + *x.sum += v; + let mut values = ~[]; + x.values <-> values; + vec::push(values, v); + x.values <- values; +} + +fn iter_ints(x: &ints, f: fn(x: &int) -> bool) { + let l = x.values.len(); + uint::range(0, l, |i| f(&x.values[i])) +} + +fn main() { + let mut ints = ~{sum: ~0, values: ~[]}; + add_int(ints, 22); + add_int(ints, 44); + + for iter_ints(ints) |i| { + #error["int = %d", *i]; + } + + #error["ints=%?", ints]; +}