Implement a discriminant_value intrinsic

[Aatch]

Implements an intrinsic for extracting the value of the discriminant
enum variant values. For non-enum types, this returns zero, otherwise it
returns the value we use for discriminant comparisons. This means that
enum types that do not have a discriminant will also work in this
arrangement.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see CONTRIBUTING.md for more information.

[sfackler]

cc #20856

[Aatch]

For comparison, with @sfackler's SqlState enum, the difference between the derived and manual impls using this intrinsic is this: Gist

[alexcrichton]

cc #13860, a discussion on a topic somewhat related to this awhile ago.

Note that for derive to take advantage of this intrinsic we'd need to consider the interaction of generated code via derive and how it's checked for stability to ensure we're not forcing warnings about calls to unstable code.

[sfackler]

How hard would it be to generate an error when monomorphizing discriminant_value when T isn't an enum?

[luqmana]

@sfackler it would be simple enough, the call_info argument to trans_intrinsic_call even gives you a span.

[huonw]

That sounds good to me, although I think we would then have to restrict discriminant_value to only being called on types for which the enum-ness and discriminants are known at the call site, like transmute, or else we'd get the bad errors that invalid transmutes used to give due to monomorphisation of an invalid transmute deep inside other (cross-crate) functions.

Possible optimisations like #14540 may mean that the rules have to be quite restrictive, but that doesn't seem like such a problem. However, it would be unstable at 1.0 and is mostly an implementation detail, but using it in PartialEq may be tricky in that case as @alexcrichton points out (especially so under this scheme, since there's no way to have a stable generic wrapper).

[alexcrichton]

I agree with @huonw that monomorphization should never generate a compile-time error wherever reasonable. One other possibility could be perhaps to return Option<u64> where Some is only returned for true enums.

I'm also a little curious about how we'd spec this in terms of enumerations such as Option<Box<T>> because the discriminant isn't actually stored in memory anywhere. We can numerically label each variant, but it doesn't actually correspond to any layout in memory.

[sfackler]

None and Some for an Option<Box<T>> could still have "conceptual" discriminants - 0, and 1 respectively. It seems reasonable that the intrinsic would always be able to return them, whether the implementation's just snagging the tag out of memory, or doing something more advanced as in the case of Option<Box<T>>.

[Aatch]

Just to clarify for those that aren't as familiar with this part of the compiler internals, we always have a discriminant value for an enum, sometimes we read from a memory location, sometimes we do a comparison against a pointer value, but there is always an discriminant.

Right now, I don't think the specific values need to be specced at all. I think that, if we decide to spec anything here at all, it should simply be that the value you get is guaranteed only to be the same value as any another instance of the variant of the same type. This does mean we can't use the value for ordering, but it also means we don't have to worry about the various representations.

[nikomatsakis]

I'm not sure there's a problem with giving an actual integer result, and spec'ing that the results count from 0, but we can be cautious here for the time being. It's hard for me to imagine a representation that wouldn't efficiently allow us to get to an integer (Option<Box<T>> is not a particular problem, for example). Perhaps @jld will have an opinion, not sure.

[alexcrichton]

(previous comment: #21186 (comment))

Ok, we also ended up chatting about this PR in the meeting today as well! We think that a similar strategy may be able to happen here as well. As is this is a pretty unobtrusive feature, and the perf wins it may gain us are certainly quite enticing!

Our conclusion was that if we have a concrete use case for this in the standard libraries that it'd be a good thing to land, but otherwise it may want to hold of for an RFC. I'd of course be willing to help out writing an RFC and sheperding it through as well!

In general we'd just want to make sure that it's put to good use in the standard library before landing an implementation in the compiler. Thanks again for the awesome perf work here!

[taralx]

Went looking for a "get discriminant" trait today in order to implement EnumSet. If this were a (compiler-generated?) trait instead of an intrinsic, the monomorphization wouldn't be a problem...

[alexcrichton]

cc rust-lang/rfcs#639 (corresponding RFC)

[bors]

☔ The latest upstream changes (presumably #23153) made this pull request unmergeable. Please resolve the merge conflicts.

[alexcrichton]

For now I'm going to close this in favor of the RFC to see what happens there.