Auto merge of #25113 - pnkfelix:dropck-itemless-traits, r=huonw

bors · bors · commit c0100ce84698 · 2015-05-05T11:06:35.000Z
Generalize dropck to ignore item-less traits Fix #24805. (This is the reopened + rebased version of PR #24898)
diff --git a/src/librustc_typeck/check/dropck.rs b/src/librustc_typeck/check/dropck.rs
@@ -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
diff --git a/src/test/compile-fail/issue-24805-dropck-child-has-items-via-parent.rs b/src/test/compile-fail/issue-24805-dropck-child-has-items-via-parent.rs
@@ -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();
+}
diff --git a/src/test/compile-fail/issue-24805-dropck-trait-has-items.rs b/src/test/compile-fail/issue-24805-dropck-trait-has-items.rs
@@ -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();
+}
diff --git a/src/test/run-pass/issue-24805-dropck-itemless.rs b/src/test/run-pass/issue-24805-dropck-itemless.rs
@@ -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();
+}
