Skip to content

Commit

Permalink
Migrate from #[structural_match] attribute a lang-item trait.
Browse files Browse the repository at this point in the history
(Or more precisely, a pair of such traits: one for `derive(PartialEq)` and one
for `derive(Eq)`.)

((The addition of the second marker trait, `StructuralEq`, is largely a hack to
work-around `fn (&T)` not implementing `PartialEq` and `Eq`; see also issue
rust-lang#46989; otherwise I would just check if `Eq` is implemented.))

Note: this does not use trait fulfillment error-reporting machinery; it just
uses the trait system to determine if the ADT was tagged or not. (Nonetheless, I
have kept an `on_unimplemented` message on the new trait for structural_match
check, even though it is currently not used.)

Note also: this does *not* resolve the ICE from rust-lang#65466, as noted
in a comment added in this commit. Further work is necessary to resolve that and
other problems with the structural match checking, especially to do so without
breaking stable code (adapted from test fn-ptr-is-structurally-matchable.rs):

```rust
fn r_sm_to(_: &SM) {}

fn main() {
    const CFN6: Wrap<fn(&SM)> = Wrap(r_sm_to);
    let input: Wrap<fn(&SM)> = Wrap(r_sm_to);
    match Wrap(input) {
        Wrap(CFN6) => {}
        Wrap(_) => {}
    };
}
```

where we would hit a problem with the strategy of unconditionally checking for
`PartialEq` because the type `for <'a> fn(&'a SM)` does not currently even
*implement* `PartialEq`.

----

added review feedback:
* use an or-pattern
* eschew `return` when tail position will do.
* don't need fresh_expansion; just add `structural_match` to appropriate `allow_internal_unstable` attributes.

also fixed example in doc comment so that it actually compiles.
  • Loading branch information
pnkfelix committed Oct 25, 2019
1 parent 620083a commit 98f5b11
Show file tree
Hide file tree
Showing 16 changed files with 508 additions and 172 deletions.
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
4 changes: 3 additions & 1 deletion src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ pub use self::context::{

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

pub use self::structural_match::{search_for_structural_match_violation, NonStructuralMatchTy};
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;

Expand Down
Loading

0 comments on commit 98f5b11

Please sign in to comment.