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

Fix issue #34101 #34109

Merged
merged 1 commit into from
Jun 9, 2016
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
24 changes: 7 additions & 17 deletions src/librustc_borrowck/borrowck/mir/elaborate_drops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,31 +198,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() {
Expand Down
51 changes: 40 additions & 11 deletions src/librustc_borrowck/borrowck/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand All @@ -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
}
Expand Down
56 changes: 56 additions & 0 deletions src/test/compile-fail/no-warn-on-field-replace-issue-34101.rs
Original file line number Diff line number Diff line change
@@ -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();
}