Skip to content

Initial implementation of UnsafePinned and UnsafeUnpin (RFC 3467) #136964

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions compiler/rustc_abi/src/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
repr: &ReprOptions,
variants: &IndexSlice<VariantIdx, IndexVec<FieldIdx, F>>,
is_enum: bool,
is_unsafe_cell: bool,
is_special_no_niche: bool,
scalar_valid_range: (Bound<u128>, Bound<u128>),
discr_range_of_repr: impl Fn(i128, i128) -> (Integer, bool),
discriminants: impl Iterator<Item = (VariantIdx, i128)>,
Expand Down Expand Up @@ -273,7 +273,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
repr,
variants,
is_enum,
is_unsafe_cell,
is_special_no_niche,
scalar_valid_range,
always_sized,
present_first,
Expand Down Expand Up @@ -418,7 +418,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
repr: &ReprOptions,
variants: &IndexSlice<VariantIdx, IndexVec<FieldIdx, F>>,
is_enum: bool,
is_unsafe_cell: bool,
is_special_no_niche: bool,
scalar_valid_range: (Bound<u128>, Bound<u128>),
always_sized: bool,
present_first: VariantIdx,
Expand All @@ -437,7 +437,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
let mut st = self.univariant(&variants[v], repr, kind)?;
st.variants = Variants::Single { index: v };

if is_unsafe_cell {
if is_special_no_niche {
let hide_niches = |scalar: &mut _| match scalar {
Scalar::Initialized { value, valid_range } => {
*valid_range = WrappingRange::full(value.size(dl))
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1828,7 +1828,8 @@ where
pub enum PointerKind {
/// Shared reference. `frozen` indicates the absence of any `UnsafeCell`.
SharedRef { frozen: bool },
/// Mutable reference. `unpin` indicates the absence of any pinned data.
/// Mutable reference. `unpin` indicates the absence of any pinned
/// data (`UnsafePinned` or `PhantomPinned`).
MutableRef { unpin: bool },
/// Box. `unpin` indicates the absence of any pinned data. `global` indicates whether this box
/// uses the global allocator or a custom one.
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_ast_ir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@ use rustc_macros::{Decodable, Encodable, HashStable_NoContext};
pub mod visit;

/// The movability of a coroutine / closure literal:
/// whether a coroutine contains self-references, causing it to be `!Unpin`.
/// whether a coroutine contains self-references, causing it to be
/// `!Unpin + !UnsafeUnpin`.
#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Copy)]
#[cfg_attr(feature = "nightly", derive(Encodable, Decodable, HashStable_NoContext))]
pub enum Movability {
/// May contain self-references, `!Unpin`.
/// May contain self-references, `!Unpin + !UnsafeUnpin`.
Static,
/// Must not contain self-references, `Unpin`.
/// Must not contain self-references, `!Unpin + !UnsafeUnpin`.
Movable,
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3407,7 +3407,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
Some(3)
} else if string.starts_with("static") {
// `static` is 6 chars long
// This is used for `!Unpin` coroutines
// This is used for immovable (self-referential) coroutines
Some(6)
} else {
None
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_codegen_ssa/src/traits/type_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ pub trait DerivedTypeCodegenMethods<'tcx>:
ty.is_freeze(self.tcx(), self.typing_env())
}

fn type_is_unsafe_unpin(&self, ty: Ty<'tcx>) -> bool {
ty.is_unsafe_unpin(self.tcx(), self.typing_env())
}

fn type_has_metadata(&self, ty: Ty<'tcx>) -> bool {
if ty.is_sized(self.tcx(), self.typing_env()) {
return false;
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,11 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
ty.is_freeze(*self.tcx, self.typing_env)
}

#[inline]
pub fn type_is_unsafe_unpin(&self, ty: Ty<'tcx>) -> bool {
ty.is_unsafe_unpin(*self.tcx, self.typing_env)
}

pub fn load_mir(
&self,
instance: ty::InstanceKind<'tcx>,
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_hir/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ language_item_table! {
DynMetadata, sym::dyn_metadata, dyn_metadata, Target::Struct, GenericRequirement::None;

Freeze, sym::freeze, freeze_trait, Target::Trait, GenericRequirement::Exact(0);
UnsafeUnpin, sym::unsafe_unpin, unsafe_unpin_trait, Target::Trait, GenericRequirement::Exact(0);

FnPtrTrait, sym::fn_ptr_trait, fn_ptr_trait, Target::Trait, GenericRequirement::Exact(0);
FnPtrAddr, sym::fn_ptr_addr, fn_ptr_addr, Target::Method(MethodKind::Trait { body: false }), GenericRequirement::None;
Expand Down Expand Up @@ -234,6 +235,8 @@ language_item_table! {
IndexMut, sym::index_mut, index_mut_trait, Target::Trait, GenericRequirement::Exact(1);

UnsafeCell, sym::unsafe_cell, unsafe_cell_type, Target::Struct, GenericRequirement::None;
UnsafePinned, sym::unsafe_pinned, unsafe_pinned_type, Target::Struct, GenericRequirement::None;

VaList, sym::va_list, va_list, Target::Struct, GenericRequirement::None;

Deref, sym::deref, deref_trait, Target::Trait, GenericRequirement::Exact(0);
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_lint/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -867,8 +867,8 @@ fn ty_is_known_nonnull<'tcx>(
return true;
}

// `UnsafeCell` has its niche hidden.
if def.is_unsafe_cell() {
// `UnsafeCell` and `UnsafePinned` have their niche hidden.
if def.is_unsafe_cell() || def.is_unsafe_pinned() {
return false;
}

Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1527,9 +1527,9 @@ rustc_queries! {
query is_freeze_raw(env: ty::PseudoCanonicalInput<'tcx, Ty<'tcx>>) -> bool {
desc { "computing whether `{}` is freeze", env.value }
}
/// Query backing `Ty::is_unpin`.
query is_unpin_raw(env: ty::PseudoCanonicalInput<'tcx, Ty<'tcx>>) -> bool {
desc { "computing whether `{}` is `Unpin`", env.value }
/// Query backing `Ty::is_unsafe_unpin`.
query is_unsafe_unpin_raw(env: ty::PseudoCanonicalInput<'tcx, Ty<'tcx>>) -> bool {
desc { "computing whether `{}` is `UnsafeUnpin`", env.value }
}
/// Query backing `Ty::needs_drop`.
query needs_drop_raw(env: ty::PseudoCanonicalInput<'tcx, Ty<'tcx>>) -> bool {
Expand Down
13 changes: 12 additions & 1 deletion compiler/rustc_middle/src/ty/adt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,10 @@ bitflags::bitflags! {
const IS_VARIANT_LIST_NON_EXHAUSTIVE = 1 << 8;
/// Indicates whether the type is `UnsafeCell`.
const IS_UNSAFE_CELL = 1 << 9;
/// Indicates whether the type is `UnsafePinned`.
const IS_UNSAFE_PINNED = 1 << 10;
/// Indicates whether the type is anonymous.
const IS_ANONYMOUS = 1 << 10;
const IS_ANONYMOUS = 1 << 11;
}
}
rustc_data_structures::external_bitflags_debug! { AdtFlags }
Expand Down Expand Up @@ -301,6 +303,9 @@ impl AdtDefData {
if tcx.is_lang_item(did, LangItem::UnsafeCell) {
flags |= AdtFlags::IS_UNSAFE_CELL;
}
if tcx.is_lang_item(did, LangItem::UnsafePinned) {
flags |= AdtFlags::IS_UNSAFE_PINNED;
}

AdtDefData { did, variants, flags, repr }
}
Expand Down Expand Up @@ -393,6 +398,12 @@ impl<'tcx> AdtDef<'tcx> {
self.flags().contains(AdtFlags::IS_UNSAFE_CELL)
}

/// Returns `true` if this is `UnsafePinned<T>`.
#[inline]
pub fn is_unsafe_pinned(self) -> bool {
self.flags().contains(AdtFlags::IS_UNSAFE_PINNED)
}

/// Returns `true` if this is `ManuallyDrop<T>`.
#[inline]
pub fn is_manually_drop(self) -> bool {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,7 @@ bidirectional_lang_item_map! {
TransmuteTrait,
Tuple,
Unpin,
UnsafeUnpin,
Unsize,
// tidy-alphabetical-end
}
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_middle/src/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1022,16 +1022,16 @@ where
}
ty::Ref(_, ty, mt) if offset.bytes() == 0 => {
// Use conservative pointer kind if not optimizing. This saves us the
// Freeze/Unpin queries, and can save time in the codegen backend (noalias
// Freeze/UnsafeUnpin queries, and can save time in the codegen backend (noalias
// attributes in LLVM have compile-time cost even in unoptimized builds).
let optimize = tcx.sess.opts.optimize != OptLevel::No;
let kind = match mt {
hir::Mutability::Not => {
PointerKind::SharedRef { frozen: optimize && ty.is_freeze(tcx, typing_env) }
}
hir::Mutability::Mut => {
PointerKind::MutableRef { unpin: optimize && ty.is_unpin(tcx, typing_env) }
}
hir::Mutability::Mut => PointerKind::MutableRef {
unpin: optimize && ty.is_unsafe_unpin(tcx, typing_env),
},
};

tcx.layout_of(typing_env.as_query_input(ty)).ok().map(|layout| PointeeInfo {
Expand Down Expand Up @@ -1124,7 +1124,7 @@ where
debug_assert!(pointee.safe.is_none());
let optimize = tcx.sess.opts.optimize != OptLevel::No;
pointee.safe = Some(PointerKind::Box {
unpin: optimize && boxed_ty.is_unpin(tcx, typing_env),
unpin: optimize && boxed_ty.is_unsafe_unpin(tcx, typing_env),
global: this.ty.is_box_global(tcx),
});
}
Expand Down
22 changes: 12 additions & 10 deletions compiler/rustc_middle/src/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1193,7 +1193,7 @@ impl<'tcx> Ty<'tcx> {

/// Fast path helper for testing if a type is `Freeze`.
///
/// Returning true means the type is known to be `Freeze`. Returning
/// Returning `true` means the type is known to be `Freeze`. Returning
/// `false` means nothing -- could be `Freeze`, might not be.
pub fn is_trivially_freeze(self) -> bool {
match self.kind() {
Expand Down Expand Up @@ -1227,16 +1227,18 @@ impl<'tcx> Ty<'tcx> {
}
}

/// Checks whether values of this type `T` implement the `Unpin` trait.
pub fn is_unpin(self, tcx: TyCtxt<'tcx>, typing_env: ty::TypingEnv<'tcx>) -> bool {
self.is_trivially_unpin() || tcx.is_unpin_raw(typing_env.as_query_input(self))
/// Checks whether values of this type `T` implement the `UnsafeUnpin` trait.
///
/// For more information, see [RFC 3467](https://rust-lang.github.io/rfcs/3467-unsafe-pinned.html).
pub fn is_unsafe_unpin(self, tcx: TyCtxt<'tcx>, typing_env: ty::TypingEnv<'tcx>) -> bool {
self.is_trivially_unsafe_unpin() || tcx.is_unsafe_unpin_raw(typing_env.as_query_input(self))
}

/// Fast path helper for testing if a type is `Unpin`.
/// Fast path helper for testing if a type is `UnsafeUnpin`.
///
/// Returning true means the type is known to be `Unpin`. Returning
/// `false` means nothing -- could be `Unpin`, might not be.
fn is_trivially_unpin(self) -> bool {
/// Returning `true` means the type is known to be `UnsafeUnpin`. Returning
/// `false` means nothing -- could be `UnsafeUnpin`, might not be.
fn is_trivially_unsafe_unpin(self) -> bool {
match self.kind() {
ty::Int(_)
| ty::Uint(_)
Expand All @@ -1250,8 +1252,8 @@ impl<'tcx> Ty<'tcx> {
| ty::FnDef(..)
| ty::Error(_)
| ty::FnPtr(..) => true,
ty::Tuple(fields) => fields.iter().all(Self::is_trivially_unpin),
ty::Pat(ty, _) | ty::Slice(ty) | ty::Array(ty, _) => ty.is_trivially_unpin(),
ty::Tuple(fields) => fields.iter().all(Self::is_trivially_unsafe_unpin),
ty::Pat(ty, _) | ty::Slice(ty) | ty::Array(ty, _) => ty.is_trivially_unsafe_unpin(),
ty::Adt(..)
| ty::Bound(..)
| ty::Closure(..)
Expand Down
12 changes: 8 additions & 4 deletions compiler/rustc_next_trait_solver/src/solve/trait_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1060,11 +1060,15 @@ where

ty::Infer(_) | ty::Bound(_, _) => panic!("unexpected type `{self_ty:?}`"),

// Coroutines have one special built-in candidate, `Unpin`, which
// takes precedence over the structural auto trait candidate being
// assembled.
// Coroutines have two special built-in candidates, `UnsafeUnpin`
// and `Unpin`, which take precedence over the structural auto trait
// candidate being assembled.
ty::Coroutine(def_id, _)
if self.cx().is_lang_item(goal.predicate.def_id(), TraitSolverLangItem::Unpin) =>
if self.cx().is_lang_item(goal.predicate.def_id(), TraitSolverLangItem::Unpin)
|| self.cx().is_lang_item(
goal.predicate.def_id(),
TraitSolverLangItem::UnsafeUnpin,
) =>
Comment on lines -1067 to +1071
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still very unsure if this is ok, I think so? I'm not sure how to "wrap" coroutine fields in UnsafePinned (and if that would make any difference from just not implementing UnsafePinned directly)

{
match self.cx().coroutine_movability(def_id) {
Movability::Static => Some(Err(NoSolution)),
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2152,6 +2152,8 @@ symbols! {
unsafe_fields,
unsafe_no_drop_flag,
unsafe_pin_internals,
unsafe_pinned,
unsafe_unpin,
unsize,
unsized_const_param_ty,
unsized_const_params,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -727,16 +727,17 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
candidates.ambiguous = true;
}
ty::Coroutine(coroutine_def_id, _)
if self.tcx().is_lang_item(def_id, LangItem::Unpin) =>
if self.tcx().is_lang_item(def_id, LangItem::Unpin)
|| self.tcx().is_lang_item(def_id, LangItem::UnsafeUnpin) =>
Comment on lines -730 to +731
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also unsure about this, just matching the logic in the new solver for now?

{
match self.tcx().coroutine_movability(coroutine_def_id) {
hir::Movability::Static => {
// Immovable coroutines are never `Unpin`, so
// suppress the normal auto-impl candidate for it.
// Immovable coroutines are never `[Unsafe]Unpin`,
// so suppress the normal auto-impl candidate for it.
}
hir::Movability::Movable => {
// Movable coroutines are always `Unpin`, so add an
// unconditional builtin candidate.
// Movable coroutines are always `[Unsafe]Unpin`,
// so add an unconditional builtin candidate.
candidates.vec.push(BuiltinCandidate { has_nested: false });
}
}
Expand Down
14 changes: 8 additions & 6 deletions compiler/rustc_ty_utils/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -379,19 +379,21 @@ fn adjust_for_rust_scalar<'tcx>(
Some(kind)
} else if let Some(pointee) = drop_target_pointee {
// The argument to `drop_in_place` is semantically equivalent to a mutable reference.
Some(PointerKind::MutableRef { unpin: pointee.is_unpin(tcx, cx.typing_env) })
Some(PointerKind::MutableRef { unpin: pointee.is_unsafe_unpin(tcx, cx.typing_env) })
} else {
None
};
if let Some(kind) = kind {
attrs.pointee_align = Some(pointee.align);

// `Box` are not necessarily dereferenceable for the entire duration of the function as
// they can be deallocated at any time. Same for non-frozen shared references (see
// <https://github.com/rust-lang/rust/pull/98017>), and for mutable references to
// potentially self-referential types (see
// <https://github.com/rust-lang/unsafe-code-guidelines/issues/381>). If LLVM had a way
// to say "dereferenceable on entry" we could use it here.
// they can be deallocated at any time. Same for `!Freeze` shared references
// (see <https://github.com/rust-lang/rust/pull/98017>), and for mutable
// references to `!UnsafeUnpin` (ie. potentially self-referential types)
// (see <https://github.com/rust-lang/unsafe-code-guidelines/issues/381>
// and <https://rust-lang.github.io/rfcs/3467-unsafe-pinned.html>).
//
// If LLVM had a way to say "dereferenceable on entry" we could use it here.
attrs.pointee_size = match kind {
PointerKind::Box { .. }
| PointerKind::SharedRef { frozen: false }
Expand Down
10 changes: 7 additions & 3 deletions compiler/rustc_ty_utils/src/common_traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ fn is_freeze_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::PseudoCanonicalInput<'tcx,
is_item_raw(tcx, query, LangItem::Freeze)
}

fn is_unpin_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::PseudoCanonicalInput<'tcx, Ty<'tcx>>) -> bool {
is_item_raw(tcx, query, LangItem::Unpin)
fn is_unsafe_unpin_raw<'tcx>(
tcx: TyCtxt<'tcx>,
query: ty::PseudoCanonicalInput<'tcx, Ty<'tcx>>,
) -> bool {
is_item_raw(tcx, query, LangItem::UnsafeUnpin)
}

fn is_item_raw<'tcx>(
Expand All @@ -33,5 +36,6 @@ fn is_item_raw<'tcx>(
}

pub(crate) fn provide(providers: &mut Providers) {
*providers = Providers { is_copy_raw, is_sized_raw, is_freeze_raw, is_unpin_raw, ..*providers };
*providers =
Providers { is_copy_raw, is_sized_raw, is_freeze_raw, is_unsafe_unpin_raw, ..*providers };
}
8 changes: 6 additions & 2 deletions compiler/rustc_ty_utils/src/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,9 @@ fn layout_of_uncached<'tcx>(
));
}

// UnsafeCell and UnsafePinned both disable niche optimizations
let is_special_no_niche = def.is_unsafe_cell() || def.is_unsafe_pinned();

let get_discriminant_type =
|min, max| abi::Integer::repr_discr(tcx, ty, &def.repr(), min, max);

Expand Down Expand Up @@ -647,7 +650,7 @@ fn layout_of_uncached<'tcx>(
&def.repr(),
&variants,
def.is_enum(),
def.is_unsafe_cell(),
is_special_no_niche,
tcx.layout_scalar_valid_range(def.did()),
get_discriminant_type,
discriminants_iter(),
Expand All @@ -673,7 +676,7 @@ fn layout_of_uncached<'tcx>(
&def.repr(),
&variants,
def.is_enum(),
def.is_unsafe_cell(),
is_special_no_niche,
tcx.layout_scalar_valid_range(def.did()),
get_discriminant_type,
discriminants_iter(),
Expand Down Expand Up @@ -1068,6 +1071,7 @@ fn coroutine_layout<'tcx>(
// would do the same for us here.
// See <https://github.com/rust-lang/rust/issues/63818>, <https://github.com/rust-lang/miri/issues/3780>.
// FIXME: Remove when <https://github.com/rust-lang/rust/issues/125735> is implemented and aliased coroutine fields are wrapped in `UnsafePinned`.
// FIXME(unsafe_pinned)
largest_niche: None,
size,
align,
Expand Down
Loading
Loading