mem::discriminant()

[durka]

The discriminant_value intrinsic won't be stabilized anytime soon in its current form because it leaks too much memory layout information. The wrapper proposed here returns an opaque newtype to allow for information hiding.

cc rust-lang/rust#24263 rust-lang/rust#34785 @ubsan @sfackler

Rendered

[sfackler]

Is there anything left to resolve here? The discussion on the PR seems to have reached a conclusion already.

[strega-nil]

Remove the Reflect bound. It's only used on one function in the standard library, and parametricity in Rust is completely broken, with size_of, size_of_val, and specialization.

With specialization (I'm not sure how to do it, but):

fn type_id<T: 'static>(t: &T) TypeId { panic!() }
/* specialize */ fn type_id<T: Any>(t: &T) -> TypeId { t.get_type_id() }

Now you can get the TypeId of any 'static type in the Rust universe.

[nagisa]

@sfackler there has been a demand for a RFC, which is a pretty good indication that one is necessary, even if there’s already some consensus.

[durka]

I removed the Reflect bound. Be sure to chime in if you think it should stay. cc @nikomatsakis @glaebhoerl

[nagisa]

cc @rust-lang/libs because it is technically a libs stuff and also @rust-lang/lang (especially wrt Reflect).

[glaebhoerl]

Sigh.. @ubsan The point, as discussed on the other thread, is that specialization is not yet stable. The Reflect bound here would just be to avoid jumping the gun on ditching parametricity for good and done before specialization itself actually stabilizes. If/when it does, the Reflect trait should probably just be removed entirely. The likelihood that the lang team would have a change of heart over the parametricity issue seems low, but nonetheless we shouldn't tie our own hands unnecessarily over something relatively trivial.

(For completeness's sake - I think the size_of issue could be managed acceptably well if there were a desire to maintain parametricity on a policy level. But as of right now, there isn't, so it doesn't seem worthwhile to sidetrack the discussion in that direction atm.)

[strega-nil]

@glaebhoerl There is no desire to maintain parametricity among the vast majority of Rust users. size_of is not an issue. Specialization is coming. And, it's likely that someone (perhaps me) will be submitting an RFC to remove Reflect. Parametricity has never, and will never be a thing in a systems language.

And making someone's day worse is a good reason to reason to get rid of this idea of "maintaining parametricity".

[durka]

The Reflect bound here would just be to avoid jumping the gun on ditching parametricity for good and done before specialization itself actually stabilizes. If/when it does, the Reflect trait should probably just be removed entirely.

I agree.

[eddyb]

One thing that may not be obvious: you can opt out of Reflect for a type so that not even specialization can work around it, as it's an "OIBIT" (like Send or Sync), but it's unstable.

[nikomatsakis]

Hmm. While I agree that we are unlikely to roll back specialization, I do feel mildly uncomfortable about removing the Reflect bound in just this particular place. That is, I'd sort of rather make a conscious decision to remove Reflect (e.g., through a separate RFC, as @ubsan suggested), than do it sort of "ad hoc" here. Curious what @aturon and rest of @rust-lang/lang thinks.

[durka]

Besides the Reflect question, the other issue that should ideally be settled during the RFC process is Unresolved Question #1:

Can the return value of discriminant(&x) be considered stable between subsequent compilations of the same code? How about if the enum in question is changed by modifying a variant's name? by adding a variant?

[strega-nil]

@nikomatsakis What speaks Reflect to you about this? There is literally one function in the entire standard library which requires a Reflect bound (and functions based on it); it seems un-rustic to require this bound.

Not to mention, to use this function stably would require people to have an Any bound, which is... weird, and adds an extra bound of 'static (assuming this function is stabilized before Reflect is removed, if it is removed).

[durka]

To have an Any bound or to not use it with generics which I think is the main reason to have a bound. But yeah that is annoying, I agree.

[nikomatsakis]

@ubsan it seems clear that it's a kind of reflection, is all -- that said, not one greater in magnitude than size_of. Just "another one". (Whereas specialization or downcasting seems greater in magnitude =)

[strega-nil]

@durka I could imagine someone implementing something, where they wanted to check quickly if two variants were the same. This is a useful small function, if you're doing it a lot for a lot of enums (for example, implementing PartialEq, or implementing a data structure, or something. I'm not sure what it might be used for, only that it might be useful).

fn enum_quick_eq_check<T: Any>(t1: T, t2: T) -> bool {
    std::mem::discriminant(t1) == std::mem::discriminant(t2)
}

With the current bounds, you must write T: Any.

Now, imagine you have an enum like this:

enum Foo<'a> {
    ...
}

You can't use Foo with enum_quick_eq_check, despite there being no real reason for that restriction being there.

(of course, one could always implement a macro, but...)

[nikomatsakis]

Note that Reflect and Any are automatically implemented ... I think you
could use the function with Foo. Am I missing something?

On Aug 2, 2016 5:52 PM, "Nicole Mazzuca" [email protected] wrote:

@durka https://github.com/durka I could imagine someone implementing
something, where they wanted to check quickly if two variants were the
same. This is a useful small function, if you're doing it a lot for a lot
of enums (for example, implementing PartialEq, or implementing a data
structure, or something. I'm not sure what it might be used for, only that
it might be useful).

fn enum_quick_eq_check<T: Any>(t1: T, t2: T) -> bool {
std::mem::discriminant(t1) == std::mem::discriminant(t2)
}

With the current bounds, you must write T: Any.

Now, imagine you have an enum like this:

enum Foo<'a> {
...
}

You can't use Foo with enum_quick_eq_check, despite there being no real
reason for that restriction being there.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1696 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAJeZjex4lUOH6ftGTaen96q6Pfu5Bdgks5qb7w6gaJpZM4JZ3XR
.

[durka]

Any requires 'static.

On Aug 3, 2016 21:52, "Niko Matsakis" [email protected] wrote:

Note that Reflect and Any are automatically implemented ... I think you
could use the function with Foo. Am I missing something?

On Aug 2, 2016 5:52 PM, "Nicole Mazzuca" [email protected] wrote:

@durka https://github.com/durka I could imagine someone implementing
something, where they wanted to check quickly if two variants were the
same. This is a useful small function, if you're doing it a lot for a lot
of enums (for example, implementing PartialEq, or implementing a data
structure, or something. I'm not sure what it might be used for, only
that
it might be useful).

fn enum_quick_eq_check<T: Any>(t1: T, t2: T) -> bool {
std::mem::discriminant(t1) == std::mem::discriminant(t2)
}

With the current bounds, you must write T: Any.

Now, imagine you have an enum like this:

enum Foo<'a> {
...
}

You can't use Foo with enum_quick_eq_check, despite there being no real
reason for that restriction being there.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1696 (comment),
or mute
the thread
<
https://github.com/notifications/unsubscribe-auth/AAJeZjex4lUOH6ftGTaen96q6Pfu5Bdgks5qb7w6gaJpZM4JZ3XR

.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1696 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAC3n0H0BcTYPv8NODIfAugR45Fm_iFqks5qcUXAgaJpZM4JZ3XR
.

[strega-nil]

I could imagine enum_quick_eq_check, with a name change, returning a std::cmp::Ordering, which would make it actually useful as an optimization technique.

[nikomatsakis]

Ah, so the problem is that Any requires 'static even if Reflect does not. Fair enough. If Reflect were stable, that'd be no big deal, but then we expect to remove Reflect anyhow.

I'm shifting my opinion slightly. It does seem like it'd be nice to have a separate RFC that talks about Reflect, but maybe it's ok to decide here "officially" that we are not going to try and preserve (what remains of) parametricity. It's not clear to me that this is a bigger hole than e.g. size_of -- I guess it is somewhat since it gives a property not just of the type but of that specific instance, even if it's not a very flexible property.

I really don't see us backing away from specialization unless there is some horrible flaw that makes the design incoherent, and even then I might imagine us trying to come up with some alternative that achieves similar goals.

[nikomatsakis]

(I mean, by accepting specialization we already decided the intent to drop parametricity, clearly.)

[glaebhoerl]

Basically what I had in the back of my mind is that, as far as I've seen, Haskellers are almost to a person surprised and disappointed that Rust is dropping parametricity, but there's this self-reinforcing sorting effect at work whereby they don't feel like it's worth getting involved with Rust, because they feel like it doesn't share their values/priorities, and so the Rust community tends to be populated by those people who already don't care about it or aren't bothered by it. But what if at some point someone does decide to involve themselves, and makes the case more persuasively than I could. Just in case.

(For what it's worth -- I do think discriminant_value would be a much bigger hole than size_of. The reason I think size_of is not too bad is because it provides essentially no semantically meaningful information (except, maybe, in the case where the size is 0) -- the only thing you can really use it for is low-level things like memory allocation, which inevitably end up getting hidden behind an API that once more behaves uniformly across types. There's no incentive to abuse it, in other words. That's very much not the case with discriminant_value, as ably demonstrated by the enum_quick_eq_check example. My idea for what to do about size_of in a world where we wanted to keep parametricity was, in fact, to just say "generic functions shall have uniform behavior across all type parameters with the same size" (modulo trait bounds etc), which lets low-level layout-calculating code do whatever it wants without fear, and other code has no reason to care about sizes, so for practical purposes it ends up being as parametric as you would expect. But you really can't accommodate discriminant_value in the same way without watering it down into uselessness.)

[petrochenkov]

as far as I've seen, Haskellers are almost to a person surprised and disappointed that Rust is dropping parametricity

After using specialization in C++ for years I was similarly surprised by people defending parametricity. "Are these people mad?" - asked I myself - "Why does this abstract property have any value for them? What useful practical guarantees does it give? What guarantees can it give at all, given that it couldn't save dropck, the only thing it was used for?" After reading dozens and dozens of pages of discussion following RFC 1210 I still ask myself these questions...

Okay, this is an offtopic, this PR is for discussing mem::discriminant 😄

[glaebhoerl]

(as I was saying - this isn't the best place to ask those questions)

[aturon]

Nominated.

[durka]

Was it discussed at the meeting?

[aturon]

@durka It was, and will be entering FCP as soon as the team has a moment to write up the summary for the FCP comment (likely tomorrow).

[nikomatsakis]

On a related note, we decided to put the Reflect trait into FCP, with the intention of deprecating and removing it. That discussion would take place on the tracking issue. I will try to copy over some relevant comments there though so that people need not repeat themselves.

[nikomatsakis]

Hear ye, hear ye! This RFC is now entering final comment period. The @rust-lang/lang team discussed it and we are inclined to accept.

Major points of discussion follow (this also includes comments from the PR):

• the return type was initially a bare u64, but this was changed to a new type:
  • the newtype supports ordering, equality, hashing etc ([RFC accepted] add wrapper for discriminant_value intrinsic rust#34785 (comment))
  • this will be future compatible with 128-bit discriminants and handles signed discriminants better, resolving this concern ([RFC accepted] add wrapper for discriminant_value intrinsic rust#34785 (comment))
• whether to support the Ord trait ([RFC accepted] add wrapper for discriminant_value intrinsic rust#34785 (comment))
  • currently no, but maybe later
• whether to have a Reflect bound (see also Tracking issue for Reflect stabilization rust#27749)
  • there is general agreement that this point is moot after specialization, but specialization is not yet stable
  • maybe a T: Enum trait instead ([RFC accepted] add wrapper for discriminant_value intrinsic rust#34785 (comment))
  • that feels like overkill to some ([RFC accepted] add wrapper for discriminant_value intrinsic rust#34785 (comment))
  • there was also some general discussion on the value of parametricity (e.g., mem::discriminant() #1696 (comment), mem::discriminant() #1696 (comment))
  • this can be settled to some extent during stabilization; note that we have put the reflect trait itself into FCP as well
• what kinds of stability guarantees follow from this fn? ([RFC accepted] add wrapper for discriminant_value intrinsic rust#34785 (comment))
  • consumers shouldn't depend on stability across versions unless specifically guaranteed ([RFC accepted] add wrapper for discriminant_value intrinsic rust#34785 (comment))
  • there was some back and forth on how much freedom the compiler had to change discriminants ([RFC accepted] add wrapper for discriminant_value intrinsic rust#34785 (comment), [RFC accepted] add wrapper for discriminant_value intrinsic rust#34785 (comment))
• whether to have some kind of HasDiscriminant trait ([RFC accepted] add wrapper for discriminant_value intrinsic rust#34785 (comment))
  • conclusion: we can have this fn, and add such a trait later ([RFC accepted] add wrapper for discriminant_value intrinsic rust#34785 (comment))

@rust-lang/lang and @rust-lang/libs members, please check off your name to signal agreement. Leave a comment with concerns or objections. Others, please leave comments. Thanks!

• @nikomatsakis
• @nrc
• @aturon
• @eddyb
• @pnkfelix (on vacation)
• @brson
• @alexcrichton
• @BurntSushi
• @sfackler
• @Kimundi

[alexcrichton]

@rust-lang/libs concluded today we approve as a team going into FCP for merging.

[briansmith]

the return type was initially a bare u64, but this was changed to a new type:

My concern is that my application needs the hinted-at Discriminant::into_inner, but this RFC seems to be designed specifically to defer adding that functionality. Let's add implementation of Discriminant::into_inner for #[repr(<integer type>)] types too. Applications that need this functionality would probably be using it to implement strongly-typed parsers and serializers.

[durka]

I guess I should state my opinions on the unresolved questions:

1. I don't think we need a Reflect bound, especially if that FCP ends up as a deprecation. We can worry about an Enum or HasDiscriminant bound later.
2. In terms of stability, I think the discriminants of an unchanged enum should not change if you compile again with the same version of rustc. We'll need this eventually for reproducible builds so I think it's not a problem to pin it down now.

[durka]

@briansmith the problem is we have no way to deal with the repr types, in the type system. We must have the HasDiscriminant trait that I described or something like it to do that. An opaque newtype can paper over this shortcoming for now (and support it when the type system is better).

[Diggsey]

If rust-lang/rust#8995 is implemented, it would be possible to just make Discriminant<T>::Repr be a "magic" associated type which is unit for non-enums, then you don't need to worry about additional trait bounds.

[nikomatsakis]

@briansmith do you plan to invoke this on arbitrary enum types that are out of your control? or would a derive-based sol'n be adequate.

I agree it'd be nice to expose the #[repr] types but I am reluctant to integrate this trait deeply into the compiler.

[briansmith]

@briansmith do you plan to invoke this on arbitrary enum types that are out of your control? or would a derive-based sol'n be adequate.

In my case, I define the enums and I write the code that uses them. See

• https://github.com/briansmith/ring/blob/c56690d356acc9d4ced4197b0090d473927666b9/src/der.rs#L48
• https://github.com/briansmith/ring/blob/a7b02de2dcd313d6c7d72fecd82b7ecc9c8eeb71/src/rsa/rsa.rs#L191-L195
• https://github.com/briansmith/webpki/blob/272de411320286c572a428b1ee0954ec2cfa0ddb/src/der.rs#L86
• https://github.com/briansmith/webpki/blob/272de411320286c572a428b1ee0954ec2cfa0ddb/src/der.rs#L115
• https://github.com/briansmith/webpki/blob/272de411320286c572a428b1ee0954ec2cfa0ddb/src/name.rs#L56

Basically, I'm just looking for a safer version of as, one that guarantees it won't truncate a larger value.

[durka]

This has now been in FCP for 4 weeks.

[bluss]

What remains to be done before this can be closed? Just curious, since the teams post says inclined to accept, but that was a long time ago.

[aturon]

@durka Whelp, my turn to apologize for the delay yet again. Between RustConf and the shift away from meetings (but without a dashboard just yet), the teams have fallen behind on administrative work.

None of the commentary during FCP changes this RFC in a deep way -- there are some possible extensions we may want to pursue, but these need not block landing the current proposal as a starting point. Given that the lang and libs teams are also in agreement, I'm going to merge.

[durka]

I'll update the PR and create a tracking issue later today.

On Thu, Sep 15, 2016 at 11:53 AM, Aaron Turon [email protected]
wrote:

Merged #1696 #1696.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1696 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAC3nwodg_mKN_-jt_smcVqWQbPR1-Zaks5qqWoBgaJpZM4JZ3XR
.

[Ericson2314]

It just occurred to me that Discriminant<T>::Repr would also be useful if we optimize away empty variants (e.g. Result<T, !>).

[Stargateur]

How could I have the discriminant value without create the real variant ?

Example:

use std::collections::HashMap;
use std::mem::discriminant;
use std::net::Ipv4Addr;

#[derive(Debug, Eq, PartialEq, Hash, Clone)]
pub enum GenericChunk<'a> {
    IpProtocolFamily(u8),
    Ipv4SourceAddr(Ipv4Addr),
    AuthenticationKey(&'a [u8]),
}

fn main() {
    let mut chunks = HashMap::new();

    chunks.insert(
        discriminant(&GenericChunk::IpProtocolFamily(0)),
        GenericChunk::IpProtocolFamily(42),
    );

    assert_eq!(
        chunks.get(&discriminant(&GenericChunk::IpProtocolFamily(0))),
        Some(&GenericChunk::IpProtocolFamily(42))
    );

    assert_eq!(
        chunks.get(&discriminant(&GenericChunk::AuthenticationKey(b"" as _))),
        None
    );
}

Here, I'm force to create a temporary value in order to have the discriminant, even if discriminant don't really need it.