Update documentation for arbitrary_enum_discriminant feature by jswrenn · Pull Request #639 · rust-lang/reference

jswrenn · 2019-07-14T20:36:17Z

Updates documentation to reflect the still unstable arbitrary_enum_discriminantfeature (tracking issue: rust-lang/rust#60553).

Havvy

Lots of good work. ❤️

Lots of little changes requested; a couple bigger ones.

Havvy · 2019-07-15T06:01:20Z

src/items/enumerations.md

+<ol>
+<li>
+
+if the enumeration is "C-like" (i.e., it has no tuple or struct variants); e.g.:


Suggested change

if the enumeration is "C-like" (i.e., it has no tuple or struct variants); e.g.:

if the enumeration is field-less. For example:

Per #639 (comment), these terms may not be truly interchangeable.

Havvy · 2019-07-15T06:02:04Z

src/items/enumerations.md

+if the enumeration is "C-like" (i.e., it has no tuple or struct variants); e.g.:
+
+```rust
+# #![feature(arbitrary_enum_discriminant)]


Feature flags aren't needed; the prose will only be merged after the feature is stable.

Suggested change

# #![feature(arbitrary_enum_discriminant)]

src/items/enumerations.md

Havvy · 2019-07-15T06:04:39Z

src/items/enumerations.md

+
+#### Casting
+
+If there is no data attached to *any* of the variants of an enumeration, then


There's no need to describe a field-less enum here. Just use the term.

Suggested change

If there is no data attached to *any* of the variants of an enumeration, then

If the enum is a field-less enum, then

Per #639 (comment), these terms may not be completely interchangeable. If "field-less" is meant to be synonymous to "C-like", then this change would be incorrect.

Havvy · 2019-07-15T06:07:46Z

src/items/enumerations.md

-If there is no data attached to *any* of the variants of an enumeration,
-then the discriminant can be directly chosen and accessed.
+Each enum instance has a _discriminant_: an integer logically associated to it
+that is used to determine which variant it holds. An opaque reference to this


Using mem::discriminant should be in the accessing the discriminant section.

Havvy · 2019-07-15T06:23:01Z

src/items/enumerations.md

-integer associated to it that is used to determine which variant it holds. An
-opaque reference to this discriminant can be obtained with the
-[`mem::discriminant`] function.
+called an enum variant.


The re-organization looks fine, but we lose our definition of a field-less enum. This suggestion re-adds it. I don't actually know if the anchor actually works. It should just create an anchor that can be linked too without changing any styles or making it clickable.

Suggested change

called an enum variant.

called an enum variant.

An enum where no constructors contain fields are called a *<a name="field-less-enum">field-less enum</a>*.

Havvy · 2019-07-15T06:24:10Z

src/items/enumerations.md

+if a [primitive representation] is used; e.g.:
+
+```rust
+# #![feature(arbitrary_enum_discriminant)]


Feature flag not needed.

Suggested change

# #![feature(arbitrary_enum_discriminant)]

src/items/enumerations.md

Havvy · 2019-07-15T06:30:56Z

src/items/enumerations.md

+an `isize` value. However, the compiler is allowed to use a smaller type (or
+another means of distinguishing variants) in its actual memory layout.
+
+If the [primitive representation] or the [`C` representation] is used, the


I don't understand what this paragraph is trying to say.

Havvy · 2019-07-15T06:43:11Z

src/items/enumerations.md

+leading bytes of a variant (e.g., two bytes if `#[repr(u16)]` is used), will
+correspond exactly to the discriminant.
+
+### Assigning Discriminant Values


For this entire section, it only makes sense for field-less enums or enums with a primitive representation (or the C representation?). The way it is written, it seems like explicit discriminants are only for those and implicit applies to all. Instead, it should state that having a non-opaque (feel free to pick a different descriptor) discriminant applies in those situations and in all others, it is opaque and must only be accessed via mem::discriminant.

Co-Authored-By: Ryan Scheel <Ryan.havvy@gmail.com>

jswrenn · 2019-07-15T18:24:00Z

Thank you so much for your review, @Havvy! If it's alright, I'm going to defer removing the feature flags until this gets a little closer to being merged; otherwise, I can't run mdbook tests.

Terminology

I have a lot of uncertainty about terminology. I only just saw that there's prior agreement (rust-lang/rust#46348, #244) to move away from the term "C-like enumeration" (which I believe is a very poor term) and towards "field-less enumeration". I am thrilled to strike the phrase "C-like" from this PR.

However, using "field-less enumeration" as a synonym for "C-like" isn't quite appropriate.

A C-like enumeration is one in which every variant is unit-like; for example:

enum CLike {
  VariantA,
  VariantB,
  VariantC,
}

All C-like enumerations are field-less, because they cannot include tuple-like or struct-like variants. However, not all field-less enums are are C-like. For instance:

enum Fieldless {
    Unit,
    Tuple(),
    Struct{},
}

This distinction unfortunately (ugh) matters:

Only C-like enums may specify explicit discriminant without specifying a repr.
Only field-less enums (regardless of whether they're also C-like) may be as-casted to their discriminant values.

It would be nice to have terms to describe both of these sets. My opinion:

"Field-less" is the right term to describe the set of enums which can be as-casted
However it cannot then also be used as a synonym for "C-like", so we should identify an alternative term to "C-like" to describe enums in which all variant are unit-like.

Havvy · 2019-07-16T16:16:39Z

Well, that is quite unfortunate of an edge case. I'm failing to think of any good names.

ehuss · 2019-07-16T22:29:39Z

@jswrenn Just letting you know that the reference has migrated to mdbook 0.3 (from 0.1). This means that the style of links are slightly different. They should use the .md extension, and are relative to the page they are on (previously they were relative to the root of the book). It looks like this PR should be fairly easy to rebase to resolve the conflicts. Please don't hesitate to ask if you have any questions or any trouble rebasing.

fee1-dead · 2021-06-23T03:06:21Z

@jswrenn Are you still working on this? If not, I would like to work on it.

JohnTitor · 2021-06-23T04:31:05Z

~~According to the label, this is waiting for stabilization~~ Oh, I found you posted the report (rust-lang/rust#60553 (comment)), nvm!

jswrenn · 2021-07-06T15:23:22Z

@jswrenn Are you still working on this? If not, I would like to work on it.

I'm not! Go for it!

(Sorry for the delayed reply; I thought I had already replied.)

Stabilize `arbitrary_enum_discriminant` Closes rust-lang#60553. ---- ## Stabilization Report _copied from rust-lang#60553 (comment) ### Summary Enables a user to specify *explicit* discriminants on arbitrary enums. Previously, this was hard to achieve: ```rust #[repr(u8)] enum Foo { A(u8) = 0, B(i8) = 1, C(bool) = 42, } ``` Someone would need to add 41 hidden variants in between as a workaround with implicit discriminants. In conjunction with [RFC 2195](https://github.com/rust-lang/rfcs/blob/master/text/2195-really-tagged-unions.md), this feature would provide more flexibility for FFI and unsafe code involving enums. ### Test cases Most tests are in [`src/test/ui/enum-discriminant`](https://github.com/rust-lang/rust/tree/master/src/test/ui/enum-discriminant), there are two [historical](https://github.com/rust-lang/rust/blob/master/src/test/ui/parser/tag-variant-disr-non-nullary.rs) [tests](https://github.com/rust-lang/rust/blob/master/src/test/ui/parser/issue-17383.rs) that are now covered by the feature (removed by this pr due to them being obsolete). ### Edge cases The feature is well defined and does not have many edge cases. One [edge case](rust-lang#70509) was related to another unstable feature named `repr128` and is resolved. ### Previous PRs The [implementation PR](rust-lang#60732) added documentation to the Unstable Book, rust-lang/reference#1055 was opened as a continuation of rust-lang/reference#639. ### Resolution of unresolved questions The questions are resolved in rust-lang#60553 (comment). ---- (someone please add `needs-fcp`)

ehuss · 2022-11-09T20:31:24Z

Closing as this has been continued by #1055.

Update enum documentation for arbitrary_enum_discriminant feature.

38516b0

jswrenn mentioned this pull request Jul 14, 2019

Tracking issue for RFC 2363, "Allow arbitrary enums to have explicit discriminants" rust-lang/rust#60553

Closed

5 tasks

Havvy suggested changes Jul 15, 2019

View reviewed changes

Havvy added Language Cleanup Improvements to existing language which is correct but not clear, or missing examples, or the like. New Content Missing features or aspects of language not currently documented. RFC Stabilization Docs Documentation required for stabilizing a feature labels Jul 15, 2019

"e.g." -> "for example"

ac9c117

Co-Authored-By: Ryan Scheel <Ryan.havvy@gmail.com>

jswrenn mentioned this pull request Jul 25, 2019

Enum Layout Guarantees rust-lang/unsafe-code-guidelines#177

Closed

ehuss added the S-waiting-on-stabilization Waiting for a stabilization PR to be merged in the main Rust repository label Apr 7, 2020

This was referenced Jul 4, 2021

Automatically implement AsRepr and allow deriving FromRepr for fieldless enums rust-lang/rust#81642

Closed

Update documentation for arbitrary_enum_discriminant feature #1055

Merged

Stabilize arbitrary_enum_discriminant rust-lang/rust#86860

Merged

ehuss closed this Nov 9, 2022

	if the enumeration is "C-like" (i.e., it has no tuple or struct variants); e.g.:
	if the enumeration is field-less. For example:


		#### Casting

		If there is no data attached to any of the variants of an enumeration, then

	If there is no data attached to any of the variants of an enumeration, then
	If the enum is a field-less enum, then

Conversation

jswrenn commented Jul 14, 2019

Uh oh!

Havvy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jswrenn commented Jul 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Terminology

Uh oh!

Havvy commented Jul 16, 2019

Uh oh!

ehuss commented Jul 16, 2019

Uh oh!

fee1-dead commented Jun 23, 2021

Uh oh!

JohnTitor commented Jun 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jswrenn commented Jul 6, 2021

Uh oh!

ehuss commented Nov 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jswrenn commented Jul 15, 2019 •

edited

Loading

JohnTitor commented Jun 23, 2021 •

edited

Loading