From fcf4bee7d3d69356a9eb702f9e0a9ddba238d33f Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Fri, 22 Nov 2019 10:45:49 -0800 Subject: [PATCH 1/2] Fix tests for RFC 2203 in a `const` The previous test was incorrect. `const fn`s are *always* promotable when inside a `const`, so it should pass. The error was caused by a bug in `promote_consts`. I've added a failing test outside a `const` alongside the existing one, which is now run-pass. --- .../const-fns.rs | 26 ------------------- .../fn-call-in-const.rs | 23 ++++++++++++++++ .../fn-call-in-non-const.rs | 18 +++++++++++++ ...fns.stderr => fn-call-in-non-const.stderr} | 6 ++--- 4 files changed, 44 insertions(+), 29 deletions(-) delete mode 100644 src/test/ui/consts/rfc-2203-const-array-repeat-exprs/const-fns.rs create mode 100644 src/test/ui/consts/rfc-2203-const-array-repeat-exprs/fn-call-in-const.rs create mode 100644 src/test/ui/consts/rfc-2203-const-array-repeat-exprs/fn-call-in-non-const.rs rename src/test/ui/consts/rfc-2203-const-array-repeat-exprs/{const-fns.stderr => fn-call-in-non-const.stderr} (63%) diff --git a/src/test/ui/consts/rfc-2203-const-array-repeat-exprs/const-fns.rs b/src/test/ui/consts/rfc-2203-const-array-repeat-exprs/const-fns.rs deleted file mode 100644 index 68a9227dea96e..0000000000000 --- a/src/test/ui/consts/rfc-2203-const-array-repeat-exprs/const-fns.rs +++ /dev/null @@ -1,26 +0,0 @@ -// ignore-tidy-linelength -// ignore-compare-mode-nll -#![feature(const_in_array_repeat_expressions, nll)] -#![allow(warnings)] - -// Some type that is not copyable. -struct Bar; - -const fn type_no_copy() -> Option { - None -} - -const fn type_copy() -> u32 { - 3 -} - -fn no_copy() { - const ARR: [Option; 2] = [type_no_copy(); 2]; - //~^ ERROR the trait bound `std::option::Option: std::marker::Copy` is not satisfied [E0277] -} - -fn copy() { - const ARR: [u32; 2] = [type_copy(); 2]; -} - -fn main() {} diff --git a/src/test/ui/consts/rfc-2203-const-array-repeat-exprs/fn-call-in-const.rs b/src/test/ui/consts/rfc-2203-const-array-repeat-exprs/fn-call-in-const.rs new file mode 100644 index 0000000000000..da1bae1be8d4e --- /dev/null +++ b/src/test/ui/consts/rfc-2203-const-array-repeat-exprs/fn-call-in-const.rs @@ -0,0 +1,23 @@ +// run-pass + +#![allow(unused)] +#![feature(const_in_array_repeat_expressions)] + +// Some type that is not copyable. +struct Bar; + +const fn type_no_copy() -> Option { + None +} + +const fn type_copy() -> u32 { + 3 +} + +const _: [u32; 2] = [type_copy(); 2]; + +// This is allowed because all promotion contexts use the explicit rules for promotability when +// inside an explicit const context. +const _: [Option; 2] = [type_no_copy(); 2]; + +fn main() {} diff --git a/src/test/ui/consts/rfc-2203-const-array-repeat-exprs/fn-call-in-non-const.rs b/src/test/ui/consts/rfc-2203-const-array-repeat-exprs/fn-call-in-non-const.rs new file mode 100644 index 0000000000000..38f744e99aab4 --- /dev/null +++ b/src/test/ui/consts/rfc-2203-const-array-repeat-exprs/fn-call-in-non-const.rs @@ -0,0 +1,18 @@ +#![feature(const_in_array_repeat_expressions)] + +// Some type that is not copyable. +struct Bar; + +const fn no_copy() -> Option { + None +} + +const fn copy() -> u32 { + 3 +} + +fn main() { + let _: [u32; 2] = [copy(); 2]; + let _: [Option; 2] = [no_copy(); 2]; + //~^ ERROR the trait bound `std::option::Option: std::marker::Copy` is not satisfied +} diff --git a/src/test/ui/consts/rfc-2203-const-array-repeat-exprs/const-fns.stderr b/src/test/ui/consts/rfc-2203-const-array-repeat-exprs/fn-call-in-non-const.stderr similarity index 63% rename from src/test/ui/consts/rfc-2203-const-array-repeat-exprs/const-fns.stderr rename to src/test/ui/consts/rfc-2203-const-array-repeat-exprs/fn-call-in-non-const.stderr index 82272af958a2e..8219d836a20e9 100644 --- a/src/test/ui/consts/rfc-2203-const-array-repeat-exprs/const-fns.stderr +++ b/src/test/ui/consts/rfc-2203-const-array-repeat-exprs/fn-call-in-non-const.stderr @@ -1,8 +1,8 @@ error[E0277]: the trait bound `std::option::Option: std::marker::Copy` is not satisfied - --> $DIR/const-fns.rs:18:35 + --> $DIR/fn-call-in-non-const.rs:16:31 | -LL | const ARR: [Option; 2] = [type_no_copy(); 2]; - | ^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `std::option::Option` +LL | let _: [Option; 2] = [no_copy(); 2]; + | ^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `std::option::Option` | = help: the following implementations were found: as std::marker::Copy> From f9ed2199ff5a69b88d43949c138042a42ecd98b5 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Fri, 22 Nov 2019 10:58:58 -0800 Subject: [PATCH 2/2] Create promoted MIR fragments in `const` and `static`s The previous strategy of removing `Drop` and `StorageDead` for promoted locals only worked for rvalue lifetime extension. We now use the same implementation for promotion across all kinds of items. --- src/librustc_mir/transform/promote_consts.rs | 104 +------------------ 1 file changed, 4 insertions(+), 100 deletions(-) diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index c79d382a37480..4e77f8945dc35 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -24,7 +24,6 @@ use syntax::symbol::sym; use syntax_pos::{Span, DUMMY_SP}; use rustc_index::vec::{IndexVec, Idx}; -use rustc_index::bit_set::HybridBitSet; use rustc_target::spec::abi::Abi; use std::cell::Cell; @@ -35,10 +34,8 @@ use crate::transform::check_consts::{qualifs, Item, ConstKind, is_lang_panic_fn} /// A `MirPass` for promotion. /// -/// In this case, "promotion" entails the following: -/// - Extract promotable temps in `fn` and `const fn` into their own MIR bodies. -/// - Extend lifetimes in `const` and `static` by removing `Drop` and `StorageDead`. -/// - Emit errors if the requirements of `#[rustc_args_required_const]` are not met. +/// Promotion is the extraction of promotable temps into separate MIR bodies. This pass also emits +/// errors when promotion of `#[rustc_args_required_const]` arguments fails. /// /// After this pass is run, `promoted_fragments` will hold the MIR body corresponding to each /// newly created `StaticKind::Promoted`. @@ -63,26 +60,13 @@ impl<'tcx> MirPass<'tcx> for PromoteTemps<'tcx> { let def_id = src.def_id(); - let item = Item::new(tcx, def_id, body); let mut rpo = traversal::reverse_postorder(body); let (temps, all_candidates) = collect_temps_and_candidates(tcx, body, &mut rpo); let promotable_candidates = validate_candidates(tcx, body, def_id, &temps, &all_candidates); - // For now, lifetime extension is done in `const` and `static`s without creating promoted - // MIR fragments by removing `Drop` and `StorageDead` for each referent. However, this will - // not work inside loops when they are allowed in `const`s. - // - // FIXME: use promoted MIR fragments everywhere? - let promoted_fragments = if should_create_promoted_mir_fragments(item.const_kind) { - promote_candidates(def_id, body, tcx, temps, promotable_candidates) - } else { - // FIXME: promote const array initializers in consts. - remove_drop_and_storage_dead_on_promoted_locals(tcx, body, &promotable_candidates); - IndexVec::new() - }; - - self.promoted_fragments.set(promoted_fragments); + let promoted = promote_candidates(def_id, body, tcx, temps, promotable_candidates); + self.promoted_fragments.set(promoted); } } @@ -1188,83 +1172,3 @@ crate fn should_suggest_const_in_array_repeat_expressions_attribute<'tcx>( should_promote={:?} feature_flag={:?}", mir_def_id, should_promote, feature_flag); should_promote && !feature_flag } - -fn should_create_promoted_mir_fragments(const_kind: Option) -> bool { - match const_kind { - Some(ConstKind::ConstFn) | None => true, - Some(ConstKind::Const) | Some(ConstKind::Static) | Some(ConstKind::StaticMut) => false, - } -} - -/// In `const` and `static` everything without `StorageDead` -/// is `'static`, we don't have to create promoted MIR fragments, -/// just remove `Drop` and `StorageDead` on "promoted" locals. -fn remove_drop_and_storage_dead_on_promoted_locals( - tcx: TyCtxt<'tcx>, - body: &mut Body<'tcx>, - promotable_candidates: &[Candidate], -) { - debug!("run_pass: promotable_candidates={:?}", promotable_candidates); - - // Removing `StorageDead` will cause errors for temps declared inside a loop body. For now we - // simply skip promotion if a loop exists, since loops are not yet allowed in a `const`. - // - // FIXME: Just create MIR fragments for `const`s instead of using this hackish approach? - if body.is_cfg_cyclic() { - tcx.sess.delay_span_bug(body.span, "Control-flow cycle detected in `const`"); - return; - } - - // The underlying local for promotion contexts like `&temp` and `&(temp.proj)`. - let mut requires_lifetime_extension = HybridBitSet::new_empty(body.local_decls.len()); - - promotable_candidates - .iter() - .filter_map(|c| { - match c { - Candidate::Ref(loc) => Some(loc), - Candidate::Repeat(_) | Candidate::Argument { .. } => None, - } - }) - .map(|&Location { block, statement_index }| { - // FIXME: store the `Local` for each `Candidate` when it is created. - let place = match &body[block].statements[statement_index].kind { - StatementKind::Assign(box ( _, Rvalue::Ref(_, _, place))) => place, - _ => bug!("`Candidate::Ref` without corresponding assignment"), - }; - - match place.base { - PlaceBase::Local(local) => local, - PlaceBase::Static(_) => bug!("`Candidate::Ref` for a non-local"), - } - }) - .for_each(|local| { - requires_lifetime_extension.insert(local); - }); - - // Remove `Drop` terminators and `StorageDead` statements for all promotable temps that require - // lifetime extension. - for block in body.basic_blocks_mut() { - block.statements.retain(|statement| { - match statement.kind { - StatementKind::StorageDead(index) => !requires_lifetime_extension.contains(index), - _ => true - } - }); - let terminator = block.terminator_mut(); - match &terminator.kind { - TerminatorKind::Drop { - location, - target, - .. - } => { - if let Some(index) = location.as_local() { - if requires_lifetime_extension.contains(index) { - terminator.kind = TerminatorKind::Goto { target: *target }; - } - } - } - _ => {} - } - } -}