mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

properly fill a promoted's required_consts

Browse Source 
then we can also make all_required_consts_are_checked a constant instead of a function
This commit is contained in:
Ralf Jung 2024-03-21 13:53:00 +01:00
parent bf021ea625
commit 173d1bd36b
13 changed files with 121 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
compiler/rustc_const_eval/src/const_eval/dummy_machine.rs
View File

@ -46,11 +46,8 @@ impl<'mir, 'tcx: 'mir> interpret::Machine<'mir, 'tcx> for DummyMachine {
type MemoryKind = !;
const PANIC_ON_ALLOC_FAIL: bool = true;
#[inline(always)]
fn all_required_consts_are_checked(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {
// We want to just eval random consts in the program, so `eval_mir_const` can fail.
false
}
// We want to just eval random consts in the program, so `eval_mir_const` can fail.
const ALL_CONSTS_ARE_PRECHECKED: bool = false;
#[inline(always)]
fn enforce_alignment(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {

14
compiler/rustc_const_eval/src/const_eval/machine.rs
View File

@ -375,20 +375,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
const PANIC_ON_ALLOC_FAIL: bool = false; // will be raised as a proper error
#[inline]
fn all_required_consts_are_checked(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {
// Generally we expect required_consts to be properly filled, except for promoteds where
// storing these consts shows up negatively in benchmarks. A promoted can only be relevant
// if its parent MIR is relevant, and the consts in the promoted will be in the parent's
// `required_consts`, so we are still sure to catch any const-eval bugs, just a bit less
// directly.
if ecx.frame_idx() == 0 && ecx.frame().body.source.promoted.is_some() {
false
} else {
true
}
}
#[inline(always)]
fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {
matches!(ecx.machine.check_alignment, CheckAlignment::Error)

4
compiler/rustc_const_eval/src/interpret/eval_context.rs
View File

@ -1179,9 +1179,7 @@ pub fn eval_mir_constant(
) -> InterpResult<'tcx, OpTy<'tcx, M::Provenance>> {
M::eval_mir_constant(self, *val, span, layout, |ecx, val, span, layout| {
let const_val = val.eval(*ecx.tcx, ecx.param_env, span).map_err(|err| {
if M::all_required_consts_are_checked(self)
&& !matches!(err, ErrorHandled::TooGeneric(..))
{
if M::ALL_CONSTS_ARE_PRECHECKED && !matches!(err, ErrorHandled::TooGeneric(..)) {
// Looks like the const is not captued by `required_consts`, that's bad.
bug!("interpret const eval failure of {val:?} which is not in required_consts");
}

2
compiler/rustc_const_eval/src/interpret/machine.rs
View File

@ -142,7 +142,7 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
/// Determines whether `eval_mir_constant` can never fail because all required consts have
/// already been checked before.
fn all_required_consts_are_checked(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;
const ALL_CONSTS_ARE_PRECHECKED: bool = true;
/// Whether memory accesses should be alignment-checked.
fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;

15
compiler/rustc_middle/src/mir/consts.rs
View File

@ -238,6 +238,21 @@ pub fn ty(&self) -> Ty<'tcx> {
}
}
/// Determines whether we need to add this const to `required_consts`. This is the case if and
/// only if evaluating it may error.
#[inline]
pub fn is_required_const(&self) -> bool {
match self {
Const::Ty(c) => match c.kind() {
ty::ConstKind::Value(_) => false, // already a value, cannot error
ty::ConstKind::Param(_) | ty::ConstKind::Error(_) => true, // these are errors or could be replaced by errors
_ => bug!("is_required_const: unexpected ty::ConstKind {:#?}", c),
},
Const::Unevaluated(..) => true,
Const::Val(..) => false, // already a value, cannot error
}
}
#[inline]
pub fn try_to_scalar(self) -> Option<Scalar> {
match self {

14
compiler/rustc_mir_transform/src/promote_consts.rs
View File

@ -951,6 +951,14 @@ fn visit_local(&mut self, local: &mut Local, _: PlaceContext, _: Location) {
*local = self.promote_temp(*local);
}
}
fn visit_constant(&mut self, constant: &mut ConstOperand<'tcx>, _location: Location) {
if constant.const_.is_required_const() {
self.promoted.required_consts.push(*constant);
}
// Skipping `super_constant` as the visitor is otherwise only looking for locals.
}
}
fn promote_candidates<'tcx>(
@ -995,11 +1003,6 @@ fn promote_candidates<'tcx>(
None,
body.tainted_by_errors,
);
// We keep `required_consts` of the new MIR body empty. All consts mentioned here have
// already been added to the parent MIR's `required_consts` (that is computed before
// promotion), and no matter where this promoted const ends up, our parent MIR must be
// somewhere in the reachable dependency chain so we can rely on its required consts being
// evaluated.
promoted.phase = MirPhase::Analysis(AnalysisPhase::Initial);
let promoter = Promoter {
@ -1012,6 +1015,7 @@ fn promote_candidates<'tcx>(
add_to_required: false,
};
// `required_consts` of the promoted itself gets filled while building the MIR body.
let mut promoted = promoter.promote_candidate(candidate, promotions.len());
promoted.source.promoted = Some(promotions.next_index());
promotions.push(promoted);

19
compiler/rustc_mir_transform/src/required_consts.rs
View File

@ -1,6 +1,5 @@
use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::{Const, ConstOperand, Location};
use rustc_middle::ty;
use rustc_middle::mir::{ConstOperand, Location};
pub struct RequiredConstsVisitor<'a, 'tcx> {
required_consts: &'a mut Vec<ConstOperand<'tcx>>,
@ -14,21 +13,7 @@ pub fn new(required_consts: &'a mut Vec<ConstOperand<'tcx>>) -> Self {
impl<'tcx> Visitor<'tcx> for RequiredConstsVisitor<'_, 'tcx> {
fn visit_constant(&mut self, constant: &ConstOperand<'tcx>, _: Location) {
// Only unevaluated consts have to be added to `required_consts` as only those can possibly
// still have latent const-eval errors.
let is_required = match constant.const_ {
Const::Ty(c) => match c.kind() {
ty::ConstKind::Value(_) => false, // already a value, cannot error
ty::ConstKind::Param(_) | ty::ConstKind::Error(_) => true, // these are errors or could be replaced by errors
_ => bug!(
"only ConstKind::Param/Value/Error should be encountered here, got {:#?}",
c
),
},
Const::Unevaluated(..) => true,
Const::Val(..) => false, // already a value, cannot error
};
if is_required {
if constant.const_.is_required_const() {
self.required_consts.push(*constant);
}
}

6
src/tools/miri/src/machine.rs
View File

@ -872,12 +872,6 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
const PANIC_ON_ALLOC_FAIL: bool = false;
#[inline(always)]
fn all_required_consts_are_checked(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {
// We want to catch any cases of missing required_consts
true
}
#[inline(always)]
fn enforce_alignment(ecx: &MiriInterpCx<'mir, 'tcx>) -> bool {
ecx.machine.check_alignment != AlignmentCheck::None

23
tests/ui/consts/required-consts/collect-in-promoted-const.noopt.stderr Normal file
View File

@ -0,0 +1,23 @@
error[E0080]: evaluation of `Fail::<i32>::C` failed
--> $DIR/collect-in-promoted-const.rs:9:19
|
LL | const C: () = panic!();
| ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-promoted-const.rs:9:19
|
= note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)
note: erroneous constant encountered
--> $DIR/collect-in-promoted-const.rs:20:21
|
LL | let _val = &Fail::<T>::C;
| ^^^^^^^^^^^^
note: the above error was encountered while instantiating `fn f::<i32>`
--> $DIR/collect-in-promoted-const.rs:25:5
|
LL | f::<i32>();
| ^^^^^^^^^^
error: aborting due to 1 previous error
For more information about this error, try `rustc --explain E0080`.

39
tests/ui/consts/required-consts/collect-in-promoted-const.opt.stderr Normal file
View File

@ -0,0 +1,39 @@
error[E0080]: evaluation of `Fail::<T>::C` failed
--> $DIR/collect-in-promoted-const.rs:9:19
|
LL | const C: () = panic!();
| ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-promoted-const.rs:9:19
|
= note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)
note: erroneous constant encountered
--> $DIR/collect-in-promoted-const.rs:20:21
|
LL | let _val = &Fail::<T>::C;
| ^^^^^^^^^^^^
error[E0080]: evaluation of `Fail::<i32>::C` failed
--> $DIR/collect-in-promoted-const.rs:9:19
|
LL | const C: () = panic!();
| ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-promoted-const.rs:9:19
|
= note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)
note: erroneous constant encountered
--> $DIR/collect-in-promoted-const.rs:20:21
|
LL | let _val = &Fail::<T>::C;
| ^^^^^^^^^^^^
|
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
note: the above error was encountered while instantiating `fn f::<i32>`
--> $DIR/collect-in-promoted-const.rs:25:5
|
LL | f::<i32>();
| ^^^^^^^^^^
error: aborting due to 2 previous errors
For more information about this error, try `rustc --explain E0080`.

26
tests/ui/consts/required-consts/collect-in-promoted-const.rs Normal file
View File

@ -0,0 +1,26 @@
//@revisions: noopt opt
//@ build-fail
//@[noopt] compile-flags: -Copt-level=0
//@[opt] compile-flags: -O
//! Make sure we error on erroneous consts even if they get promoted.
struct Fail<T>(T);
impl<T> Fail<T> {
const C: () = panic!(); //~ERROR evaluation of `Fail::<i32>::C` failed
//[opt]~^ ERROR evaluation of `Fail::<T>::C` failed
// (Not sure why optimizations lead to this being emitted twice, but as long as compilation
// fails either way it's fine.)
}
#[inline(never)]
fn f<T>() {
if false {
// If promotion moved `C` from our required_consts to its own, without adding itself to
// our required_consts, then we'd miss the const-eval failure here.
let _val = &Fail::<T>::C;
}
}
fn main() {
f::<i32>();
}

2
tests/ui/consts/required-consts/interpret-in-promoted.rs
View File

@ -1,7 +1,7 @@
//@revisions: noopt opt
//@[noopt] compile-flags: -Copt-level=0
//@[opt] compile-flags: -O
//! Make sure we error on erroneous consts even if they are unused.
//! Make sure we evaluate const fn calls even if they get promoted and their result ignored.
const unsafe fn ub() {
std::hint::unreachable_unchecked();

4
tests/ui/rfcs/rfc-1623-static/rfc1623-2.rs
View File

@ -26,8 +26,8 @@ fn id<T>(t: T) -> T {
foo: &Foo { bools: &[false, true] },
bar: &Bar { bools: &[true, true] },
f: &id,
//~^ ERROR mismatched types
//~| ERROR mismatched types
//~^ ERROR implementation of `FnOnce` is not general enough
//~| ERROR implementation of `FnOnce` is not general enough
//~| ERROR implementation of `Fn` is not general enough
//~| ERROR implementation of `Fn` is not general enough
};