Auto merge of #3940 - rust-lang:refutable_slice, r=RalfJung

Prefer refutable slice patterns over len check + index op

Just something I noticed while reviewing other PRs

We do it for shim arguments almost everywhere, but when the size is not statically known, we didn't use the helpers as they rely on array ops. But we can avoid a len check followed by several index ops by just using a refutable slice pattern with `let else`.

The pattern is so common, it seems almost worth a dedicated macro
This commit is contained in:
bors 2024-10-05 09:39:25 +00:00
commit 3b418b1485
5 changed files with 34 additions and 33 deletions

View File

@ -859,14 +859,15 @@ fn can_be_replaced_by_single_child(
) -> Option<UniIndex> { ) -> Option<UniIndex> {
let node = self.nodes.get(idx).unwrap(); let node = self.nodes.get(idx).unwrap();
let [child_idx] = node.children[..] else { return None };
// We never want to replace the root node, as it is also kept in `root_ptr_tags`. // We never want to replace the root node, as it is also kept in `root_ptr_tags`.
if node.children.len() != 1 || live.contains(&node.tag) || node.parent.is_none() { if live.contains(&node.tag) || node.parent.is_none() {
return None; return None;
} }
// Since protected nodes are never GC'd (see `borrow_tracker::FrameExtra::visit_provenance`), // Since protected nodes are never GC'd (see `borrow_tracker::FrameExtra::visit_provenance`),
// we know that `node` is not protected because otherwise `live` would // we know that `node` is not protected because otherwise `live` would
// have contained `node.tag`. // have contained `node.tag`.
let child_idx = node.children[0];
let child = self.nodes.get(child_idx).unwrap(); let child = self.nodes.get(child_idx).unwrap();
// Check that for that one child, `can_be_replaced_by_child` holds for the permission // Check that for that one child, `can_be_replaced_by_child` holds for the permission
// on all locations. // on all locations.

View File

@ -481,14 +481,14 @@ fn flock(&mut self, fd_num: i32, op: i32) -> InterpResult<'tcx, Scalar> {
fn fcntl(&mut self, args: &[OpTy<'tcx>]) -> InterpResult<'tcx, Scalar> { fn fcntl(&mut self, args: &[OpTy<'tcx>]) -> InterpResult<'tcx, Scalar> {
let this = self.eval_context_mut(); let this = self.eval_context_mut();
if args.len() < 2 { let [fd_num, cmd, ..] = args else {
throw_ub_format!( throw_ub_format!(
"incorrect number of arguments for fcntl: got {}, expected at least 2", "incorrect number of arguments for fcntl: got {}, expected at least 2",
args.len() args.len()
); );
} };
let fd_num = this.read_scalar(&args[0])?.to_i32()?; let fd_num = this.read_scalar(fd_num)?.to_i32()?;
let cmd = this.read_scalar(&args[1])?.to_i32()?; let cmd = this.read_scalar(cmd)?.to_i32()?;
// We only support getting the flags for a descriptor. // We only support getting the flags for a descriptor.
if cmd == this.eval_libc_i32("F_GETFD") { if cmd == this.eval_libc_i32("F_GETFD") {
@ -508,13 +508,13 @@ fn fcntl(&mut self, args: &[OpTy<'tcx>]) -> InterpResult<'tcx, Scalar> {
// because exec() isn't supported. The F_DUPFD and F_DUPFD_CLOEXEC commands only // because exec() isn't supported. The F_DUPFD and F_DUPFD_CLOEXEC commands only
// differ in whether the FD_CLOEXEC flag is pre-set on the new file descriptor, // differ in whether the FD_CLOEXEC flag is pre-set on the new file descriptor,
// thus they can share the same implementation here. // thus they can share the same implementation here.
if args.len() < 3 { let [_, _, start, ..] = args else {
throw_ub_format!( throw_ub_format!(
"incorrect number of arguments for fcntl with cmd=`F_DUPFD`/`F_DUPFD_CLOEXEC`: got {}, expected at least 3", "incorrect number of arguments for fcntl with cmd=`F_DUPFD`/`F_DUPFD_CLOEXEC`: got {}, expected at least 3",
args.len() args.len()
); );
} };
let start = this.read_scalar(&args[2])?.to_i32()?; let start = this.read_scalar(start)?.to_i32()?;
match this.machine.fds.get(fd_num) { match this.machine.fds.get(fd_num) {
Some(fd) => Some(fd) =>

View File

@ -433,18 +433,18 @@ fn maybe_sync_file(
impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
fn open(&mut self, args: &[OpTy<'tcx>]) -> InterpResult<'tcx, Scalar> { fn open(&mut self, args: &[OpTy<'tcx>]) -> InterpResult<'tcx, Scalar> {
if args.len() < 2 { let [path_raw, flag, ..] = args else {
throw_ub_format!( throw_ub_format!(
"incorrect number of arguments for `open`: got {}, expected at least 2", "incorrect number of arguments for `open`: got {}, expected at least 2",
args.len() args.len()
); );
} };
let this = self.eval_context_mut(); let this = self.eval_context_mut();
let path_raw = this.read_pointer(&args[0])?; let path_raw = this.read_pointer(path_raw)?;
let path = this.read_path_from_c_str(path_raw)?; let path = this.read_path_from_c_str(path_raw)?;
let flag = this.read_scalar(&args[1])?.to_i32()?; let flag = this.read_scalar(flag)?.to_i32()?;
let mut options = OpenOptions::new(); let mut options = OpenOptions::new();

View File

@ -122,19 +122,19 @@ fn emulate_foreign_item_inner(
id if id == sys_getrandom => { id if id == sys_getrandom => {
// Used by getrandom 0.1 // Used by getrandom 0.1
// The first argument is the syscall id, so skip over it. // The first argument is the syscall id, so skip over it.
if args.len() < 4 { let [_, ptr, len, flags, ..] = args else {
throw_ub_format!( throw_ub_format!(
"incorrect number of arguments for `getrandom` syscall: got {}, expected at least 4", "incorrect number of arguments for `getrandom` syscall: got {}, expected at least 4",
args.len() args.len()
); );
} };
let ptr = this.read_pointer(&args[1])?; let ptr = this.read_pointer(ptr)?;
let len = this.read_target_usize(&args[2])?; let len = this.read_target_usize(len)?;
// The only supported flags are GRND_RANDOM and GRND_NONBLOCK, // The only supported flags are GRND_RANDOM and GRND_NONBLOCK,
// neither of which have any effect on our current PRNG. // neither of which have any effect on our current PRNG.
// See <https://github.com/rust-lang/rust/pull/79196> for a discussion of argument sizes. // See <https://github.com/rust-lang/rust/pull/79196> for a discussion of argument sizes.
let _flags = this.read_scalar(&args[3])?.to_i32()?; let _flags = this.read_scalar(flags)?.to_i32()?;
this.gen_random(ptr, len)?; this.gen_random(ptr, len)?;
this.write_scalar(Scalar::from_target_usize(len, this), dest)?; this.write_scalar(Scalar::from_target_usize(len, this), dest)?;

View File

@ -15,19 +15,19 @@ pub fn futex<'tcx>(
// may or may not be left out from the `syscall()` call. // may or may not be left out from the `syscall()` call.
// Therefore we don't use `check_arg_count` here, but only check for the // Therefore we don't use `check_arg_count` here, but only check for the
// number of arguments to fall within a range. // number of arguments to fall within a range.
if args.len() < 3 { let [addr, op, val, ..] = args else {
throw_ub_format!( throw_ub_format!(
"incorrect number of arguments for `futex` syscall: got {}, expected at least 3", "incorrect number of arguments for `futex` syscall: got {}, expected at least 3",
args.len() args.len()
); );
} };
// The first three arguments (after the syscall number itself) are the same to all futex operations: // The first three arguments (after the syscall number itself) are the same to all futex operations:
// (int *addr, int op, int val). // (int *addr, int op, int val).
// We checked above that these definitely exist. // We checked above that these definitely exist.
let addr = this.read_pointer(&args[0])?; let addr = this.read_pointer(addr)?;
let op = this.read_scalar(&args[1])?.to_i32()?; let op = this.read_scalar(op)?.to_i32()?;
let val = this.read_scalar(&args[2])?.to_i32()?; let val = this.read_scalar(val)?.to_i32()?;
// This is a vararg function so we have to bring our own type for this pointer. // This is a vararg function so we have to bring our own type for this pointer.
let addr = this.ptr_to_mplace(addr, this.machine.layouts.i32); let addr = this.ptr_to_mplace(addr, this.machine.layouts.i32);
@ -55,15 +55,15 @@ pub fn futex<'tcx>(
let wait_bitset = op & !futex_realtime == futex_wait_bitset; let wait_bitset = op & !futex_realtime == futex_wait_bitset;
let bitset = if wait_bitset { let bitset = if wait_bitset {
if args.len() < 6 { let [_, _, _, timeout, uaddr2, bitset, ..] = args else {
throw_ub_format!( throw_ub_format!(
"incorrect number of arguments for `futex` syscall with `op=FUTEX_WAIT_BITSET`: got {}, expected at least 6", "incorrect number of arguments for `futex` syscall with `op=FUTEX_WAIT_BITSET`: got {}, expected at least 6",
args.len() args.len()
); );
} };
let _timeout = this.read_pointer(&args[3])?; let _timeout = this.read_pointer(timeout)?;
let _uaddr2 = this.read_pointer(&args[4])?; let _uaddr2 = this.read_pointer(uaddr2)?;
this.read_scalar(&args[5])?.to_u32()? this.read_scalar(bitset)?.to_u32()?
} else { } else {
if args.len() < 4 { if args.len() < 4 {
throw_ub_format!( throw_ub_format!(
@ -183,15 +183,15 @@ pub fn futex<'tcx>(
// Same as FUTEX_WAKE, but allows you to specify a bitset to select which threads to wake up. // Same as FUTEX_WAKE, but allows you to specify a bitset to select which threads to wake up.
op if op == futex_wake || op == futex_wake_bitset => { op if op == futex_wake || op == futex_wake_bitset => {
let bitset = if op == futex_wake_bitset { let bitset = if op == futex_wake_bitset {
if args.len() < 6 { let [_, _, _, timeout, uaddr2, bitset, ..] = args else {
throw_ub_format!( throw_ub_format!(
"incorrect number of arguments for `futex` syscall with `op=FUTEX_WAKE_BITSET`: got {}, expected at least 6", "incorrect number of arguments for `futex` syscall with `op=FUTEX_WAKE_BITSET`: got {}, expected at least 6",
args.len() args.len()
); );
} };
let _timeout = this.read_pointer(&args[3])?; let _timeout = this.read_pointer(timeout)?;
let _uaddr2 = this.read_pointer(&args[4])?; let _uaddr2 = this.read_pointer(uaddr2)?;
this.read_scalar(&args[5])?.to_u32()? this.read_scalar(bitset)?.to_u32()?
} else { } else {
u32::MAX u32::MAX
}; };