Skip to content

Commit

Permalink
Rollup merge of #103584 - ouz-a:issue-102303, r=oli-obk
Browse files Browse the repository at this point in the history 
Remove bounds check when array is indexed by enum

As the title says, this reverts the behavior introduced with 1.64.

Fixes #102303

r? `@oli-obk`
  • Loading branch information
@Dylan-DPC
Dylan-DPC committed Nov 1, 2022
2 parents 4363467 + a1672ad commit 94241e7
Show file tree
Hide file tree
Showing 9 changed files with 120 additions and 19 deletions.
51 changes: 50 additions & 1 deletion compiler/rustc_mir_build/src/build/expr/as_rvalue.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use rustc_index::vec::Idx;
use rustc_middle::ty::util::IntTypeExt;
use rustc_target::abi::{Abi, Primitive};

use crate::build::expr::as_place::PlaceBase;
use crate::build::expr::category::{Category, RvalueFunc};
Expand Down Expand Up @@ -198,15 +199,63 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let (source, ty) = if let ty::Adt(adt_def, ..) = source.ty.kind() && adt_def.is_enum() {
let discr_ty = adt_def.repr().discr_type().to_ty(this.tcx);
let temp = unpack!(block = this.as_temp(block, scope, source, Mutability::Not));
let layout = this.tcx.layout_of(this.param_env.and(source.ty));
let discr = this.temp(discr_ty, source.span);
this.cfg.push_assign(
block,
source_info,
discr,
Rvalue::Discriminant(temp.into()),
);
let (op,ty) = (Operand::Move(discr), discr_ty);

if let Abi::Scalar(scalar) = layout.unwrap().abi{
if let Primitive::Int(_, signed) = scalar.primitive() {
let range = scalar.valid_range(&this.tcx);
// FIXME: Handle wraparound cases too.
if range.end >= range.start {
let mut assumer = |range: u128, bin_op: BinOp| {
// We will be overwriting this val if our scalar is signed value
// because sign extension on unsigned types might cause unintended things
let mut range_val =
ConstantKind::from_bits(this.tcx, range, ty::ParamEnv::empty().and(discr_ty));
let bool_ty = this.tcx.types.bool;
if signed {
let scalar_size_extend = scalar.size(&this.tcx).sign_extend(range);
let discr_layout = this.tcx.layout_of(this.param_env.and(discr_ty));
let truncated_val = discr_layout.unwrap().size.truncate(scalar_size_extend);
range_val = ConstantKind::from_bits(
this.tcx,
truncated_val,
ty::ParamEnv::empty().and(discr_ty),
);
}
let lit_op = this.literal_operand(expr.span, range_val);
let is_bin_op = this.temp(bool_ty, expr_span);
this.cfg.push_assign(
block,
source_info,
is_bin_op,
Rvalue::BinaryOp(bin_op, Box::new(((lit_op), (Operand::Copy(discr))))),
);
this.cfg.push(
block,
Statement {
source_info,
kind: StatementKind::Intrinsic(Box::new(NonDivergingIntrinsic::Assume(
Operand::Copy(is_bin_op),
))),
},
)
};
assumer(range.end, BinOp::Ge);
assumer(range.start, BinOp::Le);
}
}
}

(op,ty)

(Operand::Move(discr), discr_ty)
} else {
let ty = source.ty;
let source = unpack!(
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_mir_transform/src/check_unsafety.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,10 @@ impl<'tcx> Visitor<'tcx> for UnsafetyChecker<'_, 'tcx> {
| StatementKind::Retag { .. }
| StatementKind::AscribeUserType(..)
| StatementKind::Coverage(..)
| StatementKind::Intrinsic(..)
| StatementKind::Nop => {
// safe (at least as emitted during MIR construction)
}

// Move to above list once mir construction uses it.
StatementKind::Intrinsic(..) => unreachable!(),
}
self.super_statement(statement, location);
}
Expand Down
6 changes: 2 additions & 4 deletions src/test/codegen/enum-bounds-check-derived-idx.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,13 @@ pub enum Bar {
// CHECK-LABEL: @lookup_inc
#[no_mangle]
pub fn lookup_inc(buf: &[u8; 5], f: Bar) -> u8 {
// FIXME: panic check can be removed by adding the assumes back after https://github.com/rust-lang/rust/pull/98332
// CHECK: panic_bounds_check
// CHECK-NOT: panic_bounds_check
buf[f as usize + 1]
}

// CHECK-LABEL: @lookup_dec
#[no_mangle]
pub fn lookup_dec(buf: &[u8; 5], f: Bar) -> u8 {
// FIXME: panic check can be removed by adding the assumes back after https://github.com/rust-lang/rust/pull/98332
// CHECK: panic_bounds_check
// CHECK-NOT: panic_bounds_check
buf[f as usize - 1]
}
3 changes: 1 addition & 2 deletions src/test/codegen/enum-bounds-check-issue-13926.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ pub enum Exception {
// CHECK-LABEL: @access
#[no_mangle]
pub fn access(array: &[usize; 12], exc: Exception) -> usize {
// FIXME: panic check can be removed by adding the assumes back after https://github.com/rust-lang/rust/pull/98332
// CHECK: panic_bounds_check
// CHECK-NOT: panic_bounds_check
array[(exc as u8 - 4) as usize]
}
3 changes: 1 addition & 2 deletions src/test/codegen/enum-bounds-check.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ pub enum Bar {
// CHECK-LABEL: @lookup_unmodified
#[no_mangle]
pub fn lookup_unmodified(buf: &[u8; 5], f: Bar) -> u8 {
// FIXME: panic check can be removed by adding the assumes back after https://github.com/rust-lang/rust/pull/98332
// CHECK: panic_bounds_check
// CHECK-NOT: panic_bounds_check
buf[f as usize]
}
6 changes: 6 additions & 0 deletions src/test/mir-opt/building/enum_cast.bar.built.after.mir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,17 @@ fn bar(_1: Bar) -> usize {
let mut _0: usize; // return place in scope 0 at $DIR/enum_cast.rs:+0:21: +0:26
let _2: Bar; // in scope 0 at $DIR/enum_cast.rs:+1:5: +1:8
let mut _3: isize; // in scope 0 at $DIR/enum_cast.rs:+1:5: +1:8
let mut _4: bool; // in scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
let mut _5: bool; // in scope 0 at $DIR/enum_cast.rs:+1:5: +1:17

bb0: {
StorageLive(_2); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:8
_2 = move _1; // scope 0 at $DIR/enum_cast.rs:+1:5: +1:8
_3 = discriminant(_2); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
_4 = Ge(const 1_isize, _3); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
assume(_4); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
_5 = Le(const 0_isize, _3); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
assume(_5); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
_0 = move _3 as usize (IntToInt); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
StorageDead(_2); // scope 0 at $DIR/enum_cast.rs:+1:16: +1:17
return; // scope 0 at $DIR/enum_cast.rs:+2:2: +2:2
Expand Down
6 changes: 6 additions & 0 deletions src/test/mir-opt/building/enum_cast.boo.built.after.mir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,17 @@ fn boo(_1: Boo) -> usize {
let mut _0: usize; // return place in scope 0 at $DIR/enum_cast.rs:+0:21: +0:26
let _2: Boo; // in scope 0 at $DIR/enum_cast.rs:+1:5: +1:8
let mut _3: u8; // in scope 0 at $DIR/enum_cast.rs:+1:5: +1:8
let mut _4: bool; // in scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
let mut _5: bool; // in scope 0 at $DIR/enum_cast.rs:+1:5: +1:17

bb0: {
StorageLive(_2); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:8
_2 = move _1; // scope 0 at $DIR/enum_cast.rs:+1:5: +1:8
_3 = discriminant(_2); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
_4 = Ge(const 1_u8, _3); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
assume(_4); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
_5 = Le(const 0_u8, _3); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
assume(_5); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
_0 = move _3 as usize (IntToInt); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17
StorageDead(_2); // scope 0 at $DIR/enum_cast.rs:+1:16: +1:17
return; // scope 0 at $DIR/enum_cast.rs:+2:2: +2:2
Expand Down
20 changes: 13 additions & 7 deletions src/test/mir-opt/building/enum_cast.droppy.built.after.mir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ fn droppy() -> () {
let _2: Droppy; // in scope 0 at $DIR/enum_cast.rs:+2:13: +2:14
let _4: Droppy; // in scope 0 at $DIR/enum_cast.rs:+5:17: +5:18
let mut _5: isize; // in scope 0 at $DIR/enum_cast.rs:+5:17: +5:18
let _6: Droppy; // in scope 0 at $DIR/enum_cast.rs:+7:9: +7:10
let mut _6: bool; // in scope 0 at $DIR/enum_cast.rs:+5:17: +5:27
let mut _7: bool; // in scope 0 at $DIR/enum_cast.rs:+5:17: +5:27
let _8: Droppy; // in scope 0 at $DIR/enum_cast.rs:+7:9: +7:10
scope 1 {
debug x => _2; // in scope 1 at $DIR/enum_cast.rs:+2:13: +2:14
scope 2 {
Expand All @@ -17,7 +19,7 @@ fn droppy() -> () {
}
}
scope 4 {
debug z => _6; // in scope 4 at $DIR/enum_cast.rs:+7:9: +7:10
debug z => _8; // in scope 4 at $DIR/enum_cast.rs:+7:9: +7:10
}

bb0: {
Expand All @@ -29,6 +31,10 @@ fn droppy() -> () {
StorageLive(_4); // scope 3 at $DIR/enum_cast.rs:+5:17: +5:18
_4 = move _2; // scope 3 at $DIR/enum_cast.rs:+5:17: +5:18
_5 = discriminant(_4); // scope 3 at $DIR/enum_cast.rs:+5:17: +5:27
_6 = Ge(const 2_isize, _5); // scope 3 at $DIR/enum_cast.rs:+5:17: +5:27
assume(_6); // scope 3 at $DIR/enum_cast.rs:+5:17: +5:27
_7 = Le(const 0_isize, _5); // scope 3 at $DIR/enum_cast.rs:+5:17: +5:27
assume(_7); // scope 3 at $DIR/enum_cast.rs:+5:17: +5:27
_3 = move _5 as usize (IntToInt); // scope 3 at $DIR/enum_cast.rs:+5:17: +5:27
drop(_4) -> [return: bb1, unwind: bb4]; // scope 3 at $DIR/enum_cast.rs:+5:26: +5:27
}
Expand All @@ -44,15 +50,15 @@ fn droppy() -> () {
bb2: {
StorageDead(_2); // scope 0 at $DIR/enum_cast.rs:+6:5: +6:6
StorageDead(_1); // scope 0 at $DIR/enum_cast.rs:+6:5: +6:6
StorageLive(_6); // scope 0 at $DIR/enum_cast.rs:+7:9: +7:10
_6 = Droppy::B; // scope 0 at $DIR/enum_cast.rs:+7:13: +7:22
FakeRead(ForLet(None), _6); // scope 0 at $DIR/enum_cast.rs:+7:9: +7:10
StorageLive(_8); // scope 0 at $DIR/enum_cast.rs:+7:9: +7:10
_8 = Droppy::B; // scope 0 at $DIR/enum_cast.rs:+7:13: +7:22
FakeRead(ForLet(None), _8); // scope 0 at $DIR/enum_cast.rs:+7:9: +7:10
_0 = const (); // scope 0 at $DIR/enum_cast.rs:+0:13: +8:2
drop(_6) -> [return: bb3, unwind: bb5]; // scope 0 at $DIR/enum_cast.rs:+8:1: +8:2
drop(_8) -> [return: bb3, unwind: bb5]; // scope 0 at $DIR/enum_cast.rs:+8:1: +8:2
}

bb3: {
StorageDead(_6); // scope 0 at $DIR/enum_cast.rs:+8:1: +8:2
StorageDead(_8); // scope 0 at $DIR/enum_cast.rs:+8:1: +8:2
return; // scope 0 at $DIR/enum_cast.rs:+8:2: +8:2
}

Expand Down
40 changes: 40 additions & 0 deletions src/test/ui/type-alias-enum-variants/self-in-enum-definition.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,46 @@ LL | V3 = Self::V1 {} as u8 + 2,
note: ...which requires const-evaluating + checking `Alpha::V3::{constant#0}`...
--> $DIR/self-in-enum-definition.rs:5:10
|
LL | V3 = Self::V1 {} as u8 + 2,
| ^^^^^^^^^^^^^^^^^^^^^
note: ...which requires caching mir of `Alpha::V3::{constant#0}` for CTFE...
--> $DIR/self-in-enum-definition.rs:5:10
|
LL | V3 = Self::V1 {} as u8 + 2,
| ^^^^^^^^^^^^^^^^^^^^^
note: ...which requires elaborating drops for `Alpha::V3::{constant#0}`...
--> $DIR/self-in-enum-definition.rs:5:10
|
LL | V3 = Self::V1 {} as u8 + 2,
| ^^^^^^^^^^^^^^^^^^^^^
note: ...which requires borrow-checking `Alpha::V3::{constant#0}`...
--> $DIR/self-in-enum-definition.rs:5:10
|
LL | V3 = Self::V1 {} as u8 + 2,
| ^^^^^^^^^^^^^^^^^^^^^
note: ...which requires processing MIR for `Alpha::V3::{constant#0}`...
--> $DIR/self-in-enum-definition.rs:5:10
|
LL | V3 = Self::V1 {} as u8 + 2,
| ^^^^^^^^^^^^^^^^^^^^^
note: ...which requires const checking `Alpha::V3::{constant#0}`...
--> $DIR/self-in-enum-definition.rs:5:10
|
LL | V3 = Self::V1 {} as u8 + 2,
| ^^^^^^^^^^^^^^^^^^^^^
note: ...which requires preparing `Alpha::V3::{constant#0}` for borrow checking...
--> $DIR/self-in-enum-definition.rs:5:10
|
LL | V3 = Self::V1 {} as u8 + 2,
| ^^^^^^^^^^^^^^^^^^^^^
note: ...which requires unsafety-checking `Alpha::V3::{constant#0}`...
--> $DIR/self-in-enum-definition.rs:5:10
|
LL | V3 = Self::V1 {} as u8 + 2,
| ^^^^^^^^^^^^^^^^^^^^^
note: ...which requires building MIR for `Alpha::V3::{constant#0}`...
--> $DIR/self-in-enum-definition.rs:5:10
|
LL | V3 = Self::V1 {} as u8 + 2,
| ^^^^^^^^^^^^^^^^^^^^^
= note: ...which requires computing layout of `Alpha`...
Expand Down

0 comments on commit 94241e7

Please sign in to comment.