Auto merge of #34109 - pnkfelix:fix-issue-34101, r=arielb1

Fix issue #34101

Fix issue #34101: do not track subcontent of type with dtor nor gather flags for untracked content.

(Includes a regression test, which needed to go into `compile-fail/`
due to weaknesses when combining `#[deny(warnings)]` with
`tcx.sess.span_warn(..)`)

src/librustc_borrowck/borrowck/mir/elaborate_drops.rs

@@ -197,31 +197,21 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> {
     }
 
     /// Returns whether this lvalue is tracked by drop elaboration. This
-    /// includes all lvalues, except these behind references or arrays.
+    /// includes all lvalues, except these (1.) behind references or arrays,
-    ///
+    ///  or (2.) behind ADT's with a Drop impl.
-    /// Lvalues behind references or arrays are not tracked by elaboration
-    /// and are always assumed to be initialized when accessible. As
-    /// references and indexes can be reseated, trying to track them
-    /// can only lead to trouble.
     fn lvalue_is_tracked(&self, lv: &Lvalue<'tcx>) -> bool
     {
+        // `lvalue_contents_drop_state_cannot_differ` only compares
+        // the `lv` to its immediate contents, while this recursively
+        // follows parent chain formed by `base` of each projection.
         if let &Lvalue::Projection(ref data) = lv {
-            self.lvalue_contents_are_tracked(&data.base)
+            !super::lvalue_contents_drop_state_cannot_differ(self.tcx, self.mir, &data.base) &&
+                self.lvalue_is_tracked(&data.base)
         } else {
             true
         }
     }
 
-    fn lvalue_contents_are_tracked(&self, lv: &Lvalue<'tcx>) -> bool {
-        let ty = self.mir.lvalue_ty(self.tcx, lv).to_ty(self.tcx);
-        match ty.sty {
-            ty::TyArray(..) | ty::TySlice(..) | ty::TyRef(..) | ty::TyRawPtr(..) => {
-                false
-            }
-            _ => self.lvalue_is_tracked(lv)
-        }
-    }
-
     fn collect_drop_flags(&mut self)
     {
         for bb in self.mir.all_basic_blocks() {

src/librustc_borrowck/borrowck/mir/mod.rs

@@ -235,6 +235,45 @@ fn move_path_children_matching<'tcx, F>(move_paths: &MovePathData<'tcx>,
     None
 }
 
+/// When enumerating the child fragments of a path, don't recurse into
+/// paths (1.) past arrays, slices, and pointers, nor (2.) into a type
+/// that implements `Drop`.
+///
+/// Lvalues behind references or arrays are not tracked by elaboration
+/// and are always assumed to be initialized when accessible. As
+/// references and indexes can be reseated, trying to track them can
+/// only lead to trouble.
+///
+/// Lvalues behind ADT's with a Drop impl are not tracked by
+/// elaboration since they can never have a drop-flag state that
+/// differs from that of the parent with the Drop impl.
+///
+/// In both cases, the contents can only be accessed if and only if
+/// their parents are initialized. This implies for example that there
+/// is no need to maintain separate drop flags to track such state.
+///
+/// FIXME: we have to do something for moving slice patterns.
+fn lvalue_contents_drop_state_cannot_differ<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
+                                                      mir: &Mir<'tcx>,
+                                                      lv: &repr::Lvalue<'tcx>) -> bool {
+    let ty = mir.lvalue_ty(tcx, lv).to_ty(tcx);
+    match ty.sty {
+        ty::TyArray(..) | ty::TySlice(..) | ty::TyRef(..) | ty::TyRawPtr(..) => {
+            debug!("lvalue_contents_drop_state_cannot_differ lv: {:?} ty: {:?} refd => false",
+                   lv, ty);
+            true
+        }
+        ty::TyStruct(def, _) | ty::TyEnum(def, _) if def.has_dtor() => {
+            debug!("lvalue_contents_drop_state_cannot_differ lv: {:?} ty: {:?} Drop => false",
+                   lv, ty);
+            true
+        }
+        _ => {
+            false
+        }
+    }
+}
+
 fn on_all_children_bits<'a, 'tcx, F>(
     tcx: TyCtxt<'a, 'tcx, 'tcx>,
     mir: &Mir<'tcx>,
@@ -251,17 +290,7 @@ fn on_all_children_bits<'a, 'tcx, F>(
     {
         match move_data.move_paths[path].content {
             MovePathContent::Lvalue(ref lvalue) => {
-                match mir.lvalue_ty(tcx, lvalue).to_ty(tcx).sty {
+                lvalue_contents_drop_state_cannot_differ(tcx, mir, lvalue)
-                    // don't trace paths past arrays, slices, and
-                    // pointers. They can only be accessed while
-                    // their parents are initialized.
-                    //
-                    // FIXME: we have to do something for moving
-                    // slice patterns.
-                    ty::TyArray(..) | ty::TySlice(..) |
-                    ty::TyRef(..) | ty::TyRawPtr(..) => true,
-                    _ => false
-                }
             }
             _ => true
         }

src/test/compile-fail/no-warn-on-field-replace-issue-34101.rs

@@ -0,0 +1,56 @@
+// 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 <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.
+
+// Issue 34101: Circa 2016-06-05, `fn inline` below issued an
+// erroneous warning from the elaborate_drops pass about moving out of
+// a field in `Foo`, which has a destructor (and thus cannot have
+// content moved out of it). The reason that the warning is erroneous
+// in this case is that we are doing a *replace*, not a move, of the
+// content in question, and it is okay to replace fields within `Foo`.
+//
+// Another more subtle problem was that the elaborate_drops was
+// creating a separate drop flag for that internally replaced content,
+// even though the compiler should enforce an invariant that any drop
+// flag for such subcontent of `Foo` will always have the same value
+// as the drop flag for `Foo` itself.
+//
+// This test is structured in a funny way; we cannot test for emission
+// of the warning in question via the lint system, and therefore
+// `#![deny(warnings)]` does nothing to detect it.
+//
+// So instead we use `#[rustc_error]` and put the test into
+// `compile_fail`, where the emitted warning *will* be caught.
+
+#![feature(rustc_attrs)]
+
+struct Foo(String);
+
+impl Drop for Foo {
+    fn drop(&mut self) {}
+}
+
+fn inline() {
+    // (dummy variable so `f` gets assigned `var1` in MIR for both fn's)
+    let _s = ();
+    let mut f = Foo(String::from("foo"));
+    f.0 = String::from("bar");
+}
+
+fn outline() {
+    let _s = String::from("foo");
+    let mut f = Foo(_s);
+    f.0 = String::from("bar");
+}
+
+#[rustc_error]
+fn main() { //~ ERROR compilation successful
+    inline();
+    outline();
+}