Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generalize dropck to ignore item-less traits #25113

Merged
merged 3 commits into from
May 5, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 47 additions & 35 deletions src/librustc_typeck/check/dropck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -450,45 +450,57 @@ fn iterate_over_potentially_unsafe_regions_in_type<'a, 'tcx>(

let dtor_typescheme = ty::lookup_item_type(rcx.tcx(), impl_did);
let dtor_generics = dtor_typescheme.generics;
let dtor_predicates = ty::lookup_predicates(rcx.tcx(), impl_did);

let has_pred_of_interest = dtor_predicates.predicates.iter().any(|pred| {
// In `impl<T> Drop where ...`, we automatically
// assume some predicate will be meaningful and thus
// represents a type through which we could reach
// borrowed data. However, there can be implicit
// predicates (namely for Sized), and so we still need
// to walk through and filter out those cases.

let result = match *pred {
ty::Predicate::Trait(ty::Binder(ref t_pred)) => {
let def_id = t_pred.trait_ref.def_id;
match rcx.tcx().lang_items.to_builtin_kind(def_id) {
// Issue 24895: deliberately do not include `BoundCopy` here.
Some(ty::BoundSend) |
Some(ty::BoundSized) |
Some(ty::BoundSync) => false,
_ => true,

let mut has_pred_of_interest = false;

let mut seen_items = Vec::new();
let mut items_to_inspect = vec![impl_did];
'items: while let Some(item_def_id) = items_to_inspect.pop() {
if seen_items.contains(&item_def_id) {
continue;
}

for pred in ty::lookup_predicates(rcx.tcx(), item_def_id).predicates {
let result = match pred {
ty::Predicate::Equate(..) |
ty::Predicate::RegionOutlives(..) |
ty::Predicate::TypeOutlives(..) |
ty::Predicate::Projection(..) => {
// For now, assume all these where-clauses
// may give drop implementation capabilty
// to access borrowed data.
true
}
}
ty::Predicate::Equate(..) |
ty::Predicate::RegionOutlives(..) |
ty::Predicate::TypeOutlives(..) |
ty::Predicate::Projection(..) => {
// we assume all of these where-clauses may
// give the drop implementation the capabilty
// to access borrowed data.
true
}
};

if result {
debug!("typ: {} has interesting dtor due to generic preds, e.g. {}",
typ.repr(rcx.tcx()), pred.repr(rcx.tcx()));
ty::Predicate::Trait(ty::Binder(ref t_pred)) => {
let def_id = t_pred.trait_ref.def_id;
if ty::trait_items(rcx.tcx(), def_id).len() != 0 {
// If trait has items, assume it adds
// capability to access borrowed data.
true
} else {
// Trait without items is itself
// uninteresting from POV of dropck.
//
// However, may have parent w/ items;
// so schedule checking of predicates,
items_to_inspect.push(def_id);
// and say "no capability found" for now.
false
}
}
};

if result {
has_pred_of_interest = true;
debug!("typ: {} has interesting dtor due to generic preds, e.g. {}",
typ.repr(rcx.tcx()), pred.repr(rcx.tcx()));
break 'items;
}
}

result
});
seen_items.push(item_def_id);
}

// In `impl<'a> Drop ...`, we automatically assume
// `'a` is meaningful and thus represents a bound
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright 2015 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Check that child trait who only has items via its *parent* trait
// does cause dropck to inject extra region constraints.

#![allow(non_camel_case_types)]

trait Parent { fn foo(&self); }
trait Child: Parent { }

impl Parent for i32 { fn foo(&self) { } }
impl<'a> Parent for &'a D_Child<i32> {
fn foo(&self) {
println!("accessing child value: {}", self.0);
}
}

impl Child for i32 { }
impl<'a> Child for &'a D_Child<i32> { }

struct D_Child<T:Child>(T);
impl <T:Child> Drop for D_Child<T> { fn drop(&mut self) { self.0.foo() } }

fn f_child() {
// `_d` and `d1` are assigned the *same* lifetime by region inference ...
let (_d, d1);

d1 = D_Child(1);
// ... we store a reference to `d1` within `_d` ...
_d = D_Child(&d1); //~ ERROR `d1` does not live long enough

// ... dropck *should* complain, because Drop of _d could (and
// does) access the already dropped `d1` via the `foo` method.
}

fn main() {
f_child();
}
64 changes: 64 additions & 0 deletions src/test/compile-fail/issue-24805-dropck-trait-has-items.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// Copyright 2015 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Check that traits with various kinds of associated items cause
// dropck to inject extra region constraints.

#![allow(non_camel_case_types)]

trait HasSelfMethod { fn m1(&self) { } }
trait HasMethodWithSelfArg { fn m2(x: &Self) { } }
trait HasType { type Something; }

impl HasSelfMethod for i32 { }
impl HasMethodWithSelfArg for i32 { }
impl HasType for i32 { type Something = (); }

impl<'a,T> HasSelfMethod for &'a T { }
impl<'a,T> HasMethodWithSelfArg for &'a T { }
impl<'a,T> HasType for &'a T { type Something = (); }

// e.g. `impl_drop!(Send, D_Send)` expands to:
// ```rust
// struct D_Send<T:Send>(T);
// impl<T:Send> Drop for D_Send<T> { fn drop(&mut self) { } }
// ```
macro_rules! impl_drop {
($Bound:ident, $Id:ident) => {
struct $Id<T:$Bound>(T);
impl <T:$Bound> Drop for $Id<T> { fn drop(&mut self) { } }
}
}

impl_drop!{HasSelfMethod, D_HasSelfMethod}
impl_drop!{HasMethodWithSelfArg, D_HasMethodWithSelfArg}
impl_drop!{HasType, D_HasType}

fn f_sm() {
let (_d, d1);
d1 = D_HasSelfMethod(1);
_d = D_HasSelfMethod(&d1); //~ ERROR `d1` does not live long enough
}
fn f_mwsa() {
let (_d, d1);
d1 = D_HasMethodWithSelfArg(1);
_d = D_HasMethodWithSelfArg(&d1); //~ ERROR `d1` does not live long enough
}
fn f_t() {
let (_d, d1);
d1 = D_HasType(1);
_d = D_HasType(&d1); //~ ERROR `d1` does not live long enough
}

fn main() {
f_sm();
f_mwsa();
f_t();
}
81 changes: 81 additions & 0 deletions src/test/run-pass/issue-24805-dropck-itemless.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// Copyright 2015 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Check that item-less traits do not cause dropck to inject extra
// region constraints.

#![allow(non_camel_case_types)]

trait UserDefined { }

impl UserDefined for i32 { }
impl<'a, T> UserDefined for &'a T { }

// e.g. `impl_drop!(Send, D_Send)` expands to:
// ```rust
// struct D_Send<T:Send>(T);
// impl<T:Send> Drop for D_Send<T> { fn drop(&mut self) { } }
// ```
macro_rules! impl_drop {
($Bound:ident, $Id:ident) => {
struct $Id<T:$Bound>(T);
impl <T:$Bound> Drop for $Id<T> { fn drop(&mut self) { } }
}
}

impl_drop!{Send, D_Send}
impl_drop!{Sized, D_Sized}

// See note below regarding Issue 24895
// impl_drop!{Copy, D_Copy}

impl_drop!{Sync, D_Sync}
impl_drop!{UserDefined, D_UserDefined}

macro_rules! body {
($id:ident) => { {
// `_d` and `d1` are assigned the *same* lifetime by region inference ...
let (_d, d1);

d1 = $id(1);
// ... we store a reference to `d1` within `_d` ...
_d = $id(&d1);

// ... a *conservative* dropck will thus complain, because it
// thinks Drop of _d could access the already dropped `d1`.
} }
}

fn f_send() { body!(D_Send) }
fn f_sized() { body!(D_Sized) }
fn f_sync() { body!(D_Sync) }

// Issue 24895: Copy: Clone implies `impl<T:Copy> Drop for ...` can
// access a user-defined clone() method, which causes this test case
// to fail.
//
// If 24895 is resolved by removing the `Copy: Clone` relationship,
// then this definition and the call below should be uncommented. If
// it is resolved by deciding to keep the `Copy: Clone` relationship,
// then this comment and the associated bits of code can all be
// removed.

// fn f_copy() { body!(D_Copy) }

fn f_userdefined() { body!(D_UserDefined) }

fn main() {
f_send();
f_sized();
// See note above regarding Issue 24895.
// f_copy();
f_sync();
f_userdefined();
}