From 7eba13f45164f42892b1430badae197ec2a77c5a Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 4 Jul 2018 20:08:15 +0100 Subject: [PATCH 01/17] Correctly handle an activation from dead code. --- src/librustc_mir/borrow_check/borrow_set.rs | 52 +++++++-------------- src/librustc_mir/borrow_check/path_utils.rs | 10 ++-- src/test/run-pass/issue-51345.rs | 17 +++++++ 3 files changed, 38 insertions(+), 41 deletions(-) create mode 100644 src/test/run-pass/issue-51345.rs diff --git a/src/librustc_mir/borrow_check/borrow_set.rs b/src/librustc_mir/borrow_check/borrow_set.rs index 3d6f49c377226..ce02a7dc16602 100644 --- a/src/librustc_mir/borrow_check/borrow_set.rs +++ b/src/librustc_mir/borrow_check/borrow_set.rs @@ -53,15 +53,13 @@ impl<'tcx> Index for BorrowSet<'tcx> { } } -/// Every two-phase borrow has *exactly one* use (or else it is not a -/// proper two-phase borrow under our current definition). However, not -/// all uses are actually ones that activate the reservation.. In -/// particular, a shared borrow of a `&mut` does not activate the -/// reservation. +/// Location where a two phase borrow is activated, if a borrow +/// is in fact a two phase borrow. #[derive(Copy, Clone, PartialEq, Eq, Debug)] -crate enum TwoPhaseUse { - MutActivate, - SharedUse, +crate enum TwoPhaseActivation { + NotTwoPhase, + NotActivated, + ActivatedAt(Location), } #[derive(Debug)] @@ -69,9 +67,8 @@ crate struct BorrowData<'tcx> { /// Location where the borrow reservation starts. /// In many cases, this will be equal to the activation location but not always. crate reserve_location: Location, - /// Location where the borrow is activated. None if this is not a - /// 2-phase borrow. - crate activation_location: Option<(TwoPhaseUse, Location)>, + /// Location where the borrow is activated. + crate activation_location: TwoPhaseActivation, /// What kind of borrow this is crate kind: mir::BorrowKind, /// The region for which this borrow is live @@ -116,19 +113,6 @@ impl<'tcx> BorrowSet<'tcx> { visitor.visit_basic_block_data(block, block_data); } - // Double check: We should have found an activation for every pending - // activation. - assert_eq!( - visitor - .pending_activations - .iter() - .find(|&(_local, &borrow_index)| visitor.idx_vec[borrow_index] - .activation_location - .is_none()), - None, - "never found an activation for this borrow!", - ); - BorrowSet { borrows: visitor.idx_vec, location_map: visitor.location_map, @@ -183,7 +167,7 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> { kind, region, reserve_location: location, - activation_location: None, + activation_location: TwoPhaseActivation::NotTwoPhase, borrowed_place: borrowed_place.clone(), assigned_place: assigned_place.clone(), }; @@ -232,38 +216,34 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> { return; } - if let Some(other_activation) = borrow_data.activation_location { + if let TwoPhaseActivation::ActivatedAt(other_location) = + borrow_data.activation_location { span_bug!( self.mir.source_info(location).span, "found two uses for 2-phase borrow temporary {:?}: \ {:?} and {:?}", temp, location, - other_activation, + other_location, ); } // Otherwise, this is the unique later use // that we expect. - - let two_phase_use; - - match context { + borrow_data.activation_location = match context { // The use of TMP in a shared borrow does not // count as an actual activation. PlaceContext::Borrow { kind: mir::BorrowKind::Shared, .. } => { - two_phase_use = TwoPhaseUse::SharedUse; + TwoPhaseActivation::NotActivated } _ => { - two_phase_use = TwoPhaseUse::MutActivate; self.activation_map .entry(location) .or_insert(Vec::new()) .push(borrow_index); + TwoPhaseActivation::ActivatedAt(location) } - } - - borrow_data.activation_location = Some((two_phase_use, location)); + }; } None => {} diff --git a/src/librustc_mir/borrow_check/path_utils.rs b/src/librustc_mir/borrow_check/path_utils.rs index 8ae98bde00344..ca2a120ceb737 100644 --- a/src/librustc_mir/borrow_check/path_utils.rs +++ b/src/librustc_mir/borrow_check/path_utils.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use borrow_check::borrow_set::{BorrowSet, BorrowData, TwoPhaseUse}; +use borrow_check::borrow_set::{BorrowSet, BorrowData, TwoPhaseActivation}; use borrow_check::places_conflict; use borrow_check::Context; use borrow_check::ShallowOrDeep; @@ -83,11 +83,11 @@ pub(super) fn is_active<'tcx>( let activation_location = match borrow_data.activation_location { // If this is not a 2-phase borrow, it is always active. - None => return true, + TwoPhaseActivation::NotTwoPhase => return true, // And if the unique 2-phase use is not an activation, then it is *never* active. - Some((TwoPhaseUse::SharedUse, _)) => return false, - // Otherwise, we derive info from the activation point `v`: - Some((TwoPhaseUse::MutActivate, v)) => v, + TwoPhaseActivation::NotActivated => return false, + // Otherwise, we derive info from the activation point `loc`: + TwoPhaseActivation::ActivatedAt(loc) => loc, }; // Otherwise, it is active for every location *except* in between diff --git a/src/test/run-pass/issue-51345.rs b/src/test/run-pass/issue-51345.rs new file mode 100644 index 0000000000000..7d392944685b8 --- /dev/null +++ b/src/test/run-pass/issue-51345.rs @@ -0,0 +1,17 @@ +// Copyright 2012 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(nll)] + +fn main() { + let mut v = Vec::new(); + + loop { v.push(break) } +} From aeb16828945ef7d3cda88d27da96a1b475acd90a Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 4 Jul 2018 21:35:38 +0100 Subject: [PATCH 02/17] Ensure that borrows wind up unactivated. --- src/librustc_mir/borrow_check/borrow_set.rs | 12 ++++++++++++ src/test/run-fail/issue-51345.rs | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 src/test/run-fail/issue-51345.rs diff --git a/src/librustc_mir/borrow_check/borrow_set.rs b/src/librustc_mir/borrow_check/borrow_set.rs index ce02a7dc16602..a427b9dd5b7d5 100644 --- a/src/librustc_mir/borrow_check/borrow_set.rs +++ b/src/librustc_mir/borrow_check/borrow_set.rs @@ -237,6 +237,14 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> { TwoPhaseActivation::NotActivated } _ => { + // Double check: We should have found an activation for every pending + // activation. + assert_eq!( + borrow_data.activation_location, + TwoPhaseActivation::NotActivated, + "never found an activation for this borrow!", + ); + self.activation_map .entry(location) .or_insert(Vec::new()) @@ -322,6 +330,10 @@ impl<'a, 'gcx, 'tcx> GatherBorrows<'a, 'gcx, 'tcx> { ); }; + // Consider the borrow not activated. + let borrow_data = &mut self.idx_vec[borrow_index]; + borrow_data.activation_location = TwoPhaseActivation::NotActivated; + // Insert `temp` into the list of pending activations. From // now on, we'll be on the lookout for a use of it. Note that // we are guaranteed that this use will come after the diff --git a/src/test/run-fail/issue-51345.rs b/src/test/run-fail/issue-51345.rs new file mode 100644 index 0000000000000..9efd08195a1bb --- /dev/null +++ b/src/test/run-fail/issue-51345.rs @@ -0,0 +1,18 @@ +// Copyright 2012 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// error-pattern: thread 'main' panicked at 'explicit panic' + +#![feature(nll)] + +fn main() { + let mut vec = vec![]; + vec.push((vec.len(), panic!())); +} From f90eada1c97dd0d0a752d34090a957661789379a Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 4 Jul 2018 21:48:25 +0100 Subject: [PATCH 03/17] Improve comments. --- src/librustc_mir/borrow_check/borrow_set.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/borrow_check/borrow_set.rs b/src/librustc_mir/borrow_check/borrow_set.rs index a427b9dd5b7d5..e9e0c5c361353 100644 --- a/src/librustc_mir/borrow_check/borrow_set.rs +++ b/src/librustc_mir/borrow_check/borrow_set.rs @@ -237,8 +237,9 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> { TwoPhaseActivation::NotActivated } _ => { - // Double check: We should have found an activation for every pending - // activation. + // Double check: This borrow is indeed a two-phase borrow (that is, + // we are 'transitioning' from `NotActivated` to `ActivatedAt`) and + // we've not found any other activations (checked above). assert_eq!( borrow_data.activation_location, TwoPhaseActivation::NotActivated, @@ -330,7 +331,8 @@ impl<'a, 'gcx, 'tcx> GatherBorrows<'a, 'gcx, 'tcx> { ); }; - // Consider the borrow not activated. + // Consider the borrow not activated to start. When we find an activation, we'll update + // this field. let borrow_data = &mut self.idx_vec[borrow_index]; borrow_data.activation_location = TwoPhaseActivation::NotActivated; From 37228fe63286a920ad18cf72d37dfe69c7e8dc2d Mon Sep 17 00:00:00 2001 From: csmoe <35686186+csmoe@users.noreply.github.com> Date: Thu, 5 Jul 2018 09:06:04 +0800 Subject: [PATCH 04/17] reverse_postorder --- src/librustc_mir/dataflow/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/dataflow/mod.rs b/src/librustc_mir/dataflow/mod.rs index 98cd9c35d8809..8ed42a707dd86 100644 --- a/src/librustc_mir/dataflow/mod.rs +++ b/src/librustc_mir/dataflow/mod.rs @@ -16,6 +16,7 @@ use rustc_data_structures::bitslice::{bitwise, BitwiseOperator}; use rustc::ty::{self, TyCtxt}; use rustc::mir::{self, Mir, BasicBlock, BasicBlockData, Location, Statement, Terminator}; +use rustc::mir::traversal; use rustc::session::Session; use std::borrow::Borrow; @@ -333,7 +334,7 @@ pub(crate) trait DataflowResultsConsumer<'a, 'tcx: 'a> { fn analyze_results(&mut self, flow_uninit: &mut Self::FlowState) { let flow = flow_uninit; - for bb in self.mir().basic_blocks().indices() { + for (bb, _) in traversal::reverse_postorder(self.mir()) { flow.reset_to_entry_of(bb); self.process_basic_block(bb, flow); } From 0e31e2fa9b984ac15e3818685fa3ad87216811df Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Wed, 4 Jul 2018 19:25:37 -0300 Subject: [PATCH 05/17] Remove rustc_mir_borrowck attribute and use rustc_mir instead --- src/librustc_mir/borrow_check/mod.rs | 2 +- src/librustc_mir/transform/rustc_peek.rs | 2 +- src/test/compile-fail/mir-dataflow/def-inits-1.rs | 2 +- src/test/compile-fail/mir-dataflow/inits-1.rs | 2 +- src/test/compile-fail/mir-dataflow/uninits-1.rs | 2 +- src/test/compile-fail/mir-dataflow/uninits-2.rs | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 3aaa3378bb005..ba649f8e7a909 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -80,7 +80,7 @@ fn mir_borrowck<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> BorrowC let mut return_early; // Return early if we are not supposed to use MIR borrow checker for this function. - return_early = !tcx.has_attr(def_id, "rustc_mir_borrowck") && !tcx.use_mir_borrowck(); + return_early = !tcx.has_attr(def_id, "rustc_mir") && !tcx.use_mir_borrowck(); if tcx.is_struct_constructor(def_id) { // We are not borrow checking the automatically generated struct constructors diff --git a/src/librustc_mir/transform/rustc_peek.rs b/src/librustc_mir/transform/rustc_peek.rs index b23f056801210..da149f420644c 100644 --- a/src/librustc_mir/transform/rustc_peek.rs +++ b/src/librustc_mir/transform/rustc_peek.rs @@ -36,7 +36,7 @@ impl MirPass for SanityCheck { src: MirSource, mir: &mut Mir<'tcx>) { let def_id = src.def_id; let id = tcx.hir.as_local_node_id(def_id).unwrap(); - if !tcx.has_attr(def_id, "rustc_mir_borrowck") { + if !tcx.has_attr(def_id, "rustc_mir") { debug!("skipping rustc_peek::SanityCheck on {}", tcx.item_path_str(def_id)); return; } else { diff --git a/src/test/compile-fail/mir-dataflow/def-inits-1.rs b/src/test/compile-fail/mir-dataflow/def-inits-1.rs index f3c9f29821ebb..f5f0ede6864f4 100644 --- a/src/test/compile-fail/mir-dataflow/def-inits-1.rs +++ b/src/test/compile-fail/mir-dataflow/def-inits-1.rs @@ -10,6 +10,7 @@ // General test of maybe_uninits state computed by MIR dataflow. +#![feature(nll)] #![feature(core_intrinsics, rustc_attrs)] use std::intrinsics::rustc_peek; @@ -17,7 +18,6 @@ use std::mem::{drop, replace}; struct S(i32); -#[rustc_mir_borrowck] #[rustc_mir(rustc_peek_definite_init,stop_after_dataflow)] fn foo(test: bool, x: &mut S, y: S, mut z: S) -> S { let ret; diff --git a/src/test/compile-fail/mir-dataflow/inits-1.rs b/src/test/compile-fail/mir-dataflow/inits-1.rs index 8a5ab6e420ade..595f01f7c94b3 100644 --- a/src/test/compile-fail/mir-dataflow/inits-1.rs +++ b/src/test/compile-fail/mir-dataflow/inits-1.rs @@ -10,6 +10,7 @@ // General test of maybe_inits state computed by MIR dataflow. +#![feature(nll)] #![feature(core_intrinsics, rustc_attrs)] use std::intrinsics::rustc_peek; @@ -17,7 +18,6 @@ use std::mem::{drop, replace}; struct S(i32); -#[rustc_mir_borrowck] #[rustc_mir(rustc_peek_maybe_init,stop_after_dataflow)] fn foo(test: bool, x: &mut S, y: S, mut z: S) -> S { let ret; diff --git a/src/test/compile-fail/mir-dataflow/uninits-1.rs b/src/test/compile-fail/mir-dataflow/uninits-1.rs index 8df66ea815c68..bb10c03254e3c 100644 --- a/src/test/compile-fail/mir-dataflow/uninits-1.rs +++ b/src/test/compile-fail/mir-dataflow/uninits-1.rs @@ -10,6 +10,7 @@ // General test of maybe_uninits state computed by MIR dataflow. +#![feature(nll)] #![feature(core_intrinsics, rustc_attrs)] use std::intrinsics::rustc_peek; @@ -17,7 +18,6 @@ use std::mem::{drop, replace}; struct S(i32); -#[rustc_mir_borrowck] #[rustc_mir(rustc_peek_maybe_uninit,stop_after_dataflow)] fn foo(test: bool, x: &mut S, y: S, mut z: S) -> S { let ret; diff --git a/src/test/compile-fail/mir-dataflow/uninits-2.rs b/src/test/compile-fail/mir-dataflow/uninits-2.rs index 2edd275e78691..9854ea779b3ad 100644 --- a/src/test/compile-fail/mir-dataflow/uninits-2.rs +++ b/src/test/compile-fail/mir-dataflow/uninits-2.rs @@ -10,6 +10,7 @@ // General test of maybe_uninits state computed by MIR dataflow. +#![feature(nll)] #![feature(core_intrinsics, rustc_attrs)] use std::intrinsics::rustc_peek; @@ -17,7 +18,6 @@ use std::mem::{drop, replace}; struct S(i32); -#[rustc_mir_borrowck] #[rustc_mir(rustc_peek_maybe_uninit,stop_after_dataflow)] fn foo(x: &mut S) { // `x` is initialized here, so maybe-uninit bit is 0. From e7e8c72fdd0195358b4fe82f16fce81668ef0c3c Mon Sep 17 00:00:00 2001 From: csmoe <35686186+csmoe@users.noreply.github.com> Date: Thu, 5 Jul 2018 17:38:44 +0800 Subject: [PATCH 06/17] update test --- src/test/ui/issue-47184.stderr | 7 +++--- src/test/ui/nll/get_default.stderr | 24 +++++++++---------- .../span/dropck_arr_cycle_checked.nll.stderr | 12 +++++----- .../dropck_direct_cycle_with_drop.nll.stderr | 16 ++++++------- .../span/dropck_vec_cycle_checked.nll.stderr | 12 +++++----- ...-must-not-hide-type-from-dropck.nll.stderr | 16 ++++++------- 6 files changed, 43 insertions(+), 44 deletions(-) diff --git a/src/test/ui/issue-47184.stderr b/src/test/ui/issue-47184.stderr index a9eb33f01e3e0..32a9783e9c213 100644 --- a/src/test/ui/issue-47184.stderr +++ b/src/test/ui/issue-47184.stderr @@ -2,10 +2,9 @@ error[E0597]: borrowed value does not live long enough --> $DIR/issue-47184.rs:14:44 | LL | let _vec: Vec<&'static String> = vec![&String::new()]; - | ^^^^^^^^^^^^^ temporary value does not live long enough -LL | //~^ ERROR borrowed value does not live long enough [E0597] -LL | } - | - temporary value only lives until here + | ^^^^^^^^^^^^^ - temporary value only lives until here + | | + | temporary value does not live long enough | = note: borrowed value must be valid for the static lifetime... diff --git a/src/test/ui/nll/get_default.stderr b/src/test/ui/nll/get_default.stderr index dd69e18652c9a..75194bf55bc9f 100644 --- a/src/test/ui/nll/get_default.stderr +++ b/src/test/ui/nll/get_default.stderr @@ -55,6 +55,18 @@ LL | | } LL | | } | |_^ +error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Mir) + --> $DIR/get_default.rs:45:17 + | +LL | match map.get() { + | --- immutable borrow occurs here +LL | Some(v) => { +LL | map.set(String::new()); // Both AST and MIR error here + | ^^^ mutable borrow occurs here +... +LL | return v; + | - borrow later used here + error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Mir) --> $DIR/get_default.rs:51:17 | @@ -76,18 +88,6 @@ LL | | } LL | | } | |_^ -error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Mir) - --> $DIR/get_default.rs:45:17 - | -LL | match map.get() { - | --- immutable borrow occurs here -LL | Some(v) => { -LL | map.set(String::new()); // Both AST and MIR error here - | ^^^ mutable borrow occurs here -... -LL | return v; - | - borrow later used here - error: aborting due to 6 previous errors For more information about this error, try `rustc --explain E0502`. diff --git a/src/test/ui/span/dropck_arr_cycle_checked.nll.stderr b/src/test/ui/span/dropck_arr_cycle_checked.nll.stderr index fee0d7feb6d36..cbb9d0429c616 100644 --- a/src/test/ui/span/dropck_arr_cycle_checked.nll.stderr +++ b/src/test/ui/span/dropck_arr_cycle_checked.nll.stderr @@ -1,7 +1,7 @@ -error[E0597]: `b1` does not live long enough - --> $DIR/dropck_arr_cycle_checked.rs:111:24 +error[E0597]: `b3` does not live long enough + --> $DIR/dropck_arr_cycle_checked.rs:105:24 | -LL | b3.a[0].v.set(Some(&b1)); +LL | b1.a[1].v.set(Some(&b3)); | ^^^ borrowed value does not live long enough ... LL | } @@ -22,10 +22,10 @@ LL | } | borrowed value only lives until here | borrow later used here, when `b1` is dropped -error[E0597]: `b3` does not live long enough - --> $DIR/dropck_arr_cycle_checked.rs:105:24 +error[E0597]: `b1` does not live long enough + --> $DIR/dropck_arr_cycle_checked.rs:111:24 | -LL | b1.a[1].v.set(Some(&b3)); +LL | b3.a[0].v.set(Some(&b1)); | ^^^ borrowed value does not live long enough ... LL | } diff --git a/src/test/ui/span/dropck_direct_cycle_with_drop.nll.stderr b/src/test/ui/span/dropck_direct_cycle_with_drop.nll.stderr index 86a4f0e13b512..37fffe886e309 100644 --- a/src/test/ui/span/dropck_direct_cycle_with_drop.nll.stderr +++ b/src/test/ui/span/dropck_direct_cycle_with_drop.nll.stderr @@ -1,9 +1,9 @@ -error[E0597]: `d1` does not live long enough - --> $DIR/dropck_direct_cycle_with_drop.rs:48:19 +error[E0597]: `d2` does not live long enough + --> $DIR/dropck_direct_cycle_with_drop.rs:46:19 | -LL | d2.p.set(Some(&d1)); +LL | d1.p.set(Some(&d2)); | ^^^ borrowed value does not live long enough -LL | //~^ ERROR `d1` does not live long enough +... LL | } | - | | @@ -12,12 +12,12 @@ LL | } | = note: values in a scope are dropped in the opposite order they are defined -error[E0597]: `d2` does not live long enough - --> $DIR/dropck_direct_cycle_with_drop.rs:46:19 +error[E0597]: `d1` does not live long enough + --> $DIR/dropck_direct_cycle_with_drop.rs:48:19 | -LL | d1.p.set(Some(&d2)); +LL | d2.p.set(Some(&d1)); | ^^^ borrowed value does not live long enough -... +LL | //~^ ERROR `d1` does not live long enough LL | } | - | | diff --git a/src/test/ui/span/dropck_vec_cycle_checked.nll.stderr b/src/test/ui/span/dropck_vec_cycle_checked.nll.stderr index b7f8b85f46cee..13bd1f5419821 100644 --- a/src/test/ui/span/dropck_vec_cycle_checked.nll.stderr +++ b/src/test/ui/span/dropck_vec_cycle_checked.nll.stderr @@ -1,7 +1,7 @@ -error[E0597]: `c1` does not live long enough - --> $DIR/dropck_vec_cycle_checked.rs:121:24 +error[E0597]: `c3` does not live long enough + --> $DIR/dropck_vec_cycle_checked.rs:115:24 | -LL | c3.v[0].v.set(Some(&c1)); +LL | c1.v[1].v.set(Some(&c3)); | ^^^ borrowed value does not live long enough ... LL | } @@ -22,10 +22,10 @@ LL | } | borrowed value only lives until here | borrow later used here, when `c1` is dropped -error[E0597]: `c3` does not live long enough - --> $DIR/dropck_vec_cycle_checked.rs:115:24 +error[E0597]: `c1` does not live long enough + --> $DIR/dropck_vec_cycle_checked.rs:121:24 | -LL | c1.v[1].v.set(Some(&c3)); +LL | c3.v[0].v.set(Some(&c1)); | ^^^ borrowed value does not live long enough ... LL | } diff --git a/src/test/ui/span/vec-must-not-hide-type-from-dropck.nll.stderr b/src/test/ui/span/vec-must-not-hide-type-from-dropck.nll.stderr index 389adb231c422..100b4c1292f44 100644 --- a/src/test/ui/span/vec-must-not-hide-type-from-dropck.nll.stderr +++ b/src/test/ui/span/vec-must-not-hide-type-from-dropck.nll.stderr @@ -1,21 +1,21 @@ -error[E0597]: `c1` does not live long enough - --> $DIR/vec-must-not-hide-type-from-dropck.rs:129:24 +error[E0597]: `c2` does not live long enough + --> $DIR/vec-must-not-hide-type-from-dropck.rs:127:24 | -LL | c2.v[0].v.set(Some(&c1)); +LL | c1.v[0].v.set(Some(&c2)); | ^^^ borrowed value does not live long enough -LL | //~^ ERROR `c1` does not live long enough +... LL | } | - | | | borrowed value only lives until here | borrow later used here, when `c1` is dropped -error[E0597]: `c2` does not live long enough - --> $DIR/vec-must-not-hide-type-from-dropck.rs:127:24 +error[E0597]: `c1` does not live long enough + --> $DIR/vec-must-not-hide-type-from-dropck.rs:129:24 | -LL | c1.v[0].v.set(Some(&c2)); +LL | c2.v[0].v.set(Some(&c1)); | ^^^ borrowed value does not live long enough -... +LL | //~^ ERROR `c1` does not live long enough LL | } | - | | From 25266c18409858ac0c482bc00a6d72c0f83f3df2 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 5 Jul 2018 15:14:34 -0300 Subject: [PATCH 07/17] Do not run AST borrowck when -Zborrowck=mir --- src/librustc/ty/context.rs | 6 ++++++ src/librustc_borrowck/borrowck/mod.rs | 2 ++ src/librustc_driver/driver.rs | 6 +++++- src/librustc_mir/transform/mod.rs | 5 ++++- src/test/ui/error-codes/E0017.nll.stderr | 12 ++++++------ src/test/ui/error-codes/E0388.nll.stderr | 12 ++++++------ 6 files changed, 29 insertions(+), 14 deletions(-) diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 5142a30ae574f..8f7f9d09423f6 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -1354,6 +1354,12 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { !self.sess.opts.debugging_opts.disable_ast_check_for_mutation_in_guard } + /// If true, we should use the AST-based borrowck (we may *also* use + /// the MIR-based borrowck). + pub fn use_ast_borrowck(self) -> bool { + self.borrowck_mode().use_ast() + } + /// If true, we should use the MIR-based borrowck (we may *also* use /// the AST-based borrowck). pub fn use_mir_borrowck(self) -> bool { diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index 4bd8e6afa76e3..df1b1138f3e88 100644 --- a/src/librustc_borrowck/borrowck/mod.rs +++ b/src/librustc_borrowck/borrowck/mod.rs @@ -89,6 +89,8 @@ pub struct AnalysisData<'a, 'tcx: 'a> { fn borrowck<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, owner_def_id: DefId) -> Lrc { + assert!(tcx.use_ast_borrowck()); + debug!("borrowck(body_owner_def_id={:?})", owner_def_id); let owner_id = tcx.hir.as_local_node_id(owner_def_id).unwrap(); diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index feeac9d938b6a..96e9616699d37 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -1279,7 +1279,11 @@ where middle::liveness::check_crate(tcx) }); - time(sess, "borrow checking", || borrowck::check_crate(tcx)); + time(sess, "borrow checking", || { + if tcx.use_ast_borrowck() { + borrowck::check_crate(tcx); + } + }); time(sess, "MIR borrow checking", diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index 06be2bb3734f4..90dfebeef1b0c 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -225,7 +225,10 @@ fn optimized_mir<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx // (Mir-)Borrowck uses `mir_validated`, so we have to force it to // execute before we can steal. let _ = tcx.mir_borrowck(def_id); - let _ = tcx.borrowck(def_id); + + if tcx.use_ast_borrowck() { + let _ = tcx.borrowck(def_id); + } let mut mir = tcx.mir_validated(def_id).steal(); run_passes![tcx, mir, def_id, 2; diff --git a/src/test/ui/error-codes/E0017.nll.stderr b/src/test/ui/error-codes/E0017.nll.stderr index ec31f5d05d787..addbbf4434f4f 100644 --- a/src/test/ui/error-codes/E0017.nll.stderr +++ b/src/test/ui/error-codes/E0017.nll.stderr @@ -10,18 +10,18 @@ error[E0017]: references in statics may only refer to immutable values LL | static STATIC_REF: &'static mut i32 = &mut X; //~ ERROR E0017 | ^^^^^^ statics require immutable values -error[E0017]: references in statics may only refer to immutable values - --> $DIR/E0017.rs:17:38 - | -LL | static CONST_REF: &'static mut i32 = &mut C; //~ ERROR E0017 - | ^^^^^^ statics require immutable values - error[E0596]: cannot borrow immutable item `X` as mutable --> $DIR/E0017.rs:15:39 | LL | static STATIC_REF: &'static mut i32 = &mut X; //~ ERROR E0017 | ^^^^^^ cannot borrow as mutable +error[E0017]: references in statics may only refer to immutable values + --> $DIR/E0017.rs:17:38 + | +LL | static CONST_REF: &'static mut i32 = &mut C; //~ ERROR E0017 + | ^^^^^^ statics require immutable values + error: aborting due to 4 previous errors Some errors occurred: E0017, E0596. diff --git a/src/test/ui/error-codes/E0388.nll.stderr b/src/test/ui/error-codes/E0388.nll.stderr index 6a4bd6b31a116..0238ca6b623da 100644 --- a/src/test/ui/error-codes/E0388.nll.stderr +++ b/src/test/ui/error-codes/E0388.nll.stderr @@ -10,18 +10,18 @@ error[E0017]: references in statics may only refer to immutable values LL | static STATIC_REF: &'static mut i32 = &mut X; //~ ERROR E0017 | ^^^^^^ statics require immutable values -error[E0017]: references in statics may only refer to immutable values - --> $DIR/E0388.rs:17:38 - | -LL | static CONST_REF: &'static mut i32 = &mut C; //~ ERROR E0017 - | ^^^^^^ statics require immutable values - error[E0596]: cannot borrow immutable item `X` as mutable --> $DIR/E0388.rs:15:39 | LL | static STATIC_REF: &'static mut i32 = &mut X; //~ ERROR E0017 | ^^^^^^ cannot borrow as mutable +error[E0017]: references in statics may only refer to immutable values + --> $DIR/E0388.rs:17:38 + | +LL | static CONST_REF: &'static mut i32 = &mut C; //~ ERROR E0017 + | ^^^^^^ statics require immutable values + error: aborting due to 4 previous errors Some errors occurred: E0017, E0596. From bbfdea28cd2b96f25f17b51dd7d31ca0611c92cc Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Thu, 5 Jul 2018 21:52:07 -0700 Subject: [PATCH 08/17] =?UTF-8?q?typo-fix=20stable=20ed'n=20error:=20"an?= =?UTF-8?q?=20onlyavailable"=20=E2=86=92=20"and=20only=20available"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/librustc/session/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 074cab3b5d251..e5f4262cbbd78 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -1810,7 +1810,7 @@ pub fn build_session_options_and_crate_config( early_error( ErrorOutputType::default(), &format!( - "Edition {} is unstable an only\ + "Edition {} is unstable and only \ available for nightly builds of rustc.", edition, ) From 7fbc3895e359b9212bc0a78692198f15bbe462b5 Mon Sep 17 00:00:00 2001 From: Thayne McCombs Date: Fri, 6 Jul 2018 01:00:40 -0600 Subject: [PATCH 09/17] Stabilize rc_downcast Fixes #44608 --- src/liballoc/rc.rs | 3 +-- src/liballoc/sync.rs | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index 32d624e8fbc79..357bc8ba2dd21 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -620,13 +620,12 @@ impl Rc { impl Rc { #[inline] - #[unstable(feature = "rc_downcast", issue = "44608")] + #[stable(feature = "rc_downcast", since = "1.29.0")] /// Attempt to downcast the `Rc` to a concrete type. /// /// # Examples /// /// ``` - /// #![feature(rc_downcast)] /// use std::any::Any; /// use std::rc::Rc; /// diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index 2abd9c85c5754..dcc3560a0709a 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -981,13 +981,12 @@ unsafe impl<#[may_dangle] T: ?Sized> Drop for Arc { impl Arc { #[inline] - #[unstable(feature = "rc_downcast", issue = "44608")] + #[stable(feature = "rc_downcast", since = "1.29.0")] /// Attempt to downcast the `Arc` to a concrete type. /// /// # Examples /// /// ``` - /// #![feature(rc_downcast)] /// use std::any::Any; /// use std::sync::Arc; /// From 15a196ec3684bfe03c9db219e17b064f4e65647e Mon Sep 17 00:00:00 2001 From: Thayne McCombs Date: Fri, 6 Jul 2018 01:06:17 -0600 Subject: [PATCH 10/17] Remove unnecessary feature gate. To fix a warning. --- src/libcore/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libcore/lib.rs b/src/libcore/lib.rs index b2b38820a89cc..bbe6ae8619fec 100644 --- a/src/libcore/lib.rs +++ b/src/libcore/lib.rs @@ -122,7 +122,6 @@ #![feature(const_slice_len)] #![feature(const_str_as_bytes)] #![feature(const_str_len)] -#![cfg_attr(stage0, feature(repr_transparent))] #[prelude_import] #[allow(unused)] From 933c29944392e20295f3a70e9c74f6b79d352cf2 Mon Sep 17 00:00:00 2001 From: Bastien Orivel Date: Fri, 6 Jul 2018 22:52:40 +0200 Subject: [PATCH 11/17] Dedupe filetime --- src/Cargo.lock | 23 ++++++----------------- src/bootstrap/Cargo.toml | 2 +- src/tools/compiletest/Cargo.toml | 2 +- 3 files changed, 8 insertions(+), 19 deletions(-) diff --git a/src/Cargo.lock b/src/Cargo.lock index bc1b2a14f6a64..7297d8a8299a8 100644 --- a/src/Cargo.lock +++ b/src/Cargo.lock @@ -147,7 +147,7 @@ dependencies = [ "build_helper 0.1.0", "cc 1.0.17 (registry+https://github.com/rust-lang/crates.io-index)", "cmake 0.1.31 (registry+https://github.com/rust-lang/crates.io-index)", - "filetime 0.1.15 (registry+https://github.com/rust-lang/crates.io-index)", + "filetime 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "getopts 0.2.17 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.42 (registry+https://github.com/rust-lang/crates.io-index)", @@ -416,7 +416,7 @@ version = "0.0.0" dependencies = [ "diff 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.5.10 (registry+https://github.com/rust-lang/crates.io-index)", - "filetime 0.1.15 (registry+https://github.com/rust-lang/crates.io-index)", + "filetime 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "getopts 0.2.17 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.42 (registry+https://github.com/rust-lang/crates.io-index)", @@ -693,16 +693,6 @@ dependencies = [ name = "features" version = "0.1.0" -[[package]] -name = "filetime" -version = "0.1.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -dependencies = [ - "cfg-if 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)", - "libc 0.2.42 (registry+https://github.com/rust-lang/crates.io-index)", - "redox_syscall 0.1.40 (registry+https://github.com/rust-lang/crates.io-index)", -] - [[package]] name = "filetime" version = "0.2.1" @@ -1139,11 +1129,11 @@ dependencies = [ [[package]] name = "lzma-sys" -version = "0.1.9" +version = "0.1.10" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "cc 1.0.17 (registry+https://github.com/rust-lang/crates.io-index)", - "filetime 0.1.15 (registry+https://github.com/rust-lang/crates.io-index)", + "filetime 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.42 (registry+https://github.com/rust-lang/crates.io-index)", "pkg-config 0.3.11 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -3075,7 +3065,7 @@ name = "xz2" version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ - "lzma-sys 0.1.9 (registry+https://github.com/rust-lang/crates.io-index)", + "lzma-sys 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -3134,7 +3124,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum error-chain 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ff511d5dc435d703f4971bc399647c9bc38e20cb41452e3b9feb4765419ed3f3" "checksum failure 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "934799b6c1de475a012a02dab0ace1ace43789ee4b99bcfbf1a2e3e8ced5de82" "checksum failure_derive 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "c7cdda555bb90c9bb67a3b670a0f42de8e73f5981524123ad8578aafec8ddb8b" -"checksum filetime 0.1.15 (registry+https://github.com/rust-lang/crates.io-index)" = "714653f3e34871534de23771ac7b26e999651a0a228f47beb324dfdf1dd4b10f" "checksum filetime 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "da4b9849e77b13195302c174324b5ba73eec9b236b24c221a61000daefb95c5f" "checksum fixedbitset 0.1.9 (registry+https://github.com/rust-lang/crates.io-index)" = "86d4de0081402f5e88cdac65c8dcdcc73118c1a7a465e2a05f0da05843a8ea33" "checksum flate2 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "9fac2277e84e5e858483756647a9d0aa8d9a2b7cba517fd84325a0aaa69a0909" @@ -3179,7 +3168,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum log 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)" = "e19e8d5c34a3e0e2223db8e060f9e8264aeeb5c5fc64a4ee9965c062211c024b" "checksum log 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)" = "61bd98ae7f7b754bc53dca7d44b604f733c6bba044ea6f41bc8d89272d8161d2" "checksum log_settings 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "19af41f0565d7c19b2058153ad0b42d4d5ce89ec4dbf06ed6741114a8b63e7cd" -"checksum lzma-sys 0.1.9 (registry+https://github.com/rust-lang/crates.io-index)" = "c1b93b78f89e8737dac81837fc8f5521ac162abcba902e1a3db949d55346d1da" +"checksum lzma-sys 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)" = "d1eaa027402541975218bb0eec67d6b0412f6233af96e0d096d31dbdfd22e614" "checksum mac 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4" "checksum maplit 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "08cbb6b4fef96b6d77bfc40ec491b1690c779e77b05cd9f07f787ed376fd4c43" "checksum markup5ever 0.7.2 (registry+https://github.com/rust-lang/crates.io-index)" = "bfedc97d5a503e96816d10fedcd5b42f760b2e525ce2f7ec71f6a41780548475" diff --git a/src/bootstrap/Cargo.toml b/src/bootstrap/Cargo.toml index af33ebf3c4250..57a526038041e 100644 --- a/src/bootstrap/Cargo.toml +++ b/src/bootstrap/Cargo.toml @@ -36,7 +36,7 @@ test = false [dependencies] build_helper = { path = "../build_helper" } cmake = "0.1.23" -filetime = "0.1" +filetime = "0.2" num_cpus = "1.0" getopts = "0.2" cc = "1.0.1" diff --git a/src/tools/compiletest/Cargo.toml b/src/tools/compiletest/Cargo.toml index 780387580232a..582ba5adfe529 100644 --- a/src/tools/compiletest/Cargo.toml +++ b/src/tools/compiletest/Cargo.toml @@ -6,7 +6,7 @@ version = "0.0.0" [dependencies] diff = "0.1.10" env_logger = { version = "0.5", default-features = false } -filetime = "0.1" +filetime = "0.2" getopts = "0.2" log = "0.4" regex = "0.2" From b7047bb89fca909779d06ef8f3e74ff1ef39e8a8 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Fri, 6 Jul 2018 17:12:03 -0500 Subject: [PATCH 12/17] ARM: expose the "mclass" target feature --- src/librustc_codegen_llvm/llvm_util.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/librustc_codegen_llvm/llvm_util.rs b/src/librustc_codegen_llvm/llvm_util.rs index 357b639e78890..87ee9ef5adb23 100644 --- a/src/librustc_codegen_llvm/llvm_util.rs +++ b/src/librustc_codegen_llvm/llvm_util.rs @@ -84,6 +84,7 @@ unsafe fn configure_llvm(sess: &Session) { // array, leading to crashes. const ARM_WHITELIST: &[(&str, Option<&str>)] = &[ + ("mclass", Some("arm_target_feature")), ("neon", Some("arm_target_feature")), ("v7", Some("arm_target_feature")), ("vfp2", Some("arm_target_feature")), From 6e2c49ff0e61e13aa3381eefba7923672a3c085f Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 29 Jun 2018 15:43:34 +0200 Subject: [PATCH 13/17] Use an aligned dangling pointer in Weak::new, rather than address 1 --- src/liballoc/sync.rs | 50 +++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index 4244b09b18f9c..abc0befeb947b 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -43,9 +43,6 @@ use vec::Vec; /// necessarily) at _exactly_ `MAX_REFCOUNT + 1` references. const MAX_REFCOUNT: usize = (isize::MAX) as usize; -/// A sentinel value that is used for the pointer of `Weak::new()`. -const WEAK_EMPTY: usize = 1; - /// A thread-safe reference-counting pointer. 'Arc' stands for 'Atomically /// Reference Counted'. /// @@ -239,9 +236,9 @@ impl, U: ?Sized> CoerceUnsized> for Arc {} #[stable(feature = "arc_weak", since = "1.4.0")] pub struct Weak { // This is a `NonNull` to allow optimizing the size of this type in enums, - // but it is actually not truly "non-null". A `Weak::new()` will set this - // to a sentinel value, instead of needing to allocate some space in the - // heap. + // but it is not necessarily a valid pointer. + // `Weak::new` sets this to a dangling pointer so that it doesn’t need + // to allocate space on the heap. ptr: NonNull>, } @@ -1035,14 +1032,18 @@ impl Weak { /// ``` #[stable(feature = "downgraded_weak", since = "1.10.0")] pub fn new() -> Weak { - unsafe { - Weak { - ptr: NonNull::new_unchecked(WEAK_EMPTY as *mut _), - } + Weak { + ptr: NonNull::dangling(), } } } +fn is_dangling(ptr: NonNull) -> bool { + let address = ptr.as_ptr() as *mut () as usize; + let align = align_of_val(unsafe { ptr.as_ref() }); + address == align +} + impl Weak { /// Attempts to upgrade the `Weak` pointer to an [`Arc`], extending /// the lifetime of the value if successful. @@ -1074,11 +1075,7 @@ impl Weak { pub fn upgrade(&self) -> Option> { // We use a CAS loop to increment the strong count instead of a // fetch_add because once the count hits 0 it must never be above 0. - let inner = if self.ptr.as_ptr() as *const u8 as usize == WEAK_EMPTY { - return None; - } else { - unsafe { self.ptr.as_ref() } - }; + let inner = self.inner()?; // Relaxed load because any write of 0 that we can observe // leaves the field in a permanently zero state (so a @@ -1109,6 +1106,17 @@ impl Weak { } } } + + /// Return `None` when the pointer is dangling and there is no allocated `ArcInner`, + /// i.e. this `Weak` was created by `Weak::new` + #[inline] + fn inner(&self) -> Option<&ArcInner> { + if is_dangling(self.ptr) { + None + } else { + Some(unsafe { self.ptr.as_ref() }) + } + } } #[stable(feature = "arc_weak", since = "1.4.0")] @@ -1126,10 +1134,10 @@ impl Clone for Weak { /// ``` #[inline] fn clone(&self) -> Weak { - let inner = if self.ptr.as_ptr() as *const u8 as usize == WEAK_EMPTY { - return Weak { ptr: self.ptr }; + let inner = if let Some(inner) = self.inner() { + inner } else { - unsafe { self.ptr.as_ref() } + return Weak { ptr: self.ptr }; }; // See comments in Arc::clone() for why this is relaxed. This can use a // fetch_add (ignoring the lock) because the weak count is only locked @@ -1204,10 +1212,10 @@ impl Drop for Weak { // weak count can only be locked if there was precisely one weak ref, // meaning that drop could only subsequently run ON that remaining weak // ref, which can only happen after the lock is released. - let inner = if self.ptr.as_ptr() as *const u8 as usize == WEAK_EMPTY { - return; + let inner = if let Some(inner) = self.inner() { + inner } else { - unsafe { self.ptr.as_ref() } + return }; if inner.weak.fetch_sub(1, Release) == 1 { From 41730b7e2e2c28a13fe6d08a7ad47d31d68eccea Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 27 Jun 2018 18:09:21 +0200 Subject: [PATCH 14/17] Rc: remove unused allocation from Weak::new() Same as https://github.com/rust-lang/rust/pull/50357 --- src/liballoc/rc.rs | 59 +++++++++++++++++++++++++++----------------- src/liballoc/sync.rs | 2 +- 2 files changed, 37 insertions(+), 24 deletions(-) diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index 32d624e8fbc79..3f32abe1ea959 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -253,7 +253,7 @@ use core::hash::{Hash, Hasher}; use core::intrinsics::abort; use core::marker; use core::marker::{Unsize, PhantomData}; -use core::mem::{self, align_of_val, forget, size_of_val, uninitialized}; +use core::mem::{self, align_of_val, forget, size_of_val}; use core::ops::Deref; use core::ops::CoerceUnsized; use core::ptr::{self, NonNull}; @@ -261,6 +261,7 @@ use core::convert::From; use alloc::{Global, Alloc, Layout, box_free, handle_alloc_error}; use string::String; +use sync::is_dangling; use vec::Vec; struct RcBox { @@ -1153,6 +1154,10 @@ impl From> for Rc<[T]> { /// [`None`]: ../../std/option/enum.Option.html#variant.None #[stable(feature = "rc_weak", since = "1.4.0")] pub struct Weak { + // This is a `NonNull` to allow optimizing the size of this type in enums, + // but it is not necessarily a valid pointer. + // `Weak::new` sets this to a dangling pointer so that it doesn’t need + // to allocate space on the heap. ptr: NonNull>, } @@ -1165,8 +1170,8 @@ impl !marker::Sync for Weak {} impl, U: ?Sized> CoerceUnsized> for Weak {} impl Weak { - /// Constructs a new `Weak`, allocating memory for `T` without initializing - /// it. Calling [`upgrade`] on the return value always gives [`None`]. + /// Constructs a new `Weak`, without allocating any memory. + /// Calling [`upgrade`] on the return value always gives [`None`]. /// /// [`upgrade`]: struct.Weak.html#method.upgrade /// [`None`]: ../../std/option/enum.Option.html @@ -1181,14 +1186,8 @@ impl Weak { /// ``` #[stable(feature = "downgraded_weak", since = "1.10.0")] pub fn new() -> Weak { - unsafe { - Weak { - ptr: Box::into_raw_non_null(box RcBox { - strong: Cell::new(0), - weak: Cell::new(1), - value: uninitialized(), - }), - } + Weak { + ptr: NonNull::dangling(), } } } @@ -1222,13 +1221,25 @@ impl Weak { /// ``` #[stable(feature = "rc_weak", since = "1.4.0")] pub fn upgrade(&self) -> Option> { - if self.strong() == 0 { + let inner = self.inner()?; + if inner.strong() == 0 { None } else { - self.inc_strong(); + inner.inc_strong(); Some(Rc { ptr: self.ptr, phantom: PhantomData }) } } + + /// Return `None` when the pointer is dangling and there is no allocated `RcBox`, + /// i.e. this `Weak` was created by `Weak::new` + #[inline] + fn inner(&self) -> Option<&RcBox> { + if is_dangling(self.ptr) { + None + } else { + Some(unsafe { self.ptr.as_ref() }) + } + } } #[stable(feature = "rc_weak", since = "1.4.0")] @@ -1258,12 +1269,14 @@ impl Drop for Weak { /// assert!(other_weak_foo.upgrade().is_none()); /// ``` fn drop(&mut self) { - unsafe { - self.dec_weak(); + if let Some(inner) = self.inner() { + inner.dec_weak(); // the weak count starts at 1, and will only go to zero if all // the strong pointers have disappeared. - if self.weak() == 0 { - Global.dealloc(self.ptr.cast(), Layout::for_value(self.ptr.as_ref())); + if inner.weak() == 0 { + unsafe { + Global.dealloc(self.ptr.cast(), Layout::for_value(self.ptr.as_ref())); + } } } } @@ -1284,7 +1297,9 @@ impl Clone for Weak { /// ``` #[inline] fn clone(&self) -> Weak { - self.inc_weak(); + if let Some(inner) = self.inner() { + inner.inc_weak() + } Weak { ptr: self.ptr } } } @@ -1317,7 +1332,7 @@ impl Default for Weak { } } -// NOTE: We checked_add here to deal with mem::forget safety. In particular +// NOTE: We checked_add here to deal with mem::forget safely. In particular // if you mem::forget Rcs (or Weaks), the ref-count can overflow, and then // you can free the allocation while outstanding Rcs (or Weaks) exist. // We abort because this is such a degenerate scenario that we don't care about @@ -1370,12 +1385,10 @@ impl RcBoxPtr for Rc { } } -impl RcBoxPtr for Weak { +impl RcBoxPtr for RcBox { #[inline(always)] fn inner(&self) -> &RcBox { - unsafe { - self.ptr.as_ref() - } + self } } diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index abc0befeb947b..ea8616bf1d05c 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -1038,7 +1038,7 @@ impl Weak { } } -fn is_dangling(ptr: NonNull) -> bool { +pub(crate) fn is_dangling(ptr: NonNull) -> bool { let address = ptr.as_ptr() as *mut () as usize; let align = align_of_val(unsafe { ptr.as_ref() }); address == align From 21526c5403d7a43144de897d0187c395fb92bacc Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 27 Jun 2018 21:36:29 +0200 Subject: [PATCH 15/17] Add a test for Weak::new() not crashing for uninhabited types --- .../run-pass/weak-new-uninhabited-issue-48493.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 src/test/run-pass/weak-new-uninhabited-issue-48493.rs diff --git a/src/test/run-pass/weak-new-uninhabited-issue-48493.rs b/src/test/run-pass/weak-new-uninhabited-issue-48493.rs new file mode 100644 index 0000000000000..eb57c12ed12c6 --- /dev/null +++ b/src/test/run-pass/weak-new-uninhabited-issue-48493.rs @@ -0,0 +1,15 @@ +// Copyright 2018 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +fn main() { + enum Void {} + std::rc::Weak::::new(); + std::sync::Weak::::new(); +} From 5717d99d1b3b76ec7814c95dfcc0399ab4ddaa83 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 6 Jul 2018 19:30:09 +0200 Subject: [PATCH 16/17] Add some unit tests for dangling Weak references --- src/liballoc/tests/arc.rs | 55 +++++++++++++++++++++++++++++++++++++++ src/liballoc/tests/lib.rs | 2 ++ src/liballoc/tests/rc.rs | 55 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+) create mode 100644 src/liballoc/tests/arc.rs create mode 100644 src/liballoc/tests/rc.rs diff --git a/src/liballoc/tests/arc.rs b/src/liballoc/tests/arc.rs new file mode 100644 index 0000000000000..753873dd294ce --- /dev/null +++ b/src/liballoc/tests/arc.rs @@ -0,0 +1,55 @@ +// Copyright 2018 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use std::any::Any; +use std::sync::{Arc, Weak}; + +#[test] +fn uninhabited() { + enum Void {} + let mut a = Weak::::new(); + a = a.clone(); + assert!(a.upgrade().is_none()); + + let mut a: Weak = a; // Unsizing + a = a.clone(); + assert!(a.upgrade().is_none()); +} + +#[test] +fn slice() { + let a: Arc<[u32; 3]> = Arc::new([3, 2, 1]); + let a: Arc<[u32]> = a; // Unsizing + let b: Arc<[u32]> = Arc::from(&[3, 2, 1][..]); // Conversion + assert_eq!(a, b); + + // Exercise is_dangling() with a DST + let mut a = Arc::downgrade(&a); + a = a.clone(); + assert!(a.upgrade().is_some()); +} + +#[test] +fn trait_object() { + let a: Arc = Arc::new(4); + let a: Arc = a; // Unsizing + + // Exercise is_dangling() with a DST + let mut a = Arc::downgrade(&a); + a = a.clone(); + assert!(a.upgrade().is_some()); + + let mut b = Weak::::new(); + b = b.clone(); + assert!(b.upgrade().is_none()); + let mut b: Weak = b; // Unsizing + b = b.clone(); + assert!(b.upgrade().is_none()); +} diff --git a/src/liballoc/tests/lib.rs b/src/liballoc/tests/lib.rs index dbac2c0bb18a6..2c361598e8928 100644 --- a/src/liballoc/tests/lib.rs +++ b/src/liballoc/tests/lib.rs @@ -32,12 +32,14 @@ extern crate rand; use std::hash::{Hash, Hasher}; use std::collections::hash_map::DefaultHasher; +mod arc; mod binary_heap; mod btree; mod cow_str; mod fmt; mod heap; mod linked_list; +mod rc; mod slice; mod str; mod string; diff --git a/src/liballoc/tests/rc.rs b/src/liballoc/tests/rc.rs new file mode 100644 index 0000000000000..baa0406acfc3d --- /dev/null +++ b/src/liballoc/tests/rc.rs @@ -0,0 +1,55 @@ +// Copyright 2018 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use std::any::Any; +use std::rc::{Rc, Weak}; + +#[test] +fn uninhabited() { + enum Void {} + let mut a = Weak::::new(); + a = a.clone(); + assert!(a.upgrade().is_none()); + + let mut a: Weak = a; // Unsizing + a = a.clone(); + assert!(a.upgrade().is_none()); +} + +#[test] +fn slice() { + let a: Rc<[u32; 3]> = Rc::new([3, 2, 1]); + let a: Rc<[u32]> = a; // Unsizing + let b: Rc<[u32]> = Rc::from(&[3, 2, 1][..]); // Conversion + assert_eq!(a, b); + + // Exercise is_dangling() with a DST + let mut a = Rc::downgrade(&a); + a = a.clone(); + assert!(a.upgrade().is_some()); +} + +#[test] +fn trait_object() { + let a: Rc = Rc::new(4); + let a: Rc = a; // Unsizing + + // Exercise is_dangling() with a DST + let mut a = Rc::downgrade(&a); + a = a.clone(); + assert!(a.upgrade().is_some()); + + let mut b = Weak::::new(); + b = b.clone(); + assert!(b.upgrade().is_none()); + let mut b: Weak = b; // Unsizing + b = b.clone(); + assert!(b.upgrade().is_none()); +} From 67202b8b68d93c90e89ca001945865f13bc97154 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Sat, 7 Jul 2018 01:44:57 +0200 Subject: [PATCH 17/17] =?UTF-8?q?Fix=20is=5Fdangling=20import=20when=20Arc?= =?UTF-8?q?=20is=20#[cfg]=E2=80=99ed=20out?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/liballoc/rc.rs | 7 ++++++- src/liballoc/sync.rs | 7 +------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index 3f32abe1ea959..5b71d0b85a03e 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -261,7 +261,6 @@ use core::convert::From; use alloc::{Global, Alloc, Layout, box_free, handle_alloc_error}; use string::String; -use sync::is_dangling; use vec::Vec; struct RcBox { @@ -1192,6 +1191,12 @@ impl Weak { } } +pub(crate) fn is_dangling(ptr: NonNull) -> bool { + let address = ptr.as_ptr() as *mut () as usize; + let align = align_of_val(unsafe { ptr.as_ref() }); + address == align +} + impl Weak { /// Attempts to upgrade the `Weak` pointer to an [`Rc`], extending /// the lifetime of the value if successful. diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index ea8616bf1d05c..6710878b31d94 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -34,6 +34,7 @@ use core::convert::From; use alloc::{Global, Alloc, Layout, box_free, handle_alloc_error}; use boxed::Box; +use rc::is_dangling; use string::String; use vec::Vec; @@ -1038,12 +1039,6 @@ impl Weak { } } -pub(crate) fn is_dangling(ptr: NonNull) -> bool { - let address = ptr.as_ptr() as *mut () as usize; - let align = align_of_val(unsafe { ptr.as_ref() }); - address == align -} - impl Weak { /// Attempts to upgrade the `Weak` pointer to an [`Arc`], extending /// the lifetime of the value if successful.