Skip to content

Commit

Permalink
Auto merge of #80290 - RalfJung:less-intrinsic-write, r=lcnr
Browse files Browse the repository at this point in the history 
implement ptr::write without dedicated intrinsic

This makes `ptr::write` more consistent with `ptr::write_unaligned`, `ptr::read`, `ptr::read_unaligned`, all of which are implemented in terms of `copy_nonoverlapping`.

This means we can also remove `move_val_init` implementations in codegen and Miri, and its special handling in the borrow checker.

Also see [this Zulip discussion](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/ptr.3A.3Aread.20vs.20ptr.3A.3Awrite).
  • Loading branch information
@bors
bors committed Jan 16, 2021
2 parents 63a83c5 + a5b89a0 commit 492b83c
Show file tree
Hide file tree
Showing 11 changed files with 46 additions and 408 deletions.
8 changes: 0 additions & 8 deletions compiler/rustc_mir/src/transform/check_unsafety.rs
View file
Expand Up @@ -223,21 +223,13 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
// Check for raw pointer `Deref`.
for (base, proj) in place.iter_projections() {
if proj == ProjectionElem::Deref {
let source_info = self.source_info; // Backup source_info so we can restore it later.
if base.projection.is_empty() && decl.internal {
// Internal locals are used in the `move_val_init` desugaring.
// We want to check unsafety against the source info of the
// desugaring, rather than the source info of the RHS.
self.source_info = self.body.local_decls[place.local].source_info;
}
let base_ty = base.ty(self.body, self.tcx).ty;
if base_ty.is_unsafe_ptr() {
self.require_unsafe(
UnsafetyViolationKind::GeneralAndConstFn,
UnsafetyViolationDetails::DerefOfRawPointer,
)
}
self.source_info = source_info; // Restore backed-up source_info.
}
}

Expand Down
108 changes: 34 additions & 74 deletions compiler/rustc_mir_build/src/build/expr/into.rs
View file
Expand Up @@ -10,9 +10,7 @@ use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_hir as hir;
use rustc_middle::middle::region;
use rustc_middle::mir::*;
use rustc_middle::ty::{self, CanonicalUserTypeAnnotation};
use rustc_span::symbol::sym;
use rustc_target::spec::abi::Abi;
use rustc_middle::ty::{CanonicalUserTypeAnnotation};

use std::slice;

Expand Down Expand Up @@ -219,79 +217,41 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
},
)
}
ExprKind::Call { ty, fun, args, from_hir_call, fn_span } => {
let intrinsic = match *ty.kind() {
ty::FnDef(def_id, _) => {
let f = ty.fn_sig(this.hir.tcx());
if f.abi() == Abi::RustIntrinsic || f.abi() == Abi::PlatformIntrinsic {
Some(this.hir.tcx().item_name(def_id))
} else {
None
}
}
_ => None,
};
ExprKind::Call { ty: _, fun, args, from_hir_call, fn_span } => {
let fun = unpack!(block = this.as_local_operand(block, fun));
if let Some(sym::move_val_init) = intrinsic {
// `move_val_init` has "magic" semantics - the second argument is
// always evaluated "directly" into the first one.

let mut args = args.into_iter();
let ptr = args.next().expect("0 arguments to `move_val_init`");
let val = args.next().expect("1 argument to `move_val_init`");
assert!(args.next().is_none(), ">2 arguments to `move_val_init`");

let ptr = this.hir.mirror(ptr);
let ptr_ty = ptr.ty;
// Create an *internal* temp for the pointer, so that unsafety
// checking won't complain about the raw pointer assignment.
let ptr_temp = this
.local_decls
.push(LocalDecl::with_source_info(ptr_ty, source_info).internal());
let ptr_temp = Place::from(ptr_temp);
// No need for a scope, ptr_temp doesn't need drop
let block = unpack!(this.into(ptr_temp, None, block, ptr));
// Maybe we should provide a scope here so that
// `move_val_init` wouldn't leak on panic even with an
// arbitrary `val` expression, but `schedule_drop`,
// borrowck and drop elaboration all prevent us from
// dropping `ptr_temp.deref()`.
this.into(this.hir.tcx().mk_place_deref(ptr_temp), None, block, val)
} else {
let args: Vec<_> = args
.into_iter()
.map(|arg| unpack!(block = this.as_local_call_operand(block, arg)))
.collect();

let success = this.cfg.start_new_block();

this.record_operands_moved(&args);

debug!("into_expr: fn_span={:?}", fn_span);

this.cfg.terminate(
block,
source_info,
TerminatorKind::Call {
func: fun,
args,
cleanup: None,
// FIXME(varkor): replace this with an uninhabitedness-based check.
// This requires getting access to the current module to call
// `tcx.is_ty_uninhabited_from`, which is currently tricky to do.
destination: if expr.ty.is_never() {
None
} else {
Some((destination, success))
},
from_hir_call,
fn_span,
let args: Vec<_> = args
.into_iter()
.map(|arg| unpack!(block = this.as_local_call_operand(block, arg)))
.collect();

let success = this.cfg.start_new_block();

this.record_operands_moved(&args);

debug!("into_expr: fn_span={:?}", fn_span);

this.cfg.terminate(
block,
source_info,
TerminatorKind::Call {
func: fun,
args,
cleanup: None,
// FIXME(varkor): replace this with an uninhabitedness-based check.
// This requires getting access to the current module to call
// `tcx.is_ty_uninhabited_from`, which is currently tricky to do.
destination: if expr.ty.is_never() {
None
} else {
Some((destination, success))
},
);
this.diverge_from(block);
schedule_drop(this);
success.unit()
}
from_hir_call,
fn_span,
},
);
this.diverge_from(block);
schedule_drop(this);
success.unit()
}
ExprKind::Use { source } => this.into(destination, scope, block, source),
ExprKind::Borrow { arg, borrow_kind } => {
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_span/src/symbol.rs
View file
Expand Up @@ -716,7 +716,6 @@ symbols! {
more_struct_aliases,
movbe_target_feature,
move_ref_pattern,
move_val_init,
mul,
mul_assign,
mul_with_overflow,
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_typeck/src/check/intrinsic.rs
View file
Expand Up @@ -157,7 +157,6 @@ pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) {
}
sym::forget => (1, vec![param(0)], tcx.mk_unit()),
sym::transmute => (2, vec![param(0)], param(1)),
sym::move_val_init => (1, vec![tcx.mk_mut_ptr(param(0)), param(0)], tcx.mk_unit()),
sym::prefetch_read_data
| sym::prefetch_write_data
| sym::prefetch_read_instruction
Expand Down
7 changes: 0 additions & 7 deletions library/core/src/intrinsics.rs
View file
Expand Up @@ -768,13 +768,6 @@ extern "rust-intrinsic" {
#[rustc_const_stable(feature = "const_size_of", since = "1.40.0")]
pub fn size_of<T>() -> usize;

/// Moves a value to an uninitialized memory location.
///
/// Drop glue is not run on the destination.
///
/// The stabilized version of this intrinsic is [`core::ptr::write`](crate::ptr::write).
pub fn move_val_init<T>(dst: *mut T, src: T);

/// The minimum alignment of a type.
///
/// The stabilized version of this intrinsic is [`core::mem::align_of`](crate::mem::align_of).
Expand Down
17 changes: 12 additions & 5 deletions library/core/src/ptr/mod.rs
View file
Expand Up @@ -883,12 +883,19 @@ pub const unsafe fn read_unaligned<T>(src: *const T) -> T {
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
pub unsafe fn write<T>(dst: *mut T, src: T) {
if cfg!(debug_assertions) && !is_aligned_and_not_null(dst) {
// Not panicking to keep codegen impact smaller.
abort();
// We are calling the intrinsics directly to avoid function calls in the generated code
// as `intrinsics::copy_nonoverlapping` is a wrapper function.
extern "rust-intrinsic" {
fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: usize);
}

// SAFETY: the caller must guarantee that `dst` is valid for writes.
// `dst` cannot overlap `src` because the caller has mutable access
// to `dst` while `src` is owned by this function.
unsafe {
copy_nonoverlapping(&src as *const T, dst, 1);
intrinsics::forget(src);
}
// SAFETY: the caller must uphold the safety contract for `move_val_init`.
unsafe { intrinsics::move_val_init(&mut *dst, src) }
}

/// Overwrites a memory location with the given value without reading or
Expand Down
19 changes: 0 additions & 19 deletions src/test/codegen/intrinsics/move-val-init.rs
View file

This file was deleted.

0 comments on commit 492b83c

Please sign in to comment.