diff options
| -rw-r--r-- | src/librustc_borrowck/borrowck/mir/elaborate_drops.rs | 24 | ||||
| -rw-r--r-- | src/librustc_borrowck/borrowck/mir/mod.rs | 51 | ||||
| -rw-r--r-- | src/test/compile-fail/no-warn-on-field-replace-issue-34101.rs | 56 |
3 files changed, 103 insertions, 28 deletions
diff --git a/src/librustc_borrowck/borrowck/mir/elaborate_drops.rs b/src/librustc_borrowck/borrowck/mir/elaborate_drops.rs index b09db70e7b8..d5539f953fb 100644 --- a/src/librustc_borrowck/borrowck/mir/elaborate_drops.rs +++ b/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. - /// - /// 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. + /// includes all lvalues, except these (1.) behind references or arrays, + /// or (2.) behind ADT's with a Drop impl. 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() { diff --git a/src/librustc_borrowck/borrowck/mir/mod.rs b/src/librustc_borrowck/borrowck/mir/mod.rs index 007cde156f4..32ffce6440b 100644 --- a/src/librustc_borrowck/borrowck/mir/mod.rs +++ b/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 { - // 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 - } + lvalue_contents_drop_state_cannot_differ(tcx, mir, lvalue) } _ => true } diff --git a/src/test/compile-fail/no-warn-on-field-replace-issue-34101.rs b/src/test/compile-fail/no-warn-on-field-replace-issue-34101.rs new file mode 100644 index 00000000000..2940b891534 --- /dev/null +++ b/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(); +} |