Skip to content

Commit

Permalink
Auto merge of #70820 - spastorino:replace-fragile-erroneous-const-sys…
Browse files Browse the repository at this point in the history
…, r=oli-obk

Replace fragile erroneous const sys

Closes #67191

r? @oli-obk
  • Loading branch information
bors committed Apr 24, 2020
2 parents 5a59527 + 7bc45f6 commit 0b95879
Show file tree
Hide file tree
Showing 13 changed files with 133 additions and 82 deletions.
14 changes: 14 additions & 0 deletions src/librustc_codegen_ssa/mir/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use crate::base;
use crate::traits::*;
use rustc_errors::ErrorReported;
use rustc_middle::mir;
use rustc_middle::mir::interpret::ErrorHandled;
use rustc_middle::ty::layout::{FnAbiExt, HasTyCtxt, TyAndLayout};
use rustc_middle::ty::{self, Instance, Ty, TypeFoldable};
use rustc_target::abi::call::{FnAbi, PassMode};
Expand Down Expand Up @@ -189,6 +191,18 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(

fx.per_local_var_debug_info = fx.compute_per_local_var_debug_info();

for const_ in &mir.required_consts {
if let Err(err) = fx.eval_mir_constant(const_) {
match err {
// errored or at least linted
ErrorHandled::Reported(ErrorReported) | ErrorHandled::Linted => {}
ErrorHandled::TooGeneric => {
span_bug!(const_.span, "codgen encountered polymorphic constant: {:?}", err)
}
}
}
}

let memory_locals = analyze::non_ssa_locals(&fx);

// Allocate variable and temp allocas
Expand Down
8 changes: 7 additions & 1 deletion src/librustc_middle/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,10 @@ pub struct Body<'tcx> {
/// A span representing this MIR, for error reporting.
pub span: Span,

/// Constants that are required to evaluate successfully for this MIR to be well-formed.
/// We hold in this field all the constants we are not able to evaluate yet.
pub required_consts: Vec<Constant<'tcx>>,

/// The user may be writing e.g. &[(SOME_CELL, 42)][i].1 and this would get promoted, because
/// we'd statically know that no thing with interior mutability will ever be available to the
/// user without some serious unsafe code. Now this means that our promoted is actually
Expand Down Expand Up @@ -203,6 +207,7 @@ impl<'tcx> Body<'tcx> {
spread_arg: None,
var_debug_info,
span,
required_consts: Vec::new(),
ignore_interior_mut_in_const_validation: false,
control_flow_destroyed,
predecessor_cache: PredecessorCache::new(),
Expand All @@ -227,6 +232,7 @@ impl<'tcx> Body<'tcx> {
arg_count: 0,
spread_arg: None,
span: DUMMY_SP,
required_consts: Vec::new(),
control_flow_destroyed: Vec::new(),
generator_kind: None,
var_debug_info: Vec::new(),
Expand Down Expand Up @@ -2395,7 +2401,7 @@ impl<'tcx> Debug for Rvalue<'tcx> {
/// this does not necessarily mean that they are "==" in Rust -- in
/// particular one must be wary of `NaN`!
#[derive(Clone, PartialEq, RustcEncodable, RustcDecodable, HashStable)]
#[derive(Clone, Copy, PartialEq, RustcEncodable, RustcDecodable, HashStable)]
pub struct Constant<'tcx> {
pub span: Span,

Expand Down
5 changes: 5 additions & 0 deletions src/librustc_middle/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,11 @@ macro_rules! make_mir_visitor {
}

self.visit_span(&$($mutability)? body.span);

for const_ in &$($mutability)? body.required_consts {
let location = START_BLOCK.start_location();
self.visit_constant(const_, location);
}
}

fn super_basic_block_data(&mut self,
Expand Down
6 changes: 6 additions & 0 deletions src/librustc_mir/transform/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,12 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> {
self.tcx
}

fn visit_body(&mut self, body: &mut Body<'tcx>) {
for (bb, data) in body.basic_blocks_mut().iter_enumerated_mut() {
self.visit_basic_block_data(bb, data);
}
}

fn visit_constant(&mut self, constant: &mut Constant<'tcx>, location: Location) {
trace!("visit_constant: {:?}", constant);
self.super_constant(constant, location);
Expand Down
12 changes: 11 additions & 1 deletion src/librustc_mir/transform/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::mir::visit::*;
use rustc_middle::mir::*;
use rustc_middle::ty::subst::{Subst, SubstsRef};
use rustc_middle::ty::{self, Instance, InstanceDef, ParamEnv, Ty, TyCtxt};
use rustc_middle::ty::{self, ConstKind, Instance, InstanceDef, ParamEnv, Ty, TyCtxt};
use rustc_session::config::Sanitizer;
use rustc_target::spec::abi::Abi;

Expand Down Expand Up @@ -123,6 +123,16 @@ impl Inliner<'tcx> {
continue;
};

// Copy only unevaluated constants from the callee_body into the caller_body.
// Although we are only pushing `ConstKind::Unevaluated` consts to
// `required_consts`, here we may not only have `ConstKind::Unevaluated`
// because we are calling `subst_and_normalize_erasing_regions`.
caller_body.required_consts.extend(
callee_body.required_consts.iter().copied().filter(|&constant| {
matches!(constant.literal.val, ConstKind::Unevaluated(_, _, _))
}),
);

let start = caller_body.basic_blocks().len();
debug!("attempting to inline callsite {:?} - body={:?}", callsite, callee_body);
if !self.inline_call(callsite, caller_body, callee_body) {
Expand Down
13 changes: 12 additions & 1 deletion src/librustc_mir/transform/mod.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
use crate::{shim, util};
use required_consts::RequiredConstsVisitor;
use rustc_ast::ast;
use rustc_hir as hir;
use rustc_hir::def_id::{CrateNum, DefId, DefIdSet, LocalDefId, LOCAL_CRATE};
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_index::vec::IndexVec;
use rustc_middle::mir::{Body, ConstQualifs, MirPhase, Promoted};
use rustc_middle::mir::visit::Visitor as _;
use rustc_middle::mir::{traversal, Body, ConstQualifs, MirPhase, Promoted};
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::steal::Steal;
use rustc_middle::ty::{InstanceDef, TyCtxt, TypeFoldable};
Expand All @@ -29,6 +31,7 @@ pub mod no_landing_pads;
pub mod promote_consts;
pub mod qualify_min_const_fn;
pub mod remove_noop_landing_pads;
pub mod required_consts;
pub mod rustc_peek;
pub mod simplify;
pub mod simplify_branches;
Expand Down Expand Up @@ -237,6 +240,14 @@ fn mir_validated(
let _ = tcx.mir_const_qualif(def_id);

let mut body = tcx.mir_const(def_id).steal();

let mut required_consts = Vec::new();
let mut required_consts_visitor = RequiredConstsVisitor::new(&mut required_consts);
for (bb, bb_data) in traversal::reverse_postorder(&body) {
required_consts_visitor.visit_basic_block_data(bb, bb_data);
}
body.required_consts = required_consts;

let promote_pass = promote_consts::PromoteTemps::default();
run_passes(
tcx,
Expand Down
23 changes: 23 additions & 0 deletions src/librustc_mir/transform/required_consts.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::{Constant, Location};
use rustc_middle::ty::ConstKind;

pub struct RequiredConstsVisitor<'a, 'tcx> {
required_consts: &'a mut Vec<Constant<'tcx>>,
}

impl<'a, 'tcx> RequiredConstsVisitor<'a, 'tcx> {
pub fn new(required_consts: &'a mut Vec<Constant<'tcx>>) -> Self {
RequiredConstsVisitor { required_consts }
}
}

impl<'a, 'tcx> Visitor<'tcx> for RequiredConstsVisitor<'a, 'tcx> {
fn visit_constant(&mut self, constant: &Constant<'tcx>, _: Location) {
let const_kind = constant.literal.val;

if let ConstKind::Unevaluated(_, _, _) = const_kind {
self.required_consts.push(*constant);
}
}
}
35 changes: 10 additions & 25 deletions src/librustc_mir/transform/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use rustc_index::bit_set::BitSet;
use rustc_index::vec::{Idx, IndexVec};
use rustc_middle::mir::visit::{MutVisitor, MutatingUseContext, PlaceContext, Visitor};
use rustc_middle::mir::*;
use rustc_middle::ty::{self, TyCtxt};
use rustc_middle::ty::TyCtxt;
use std::borrow::Cow;

pub struct SimplifyCfg {
Expand Down Expand Up @@ -400,33 +400,18 @@ impl<'a, 'tcx> Visitor<'tcx> for DeclMarker<'a, 'tcx> {
if location.statement_index != block.statements.len() {
let stmt = &block.statements[location.statement_index];

fn can_skip_constant(c: &ty::Const<'tcx>) -> bool {
// Keep assignments from unevaluated constants around, since the
// evaluation may report errors, even if the use of the constant
// is dead code.
!matches!(c.val, ty::ConstKind::Unevaluated(..))
}

fn can_skip_operand(o: &Operand<'_>) -> bool {
match o {
Operand::Copy(_) | Operand::Move(_) => true,
Operand::Constant(c) => can_skip_constant(c.literal),
}
}

if let StatementKind::Assign(box (dest, rvalue)) = &stmt.kind {
if !dest.is_indirect() && dest.local == *local {
let can_skip = match rvalue {
Rvalue::Use(op) => can_skip_operand(op),
Rvalue::Discriminant(_) => true,
Rvalue::BinaryOp(_, l, r) | Rvalue::CheckedBinaryOp(_, l, r) => {
can_skip_operand(l) && can_skip_operand(r)
}
Rvalue::Repeat(op, c) => can_skip_operand(op) && can_skip_constant(c),
Rvalue::AddressOf(_, _) => true,
Rvalue::Len(_) => true,
Rvalue::UnaryOp(_, op) => can_skip_operand(op),
Rvalue::Aggregate(_, operands) => operands.iter().all(can_skip_operand),
Rvalue::Use(_)
| Rvalue::Discriminant(_)
| Rvalue::BinaryOp(_, _, _)
| Rvalue::CheckedBinaryOp(_, _, _)
| Rvalue::Repeat(_, _)
| Rvalue::AddressOf(_, _)
| Rvalue::Len(_)
| Rvalue::UnaryOp(_, _)
| Rvalue::Aggregate(_, _) => true,

_ => false,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,41 +30,41 @@ fn main() -> () {
}

alloc0 (static: FOO, size: 8, align: 4) {
alloc25+0╼ 03 00 00 00 │ ╾──╼....
alloc21+0╼ 03 00 00 00 │ ╾──╼....
}

alloc25 (size: 48, align: 4) {
0x00 │ 00 00 00 00 __ __ __ __ ╾alloc10+0╼ 00 00 00 00 │ ....░░░░╾──╼....
0x10 │ 00 00 00 00 __ __ __ __ ╾alloc15+0╼ 02 00 00 00 │ ....░░░░╾──╼....
0x20 │ 01 00 00 00 2a 00 00 00 ╾alloc23+0╼ 03 00 00 00 │ ....*...╾──╼....
alloc21 (size: 48, align: 4) {
0x00 │ 00 00 00 00 __ __ __ __ ╾alloc4+0─╼ 00 00 00 00 │ ....░░░░╾──╼....
0x10 │ 00 00 00 00 __ __ __ __ ╾alloc9+0─╼ 02 00 00 00 │ ....░░░░╾──╼....
0x20 │ 01 00 00 00 2a 00 00 00 ╾alloc19+0╼ 03 00 00 00 │ ....*...╾──╼....
}

alloc10 (size: 0, align: 4) {}
alloc4 (size: 0, align: 4) {}

alloc15 (size: 8, align: 4) {
alloc13+0╼ ╾alloc14+0╼ │ ╾──╼╾──╼
alloc9 (size: 8, align: 4) {
alloc7+0─╼ ╾alloc8+0─╼ │ ╾──╼╾──╼
}

alloc13 (size: 1, align: 1) {
alloc7 (size: 1, align: 1) {
05 │ .
}

alloc14 (size: 1, align: 1) {
alloc8 (size: 1, align: 1) {
06 │ .
}

alloc23 (size: 12, align: 4) {
alloc19+3╼ ╾alloc20+0╼ ╾alloc22+2╼ │ ╾──╼╾──╼╾──╼
alloc19 (size: 12, align: 4) {
alloc15+3╼ ╾alloc16+0╼ ╾alloc18+2╼ │ ╾──╼╾──╼╾──╼
}

alloc19 (size: 4, align: 1) {
alloc15 (size: 4, align: 1) {
2a 45 15 6f │ *E.o
}

alloc20 (size: 1, align: 1) {
alloc16 (size: 1, align: 1) {
2a │ *
}

alloc22 (size: 4, align: 1) {
alloc18 (size: 4, align: 1) {
2a 45 15 6f │ *E.o
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,44 +30,44 @@ fn main() -> () {
}

alloc0 (static: FOO, size: 16, align: 8) {
╾──────alloc25+0──────╼ 03 00 00 00 00 00 00 00 │ ╾──────╼........
╾──────alloc21+0──────╼ 03 00 00 00 00 00 00 00 │ ╾──────╼........
}

alloc25 (size: 72, align: 8) {
0x00 │ 00 00 00 00 __ __ __ __ ╾──────alloc10+0──────╼ │ ....░░░░╾──────╼
alloc21 (size: 72, align: 8) {
0x00 │ 00 00 00 00 __ __ __ __ ╾──────alloc4+0───────╼ │ ....░░░░╾──────╼
0x10 │ 00 00 00 00 00 00 00 00 00 00 00 00 __ __ __ __ │ ............░░░░
0x20 │ ╾──────alloc15+0──────╼ 02 00 00 00 00 00 00 00 │ ╾──────╼........
0x30 │ 01 00 00 00 2a 00 00 00 ╾──────alloc23+0──────╼ │ ....*...╾──────╼
0x20 │ ╾──────alloc9+0───────╼ 02 00 00 00 00 00 00 00 │ ╾──────╼........
0x30 │ 01 00 00 00 2a 00 00 00 ╾──────alloc19+0──────╼ │ ....*...╾──────╼
0x40 │ 03 00 00 00 00 00 00 00 │ ........
}

alloc10 (size: 0, align: 8) {}
alloc4 (size: 0, align: 8) {}

alloc15 (size: 16, align: 8) {
╾──────alloc13+0──────╼ ╾──────alloc14+0──────╼ │ ╾──────╼╾──────╼
alloc9 (size: 16, align: 8) {
╾──────alloc7+0──────╼ ╾──────alloc8+0───────╼ │ ╾──────╼╾──────╼
}

alloc13 (size: 1, align: 1) {
alloc7 (size: 1, align: 1) {
05 │ .
}

alloc14 (size: 1, align: 1) {
alloc8 (size: 1, align: 1) {
06 │ .
}

alloc23 (size: 24, align: 8) {
0x00 │ ╾──────alloc19+3──────╼ ╾──────alloc20+0──────╼ │ ╾──────╼╾──────╼
0x10 │ ╾──────alloc22+2──────╼ │ ╾──────╼
alloc19 (size: 24, align: 8) {
0x00 │ ╾──────alloc15+3──────╼ ╾──────alloc16+0──────╼ │ ╾──────╼╾──────╼
0x10 │ ╾──────alloc18+2──────╼ │ ╾──────╼
}

alloc19 (size: 4, align: 1) {
alloc15 (size: 4, align: 1) {
2a 45 15 6f │ *E.o
}

alloc20 (size: 1, align: 1) {
alloc16 (size: 1, align: 1) {
2a │ *
}

alloc22 (size: 4, align: 1) {
alloc18 (size: 4, align: 1) {
2a 45 15 6f │ *E.o
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Regression test for #66975 - ensure that we don't keep unevaluated
// `!`-typed constants until codegen.
// This was originally a regression test for #66975 - ensure that we do not generate never typed
// consts in codegen. We also have tests for this that catches the error, see
// src/test/ui/consts/const-eval/index-out-of-bounds-never-type.rs.

// Force generation of optimized mir for functions that do not reach codegen.
// compile-flags: --emit mir,link
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// MIR for `no_codegen` after PreCodegen

fn no_codegen() -> () {
let mut _0: (); // return place in scope 0 at $DIR/remove-never-const.rs:19:20: 19:20
scope 1 {
}

bb0: {
unreachable; // bb0[0]: scope 0 at $DIR/remove-never-const.rs:20:13: 20:33
}
}

This file was deleted.

0 comments on commit 0b95879

Please sign in to comment.