Skip to content

Initial implementation of transmutability trait. #92268

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

Merged
merged 13 commits into from
Aug 3, 2022
Merged
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
15 changes: 15 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4553,6 +4553,7 @@ dependencies = [
"rustc_session",
"rustc_span",
"rustc_target",
"rustc_transmute",
"smallvec",
"tracing",
]
Expand All @@ -4577,6 +4578,20 @@ dependencies = [
"tracing",
]

[[package]]
name = "rustc_transmute"
version = "0.1.0"
dependencies = [
"itertools",
"rustc_data_structures",
"rustc_infer",
"rustc_macros",
"rustc_middle",
"rustc_span",
"rustc_target",
"tracing",
]

[[package]]
name = "rustc_ty_utils"
version = "0.0.0"
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 @@ -191,6 +191,9 @@ language_item_table! {
CoerceUnsized, sym::coerce_unsized, coerce_unsized_trait, Target::Trait, GenericRequirement::Minimum(1);
DispatchFromDyn, sym::dispatch_from_dyn, dispatch_from_dyn_trait, Target::Trait, GenericRequirement::Minimum(1);

// language items relating to transmutability
TransmuteTrait, sym::transmute_trait, transmute_trait, Target::Trait, GenericRequirement::Exact(6);

Add(Op), sym::add, add_trait, Target::Trait, GenericRequirement::Exact(1);
Sub(Op), sym::sub, sub_trait, Target::Trait, GenericRequirement::Exact(1);
Mul(Op), sym::mul, mul_trait, Target::Trait, GenericRequirement::Exact(1);
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ pub enum SelectionCandidate<'tcx> {
/// `false` if there are no *further* obligations.
has_nested: bool,
},

/// Implementation of transmutability trait.
TransmutabilityCandidate,

ParamCandidate(ty::PolyTraitPredicate<'tcx>),
ImplCandidate(DefId),
AutoImplCandidate(DefId),
Expand Down
91 changes: 90 additions & 1 deletion compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ pub use subst::*;
pub use vtable::*;

use std::fmt::Debug;
use std::hash::Hash;
use std::hash::{Hash, Hasher};
use std::ops::ControlFlow;
use std::{fmt, str};

Expand Down Expand Up @@ -1724,6 +1724,59 @@ impl VariantDef {
}
}

impl PartialEq for VariantDef {
#[inline]
fn eq(&self, other: &Self) -> bool {
// There should be only one `VariantDef` for each `def_id`, therefore
// it is fine to implement `PartialEq` only based on `def_id`.
//
// Below, we exhaustively destructure `self` and `other` so that if the
// definition of `VariantDef` changes, a compile-error will be produced,
// reminding us to revisit this assumption.

let Self {
def_id: lhs_def_id,
ctor_def_id: _,
name: _,
discr: _,
fields: _,
ctor_kind: _,
flags: _,
} = &self;

let Self {
def_id: rhs_def_id,
ctor_def_id: _,
name: _,
discr: _,
fields: _,
ctor_kind: _,
flags: _,
} = other;

lhs_def_id == rhs_def_id
}
}

impl Eq for VariantDef {}

impl Hash for VariantDef {
#[inline]
fn hash<H: Hasher>(&self, s: &mut H) {
// There should be only one `VariantDef` for each `def_id`, therefore
// it is fine to implement `Hash` only based on `def_id`.
//
// Below, we exhaustively destructure `self` so that if the definition
// of `VariantDef` changes, a compile-error will be produced, reminding
// us to revisit this assumption.

let Self { def_id, ctor_def_id: _, name: _, discr: _, fields: _, ctor_kind: _, flags: _ } =
&self;

def_id.hash(s)
}
}

#[derive(Copy, Clone, Debug, PartialEq, Eq, TyEncodable, TyDecodable, HashStable)]
pub enum VariantDiscr {
/// Explicit value for this variant, i.e., `X = 123`.
Expand All @@ -1744,6 +1797,42 @@ pub struct FieldDef {
pub vis: Visibility,
}

impl PartialEq for FieldDef {
#[inline]
fn eq(&self, other: &Self) -> bool {
// There should be only one `FieldDef` for each `did`, therefore it is
// fine to implement `PartialEq` only based on `did`.
//
// Below, we exhaustively destructure `self` so that if the definition
// of `FieldDef` changes, a compile-error will be produced, reminding
// us to revisit this assumption.

let Self { did: lhs_did, name: _, vis: _ } = &self;

let Self { did: rhs_did, name: _, vis: _ } = other;

lhs_did == rhs_did
}
}

impl Eq for FieldDef {}

impl Hash for FieldDef {
#[inline]
fn hash<H: Hasher>(&self, s: &mut H) {
// There should be only one `FieldDef` for each `did`, therefore it is
// fine to implement `Hash` only based on `did`.
//
// Below, we exhaustively destructure `self` so that if the definition
// of `FieldDef` changes, a compile-error will be produced, reminding
// us to revisit this assumption.

let Self { did, name: _, vis: _ } = &self;

did.hash(s)
}
}

bitflags! {
#[derive(TyEncodable, TyDecodable, Default, HashStable)]
pub struct ReprFlags: u8 {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1462,6 +1462,7 @@ symbols! {
trait_alias,
trait_upcasting,
transmute,
transmute_trait,
transparent,
transparent_enums,
transparent_unions,
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_target/src/abi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,7 @@ impl fmt::Debug for Align {

impl Align {
pub const ONE: Align = Align { pow2: 0 };
pub const MAX: Align = Align { pow2: 29 };

#[inline]
pub fn from_bits(bits: u64) -> Result<Align, String> {
Expand Down Expand Up @@ -540,7 +541,7 @@ impl Align {
if bytes != 1 {
return Err(not_power_of_2(align));
}
if pow2 > 29 {
if pow2 > Self::MAX.pow2 {
return Err(too_large(align));
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_trait_selection/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,5 @@ rustc_query_system = { path = "../rustc_query_system" }
rustc_session = { path = "../rustc_session" }
rustc_span = { path = "../rustc_span" }
rustc_target = { path = "../rustc_target" }
rustc_transmute = { path = "../rustc_transmute", features = ["rustc"] }
smallvec = { version = "1.8.1", features = ["union", "may_dangle"] }
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
self.assemble_candidates_for_unsizing(obligation, &mut candidates);
} else if lang_items.destruct_trait() == Some(def_id) {
self.assemble_const_destruct_candidates(obligation, &mut candidates);
} else if lang_items.transmute_trait() == Some(def_id) {
// User-defined transmutability impls are permitted.
self.assemble_candidates_from_impls(obligation, &mut candidates);
self.assemble_candidates_for_transmutability(obligation, &mut candidates);
} else {
if lang_items.clone_trait() == Some(def_id) {
// Same builtin conditions as `Copy`, i.e., every type which has builtin support
Expand Down Expand Up @@ -873,6 +877,24 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
};
}

#[tracing::instrument(level = "debug", skip(self, obligation, candidates))]
fn assemble_candidates_for_transmutability(
&mut self,
obligation: &TraitObligation<'tcx>,
candidates: &mut SelectionCandidateSet<'tcx>,
) {
if obligation.has_param_types_or_consts() {
return;
}

if obligation.has_infer_types_or_consts() {
candidates.ambiguous = true;
return;
}

candidates.vec.push(TransmutabilityCandidate);
}

#[tracing::instrument(level = "debug", skip(self, obligation, candidates))]
fn assemble_candidates_for_trait_alias(
&mut self,
Expand Down
52 changes: 52 additions & 0 deletions compiler/rustc_trait_selection/src/traits/select/confirmation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
ImplSource::Builtin(data)
}

TransmutabilityCandidate => {
let data = self.confirm_transmutability_candidate(obligation)?;
ImplSource::Builtin(data)
}

ParamCandidate(param) => {
let obligations =
self.confirm_param_candidate(obligation, param.map_bound(|t| t.trait_ref));
Expand Down Expand Up @@ -267,6 +272,53 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
ImplSourceBuiltinData { nested: obligations }
}

fn confirm_transmutability_candidate(
&mut self,
obligation: &TraitObligation<'tcx>,
) -> Result<ImplSourceBuiltinData<PredicateObligation<'tcx>>, SelectionError<'tcx>> {
debug!(?obligation, "confirm_transmutability_candidate");

let predicate = obligation.predicate;

let type_at = |i| predicate.map_bound(|p| p.trait_ref.substs.type_at(i));
let bool_at = |i| {
predicate
.skip_binder()
.trait_ref
.substs
.const_at(i)
.try_eval_bool(self.tcx(), obligation.param_env)
.unwrap_or(true)
Copy link
Contributor

Choose a reason for hiding this comment

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

heh, That's smart. Should probably document this behaviour in the tracking issue so it can get revisited before stabilization.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't believe this is a stability issue (even if the feature was on track for stabilization, which it isn't). The unwrap_or(true) branch is only reached if the const parameter is of the wrong type. This, in itself, is a type error, which rustc reports separately. However, since rustc nonetheless still tries to confirm this candidate, the compiler would ICE had I only written .unwrap(). So, what to do instead? I could:

  • bail out of the whole confirmation process with Err(Unimplemented)
    • this would always result in a second, "not transmutable" error message being shown to the user
  • bail out of the whole confirmation process with Ok(ImplSourceBuiltinData { nested: vec![] }),
    • this would never result in a "not transmutable" error message being shown, even if there's no way the types are safely transmutable even if that mis-provided safety check turned off
  • conservatively assume that the user wants the safety check turned off
    • this still results in an error message if the types cannot possibly be transmutable
    • but does not display an error message if the types could be transmutable under the right conditions
      • ...and those "right conditions" will become well-defined as soon as the user fixes the other error and passes in the const parameter correctly

In all possibilities, at least one error message is always displayed: the error that a const parameter of the wrong type has been passed in. So, whether or not the transmutability system also produces its own error isn't a stability issue, it's just an error message ergonomics issue.

};

let src_and_dst = predicate.map_bound(|p| rustc_transmute::Types {
src: p.trait_ref.substs.type_at(1),
dst: p.trait_ref.substs.type_at(0),
});

let scope = type_at(2).skip_binder();

let assume = rustc_transmute::Assume {
alignment: bool_at(3),
lifetimes: bool_at(4),
validity: bool_at(5),
visibility: bool_at(6),
};

let cause = obligation.cause.clone();

let mut transmute_env = rustc_transmute::TransmuteTypeEnv::new(self.infcx);

let maybe_transmutable = transmute_env.is_transmutable(cause, src_and_dst, scope, assume);

use rustc_transmute::Answer;

match maybe_transmutable {
Answer::Yes => Ok(ImplSourceBuiltinData { nested: vec![] }),
_ => Err(Unimplemented),
}
}

/// This handles the case where an `auto trait Foo` impl is being used.
/// The idea is that the impl applies to `X : Foo` if the following conditions are met:
///
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1630,6 +1630,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
);
}

// FIXME(@jswrenn): this should probably be more sophisticated
(TransmutabilityCandidate, _) | (_, TransmutabilityCandidate) => false,

// (*)
(
BuiltinCandidate { has_nested: false }
Expand Down
28 changes: 28 additions & 0 deletions compiler/rustc_transmute/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
[package]
name = "rustc_transmute"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
tracing = "0.1"
rustc_data_structures = { path = "../rustc_data_structures", optional = true}
rustc_infer = { path = "../rustc_infer", optional = true}
rustc_macros = { path = "../rustc_macros", optional = true}
rustc_middle = { path = "../rustc_middle", optional = true}
rustc_span = { path = "../rustc_span", optional = true}
rustc_target = { path = "../rustc_target", optional = true}

[features]
rustc = [
"rustc_middle",
"rustc_data_structures",
"rustc_infer",
"rustc_macros",
"rustc_span",
"rustc_target",
]

[dev-dependencies]
itertools = "0.10.1"
Loading