Skip to content
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

trait-based structural match implementation #65519

Merged
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
4 changes: 2 additions & 2 deletions src/libcore/cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ pub trait PartialEq<Rhs: ?Sized = Self> {
/// Derive macro generating an impl of the trait `PartialEq`.
#[rustc_builtin_macro]
#[stable(feature = "builtin_macro_prelude", since = "1.38.0")]
#[allow_internal_unstable(core_intrinsics)]
#[allow_internal_unstable(core_intrinsics, structural_match)]
pub macro PartialEq($item:item) { /* compiler built-in */ }

/// Trait for equality comparisons which are [equivalence relations](
Expand Down Expand Up @@ -273,7 +273,7 @@ pub trait Eq: PartialEq<Self> {
/// Derive macro generating an impl of the trait `Eq`.
#[rustc_builtin_macro]
#[stable(feature = "builtin_macro_prelude", since = "1.38.0")]
#[allow_internal_unstable(core_intrinsics, derive_eq)]
#[allow_internal_unstable(core_intrinsics, derive_eq, structural_match)]
pub macro Eq($item:item) { /* compiler built-in */ }

// FIXME: this struct is used solely by #[derive] to
Expand Down
87 changes: 87 additions & 0 deletions src/libcore/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,85 @@ pub trait Unsize<T: ?Sized> {
// Empty.
}

/// Required trait for constants used in pattern matches.
///
/// Any type that derives `PartialEq` automatically implements this trait,
/// *regardless* of whether its type-parameters implement `Eq`.
///
/// If a `const` item contains some type that does not implement this trait,
/// then that type either (1.) does not implement `PartialEq` (which means the
/// constant will not provide that comparison method, which code generation
/// assumes is available), or (2.) it implements *its own* version of
/// `PartialEq` (which we assume does not conform to a structural-equality
/// comparison).
///
/// In either of the two scenarios above, we reject usage of such a constant in
/// a pattern match.
///
/// See also the [structural match RFC][RFC1445], and [issue 63438][] which
/// motivated migrating from attribute-based design to this trait.
///
/// [RFC1445]: https://github.com/rust-lang/rfcs/blob/master/text/1445-restrict-constants-in-patterns.md
/// [issue 63438]: https://github.com/rust-lang/rust/issues/63438
#[cfg(not(bootstrap))]
#[unstable(feature = "structural_match", issue = "31434")]
#[rustc_on_unimplemented(message="the type `{Self}` does not `#[derive(PartialEq)]`")]
#[lang = "structural_peq"]
pub trait StructuralPartialEq {
// Empty.
}

/// Required trait for constants used in pattern matches.
///
/// Any type that derives `Eq` automatically implements this trait, *regardless*
/// of whether its type-parameters implement `Eq`.
///
/// This is a hack to workaround a limitation in our type-system.
///
/// Background:
///
/// We want to require that types of consts used in pattern matches
/// have the attribute `#[derive(PartialEq, Eq)]`.
///
/// In a more ideal world, we could check that requirement by just checking that
/// the given type implements both (1.) the `StructuralPartialEq` trait *and*
/// (2.) the `Eq` trait. However, you can have ADTs that *do* `derive(PartialEq, Eq)`,
/// and be a case that we want the compiler to accept, and yet the constant's
/// type fails to implement `Eq`.
///
/// Namely, a case like this:
///
/// ```rust
/// #[derive(PartialEq, Eq)]
/// struct Wrap<X>(X);
/// fn higher_order(_: &()) { }
/// const CFN: Wrap<fn(&())> = Wrap(higher_order);
/// fn main() {
/// match CFN {
/// CFN => {}
/// _ => {}
/// }
/// }
/// ```
///
/// (The problem in the above code is that `Wrap<fn(&())>` does not implement
/// `PartialEq`, nor `Eq`, because `for<'a> fn(&'a _)` does not implement those
/// traits.)
///
/// Therefore, we cannot rely on naive check for `StructuralPartialEq` and
/// mere `Eq`.
///
/// As a hack to work around this, we use two separate traits injected by each
/// of the two derives (`#[derive(PartialEq)]` and `#[derive(Eq)]`) and check
/// that both of them are present as part of structural-match checking.
#[cfg(not(bootstrap))]
#[unstable(feature = "structural_match", issue = "31434")]
#[rustc_on_unimplemented(message="the type `{Self}` does not `#[derive(Eq)]`")]
#[lang = "structural_teq"]
pub trait StructuralEq {
// Empty.
}

/// Types whose values can be duplicated simply by copying bits.
///
/// By default, variable bindings have 'move semantics.' In other
Expand Down Expand Up @@ -437,6 +516,14 @@ macro_rules! impls{
$t
}
}

#[cfg(not(bootstrap))]
#[unstable(feature = "structural_match", issue = "31434")]
impl<T: ?Sized> StructuralPartialEq for $t<T> { }

#[cfg(not(bootstrap))]
#[unstable(feature = "structural_match", issue = "31434")]
impl<T: ?Sized> StructuralEq for $t<T> { }
)
}

Expand Down
4 changes: 4 additions & 0 deletions src/librustc/middle/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,10 @@ language_item_table! {

SizedTraitLangItem, "sized", sized_trait, Target::Trait;
UnsizeTraitLangItem, "unsize", unsize_trait, Target::Trait;
// trait injected by #[derive(PartialEq)], (i.e. "Partial EQ").
StructuralPeqTraitLangItem, "structural_peq", structural_peq_trait, Target::Trait;
// trait injected by #[derive(Eq)], (i.e. "Total EQ"; no, I will not apologize).
StructuralTeqTraitLangItem, "structural_teq", structural_teq_trait, Target::Trait;
CopyTraitLangItem, "copy", copy_trait, Target::Trait;
CloneTraitLangItem, "clone", clone_trait, Target::Trait;
SyncTraitLangItem, "sync", sync_trait, Target::Trait;
Expand Down
3 changes: 3 additions & 0 deletions src/librustc/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2151,6 +2151,9 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
ObligationCauseCode::ConstSized => {
err.note("constant expressions must have a statically known size");
}
ObligationCauseCode::ConstPatternStructural => {
err.note("constants used for pattern-matching must derive `PartialEq` and `Eq`");
}
ObligationCauseCode::SharedStatic => {
err.note("shared static variables must have a type that implements `Sync`");
}
Expand Down
3 changes: 3 additions & 0 deletions src/librustc/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,9 @@ pub enum ObligationCauseCode<'tcx> {
/// Computing common supertype in the pattern guard for the arms of a match expression
MatchExpressionArmPattern { span: Span, ty: Ty<'tcx> },

/// Constants in patterns must have `Structural` type.
ConstPatternStructural,

/// Computing common supertype in an if expression
IfExpression(Box<IfExpressionCause>),

Expand Down
1 change: 1 addition & 0 deletions src/librustc/traits/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,7 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> {
super::RepeatVec => Some(super::RepeatVec),
super::FieldSized { adt_kind, last } => Some(super::FieldSized { adt_kind, last }),
super::ConstSized => Some(super::ConstSized),
super::ConstPatternStructural => Some(super::ConstPatternStructural),
super::SharedStatic => Some(super::SharedStatic),
super::BuiltinDerivedObligation(ref cause) => {
tcx.lift(cause).map(super::BuiltinDerivedObligation)
Expand Down
131 changes: 6 additions & 125 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ use syntax::symbol::{kw, sym, Symbol};
use syntax_pos::Span;

use smallvec;
use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
use rustc_data_structures::fx::{FxIndexMap};
use rustc_data_structures::stable_hasher::{StableHasher, HashStable};
use rustc_index::vec::{Idx, IndexVec};

Expand Down Expand Up @@ -84,6 +84,10 @@ pub use self::context::{

pub use self::instance::{Instance, InstanceDef};

pub use self::structural_match::search_for_structural_match_violation;
pub use self::structural_match::type_marked_structural;
pub use self::structural_match::NonStructuralMatchTy;

pub use self::trait_def::TraitDef;

pub use self::query::queries;
Expand Down Expand Up @@ -116,6 +120,7 @@ pub mod util;
mod context;
mod instance;
mod structural_impls;
mod structural_match;
mod sty;

// Data types
Expand Down Expand Up @@ -3395,130 +3400,6 @@ fn asyncness(tcx: TyCtxt<'_>, def_id: DefId) -> hir::IsAsync {
fn_like.asyncness()
}

pub enum NonStructuralMatchTy<'tcx> {
Adt(&'tcx AdtDef),
Param,
}

/// This method traverses the structure of `ty`, trying to find an
/// instance of an ADT (i.e. struct or enum) that was declared without
/// the `#[structural_match]` attribute, or a generic type parameter
/// (which cannot be determined to be `structural_match`).
///
/// The "structure of a type" includes all components that would be
/// considered when doing a pattern match on a constant of that
/// type.
///
/// * This means this method descends into fields of structs/enums,
/// and also descends into the inner type `T` of `&T` and `&mut T`
///
/// * The traversal doesn't dereference unsafe pointers (`*const T`,
/// `*mut T`), and it does not visit the type arguments of an
/// instantiated generic like `PhantomData<T>`.
///
/// The reason we do this search is Rust currently require all ADTs
/// reachable from a constant's type to be annotated with
/// `#[structural_match]`, an attribute which essentially says that
/// the implementation of `PartialEq::eq` behaves *equivalently* to a
/// comparison against the unfolded structure.
///
/// For more background on why Rust has this requirement, and issues
/// that arose when the requirement was not enforced completely, see
/// Rust RFC 1445, rust-lang/rust#61188, and rust-lang/rust#62307.
pub fn search_for_structural_match_violation<'tcx>(
tcx: TyCtxt<'tcx>,
ty: Ty<'tcx>,
) -> Option<NonStructuralMatchTy<'tcx>> {
let mut search = Search { tcx, found: None, seen: FxHashSet::default() };
ty.visit_with(&mut search);
return search.found;

struct Search<'tcx> {
tcx: TyCtxt<'tcx>,

// Records the first ADT or type parameter we find without `#[structural_match`.
found: Option<NonStructuralMatchTy<'tcx>>,

// Tracks ADTs previously encountered during search, so that
// we will not recurse on them again.
seen: FxHashSet<hir::def_id::DefId>,
}

impl<'tcx> TypeVisitor<'tcx> for Search<'tcx> {
fn visit_ty(&mut self, ty: Ty<'tcx>) -> bool {
debug!("Search visiting ty: {:?}", ty);

let (adt_def, substs) = match ty.kind {
ty::Adt(adt_def, substs) => (adt_def, substs),
ty::Param(_) => {
self.found = Some(NonStructuralMatchTy::Param);
return true; // Stop visiting.
}
ty::RawPtr(..) => {
// `#[structural_match]` ignores substructure of
// `*const _`/`*mut _`, so skip super_visit_with
//
// (But still tell caller to continue search.)
return false;
}
ty::FnDef(..) | ty::FnPtr(..) => {
// types of formals and return in `fn(_) -> _` are also irrelevant
//
// (But still tell caller to continue search.)
return false;
}
ty::Array(_, n) if n.try_eval_usize(self.tcx, ty::ParamEnv::reveal_all()) == Some(0)
=> {
// rust-lang/rust#62336: ignore type of contents
// for empty array.
return false;
}
_ => {
ty.super_visit_with(self);
return false;
}
};

if !self.tcx.has_attr(adt_def.did, sym::structural_match) {
self.found = Some(NonStructuralMatchTy::Adt(&adt_def));
debug!("Search found adt_def: {:?}", adt_def);
return true; // Stop visiting.
}

if !self.seen.insert(adt_def.did) {
debug!("Search already seen adt_def: {:?}", adt_def);
// let caller continue its search
return false;
}

// `#[structural_match]` does not care about the
// instantiation of the generics in an ADT (it
// instead looks directly at its fields outside
// this match), so we skip super_visit_with.
//
// (Must not recur on substs for `PhantomData<T>` cf
// rust-lang/rust#55028 and rust-lang/rust#55837; but also
// want to skip substs when only uses of generic are
// behind unsafe pointers `*const T`/`*mut T`.)

// even though we skip super_visit_with, we must recur on
// fields of ADT.
let tcx = self.tcx;
for field_ty in adt_def.all_fields().map(|field| field.ty(tcx, substs)) {
if field_ty.visit_with(self) {
// found an ADT without `#[structural_match]`; halt visiting!
assert!(self.found.is_some());
return true;
}
}

// Even though we do not want to recur on substs, we do
// want our caller to continue its own search.
false
}
}
}

pub fn provide(providers: &mut ty::query::Providers<'_>) {
context::provide(providers);
erase_regions::provide(providers);
Expand Down
Loading