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

Tracking issue for RFC 2102, "Unnamed fields of struct and union type" #49804

Open
4 tasks
Centril opened this issue Apr 9, 2018 · 37 comments
Open
4 tasks

Tracking issue for RFC 2102, "Unnamed fields of struct and union type" #49804

Centril opened this issue Apr 9, 2018 · 37 comments
Labels
B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC E-help-wanted Call for participation: Help is requested to fix this issue. F-unnamed_fields `#![feature(unnamed_fields)]` S-tracking-design-concerns Status: There are blocking design concerns. S-tracking-impl-incomplete Status: The implementation is incomplete. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Centril
Copy link
Contributor

Centril commented Apr 9, 2018

This is a tracking issue for the RFC "Unnamed fields of struct and union type" (rust-lang/rfcs#2102).
The feature gate for the issue is #![feature(unnamed_fields)].

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

Unresolved questions

None so far.

Implementation history

@Centril Centril added B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. T-lang Relevant to the language team, which will review and decide on the PR/issue. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels Apr 9, 2018
@joshtriplett
Copy link
Member

@Centril Regarding unresolved questions: the first three paragraphs in that section are just talking about the future possibility of full unnamed types that can appear everywhere, which would be the subject of a future RFC. The last paragraph talks about other future possibilities that also need their own RFCs.

@Centril
Copy link
Contributor Author

Centril commented Apr 9, 2018

@joshtriplett Yeah; that was also my interpretation, so ==> no unresolved questions?

@cramertj
Copy link
Member

Looks like this still needs an implementation

@igotfr

This comment has been minimized.

@joshtriplett

This comment has been minimized.

@joshtriplett joshtriplett added the E-help-wanted Call for participation: Help is requested to fix this issue. label Aug 10, 2020
@ibraheemdev
Copy link
Member

Is there any movement on this, or is it just waiting for someone to implement it?

@varkor
Copy link
Member

varkor commented Mar 9, 2021

This is still waiting on an implementation.

@ibraheemdev
Copy link
Member

@varkor maybe mentoring instructions would help someone get started?

@varkor
Copy link
Member

varkor commented Mar 13, 2021

I don't have time to write up full mentoring instructions at the moment, but if someone is interested in implementing this feature, I am happy to provide pointers for how to get started on Zulip.

  • The first place I'd look is parse_name_and_ty in compiler/rustc_parse/src/parser/item.rs, where you'll want to start parsing an underscore, along with the rest of the syntax for unnamed fields as described in the RFC.
  • This will entail modifying StructField in compiler/rustc_ast/src/ast.rs to contain one of two types: either a NamedStructField (containing most of what StructField currently contains) along with an UnnamedStructField, whose structure should be evident from the description in the RFC.
  • Having done this, you should gate the new feature by creating a new unnamed_fields feature gate in compiler/rustc_feature/src/active.rs, which should be added to INCOMPLETE_FEATURES, and then checking for UnnamedStructField in compiler/rustc_ast_passes/src/feature_gate.rs.
  • This should cover at least up to Parsing in the RFC.

@varkor varkor added the E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. label Mar 13, 2021
@jedel1043
Copy link
Contributor

jedel1043 commented Apr 18, 2021

I'm interested on implementing this, but I need some mentoring to answer some questions I have.

bors added a commit to rust-lang-ci/rust that referenced this issue May 17, 2021
…enkov

Parse unnamed fields of struct and union type

Added the `unnamed_fields` feature gate.

This is a prototype of [RFC 2102](rust-lang#49804), so any suggestions are greatly appreciated.

r? `@petrochenkov`
@jedel1043
Copy link
Contributor

Ok, the parsing step has been completed. The next step is to lower the new types from the AST to the HIR. Could someone give me some recommendations on how to approach this?

@joshtriplett joshtriplett added S-tracking-unimplemented Status: The feature has not been implemented. and removed E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Apr 27, 2022
@joshtriplett
Copy link
Member

Is there still someone able to provide mentorship for implementing this?

Is there still someone available to try implementing this?

@nwn
Copy link

nwn commented Apr 27, 2022

I would be interested to try implementing this, but would defer to the previous implementer if they are still interested.

@jedel1043
Copy link
Contributor

I would be interested to try implementing this, but would defer to the previous implementer if they are still interested.

Please do so! I'm somewhat busy nowadays, so I'd prefer leaving this work to someone who can dedicate their full attention to it :)

@fmease fmease added S-tracking-impl-incomplete Status: The implementation is incomplete. and removed S-tracking-unimplemented Status: The feature has not been implemented. labels Feb 21, 2024
@RalfJung
Copy link
Member

RalfJung commented Mar 1, 2024

Unresolved question: what should derive macros do here? This applies both to the built-in ones and user-defined ones. It seems like they all need major overhaul to support types like this. And it is pretty inevitable that people will ask for derive to be supported on these types, even if the MVP does not support them.

OTOH I assume many of them don't support unions to begin with, and these unnamed fields only really make sense when there are unions involved I think?

The RFC also explicitly lists anonymous types as a rejected alternative, and yet the implementation that recently began for this RFC does introduce anonymous ADTs to the compiler. Though maybe if it is impossible to write an expression of these types they are less problematic? That said I assume in the internal compiler IRs such expressions will exist -- the unnamed fields are getting an internal name and field accesses are desugared to use those names.

It's that kind of issue that makes me think that adding a new form of unnamed types to Rust (on top of closures/coroutines) is a mistake. The RFC was accepted 6 years ago, our approach to language design and evolution changed since then. I think we need to ensure that this is even still something we want to do in this form.

@fmease
Copy link
Member

fmease commented Mar 3, 2024

#49804 (comment)

Nominating Ralf's comment for T-lang discussion. Context for T-lang: There's currently active compiler dev going on to implement this feature (several merged and open PRs by multiple contributors). I don't want them to continue working on it if it gets thrown out in the end.

@fmease fmease added the I-lang-nominated Nominated for discussion during a lang team meeting. label Mar 3, 2024
@Jules-Bertholet
Copy link
Contributor

The RFC also explicitly lists anonymous types as a rejected alternative, and yet the implementation that recently began for this RFC does introduce anonymous ADTs to the compiler.

I think this is the wrong implementation approach. The RFC doesn't specify the introduction of any new types. My expectation would be that this feature would be implemented by storing the field list in a tree structure instead of a flat list, for example in VariantDef.fields and in VariantData (ast, hir).

@frank-king
Copy link
Contributor

Oh, previous implementation (#115732) of parsing unnamed fields as a special field kind made things very complicated and was rejected.

Was #115732 the right way to implement this feature?

@RalfJung
Copy link
Member

RalfJung commented Mar 4, 2024

If they give rise to "tree-shaped types" then should we enforce the tree to strictly alternate between struct and union, i.e., not accept struct fields inside a struct or union fields inside a union? Those would be redundant then and give rise to questions like whether they actually change how the layout is computed.

Also, that would require pretty fundamental changes to FieldsShape, which currently encodes the assumption that a type is either struct-like or union-like. This distinction has pretty fundamental consequences also for type validity, i.e. what the valid representations of a type are. So for instance

struct S {
  f1: bool,
  _: union { f2: u8, f3: u8 }
}

Should this entire type behave like a union (any byte sequence is valid), or should the first field still behave like it would inside a struct (must always be a valid bool)? The RFC doesn't even seem to say which fields can be safely accessed (if any) so it's not clear. If some fields can be safely accessed and others cannot, this would again be a fundamental language change, requiring the introduction of a novel kind of type to Rust.

All of this would be trivially resolved by allowing unnamed fields with named types only. Or, you know, just naming those fields; very little code has to interact with these complicated C types directly and they can just generate names for these fields and spell them out, making the traversal of this tree-shaped structure explicit. That would also make it much easier to reason about the code, e.g. by making it clear when two fields might overlap. Given that such types are already rare in C (people seem to always use the same 2 or 3 examples when motivating this feature), and writing C bindings is rare in Rust (in the sense that picking some random piece of Rust code, it's rare for that to be C binding code), I think a feature exclusively meant to support such a small fraction of code needs to pass a pretty high bar. The RFC does not seem to have a "prior work" section so I can only assume that thus far, no language besides C (and C++?) has this feature, further confirming my impression that it serves an extremely niche use-case.

@afetisov
Copy link

Should this entire type behave like a union (any byte sequence is valid), or should the first field still behave like it would inside a struct (must always be a valid bool)? The RFC doesn't even seem to say which fields can be safely accessed (if any) so it's not clear.

Doesn't it? Imho it's clear from the text, that the validity requirements are the same as if you had named fields with separately defined types. The only thing the RFC does is allow omitting explicit type definition and the corresponding field name in accesses:

Given a struct s of this type, code can access s.a, s.d, and either s.b or s.c. Accesses to a and d can occur in safe code; accesses to b and c require unsafe code, and b and c overlap, requiring care to access only the field whose contents make sense at the time.

Given a union u of this type, code can access u.a, or u.d, or both u.b and u.c. Since all of these fields can potentially overlap with others, accesses to any of them require unsafe code; however, b and c do not overlap with each other. Code can borrow u.b and u.c simultaneously, but cannot borrow any other fields at the same time.

Such a structure defined with repr(C) will use a representation identical to the same structure with all unnamed fields transformed to equivalent named fields of a struct or union type with the same fields.


should we enforce the tree to strictly alternate between struct and union..?

I hope not. This feature would allow structural inheritance for structs and unions. That feature would be useful e.g. for representation of C++-like class hierarchy, where subclasses have the fields of superclasses as their initial segments. For unions, it would allow patterns like ad-hoc subtyping of enums and hand-crafted ad-hoc sum types, like the ones OCaml uses to represent its errors.

@RalfJung
Copy link
Member

I hope not. This feature would allow structural inheritance for structs and unions. That feature would be useful e.g. for representation of C++-like class hierarchy, where subclasses have the fields of superclasses as their initial segments. For unions, it would allow patterns like ad-hoc subtyping of enums and hand-crafted ad-hoc sum types, like the ones OCaml uses to represent its errors.

That's outside the scope and motivation of the RFC, and the feature is likely a poor fit for this purpose. IMO that's an argument in favor of requiring strict alternation.

@afetisov
Copy link

The scope of the RFC is representation of C FFI type without introducing types and field names which don't exist in original code. Anonymous structs can appear inside structs in C, same for unions, so that is explicitly within the scope and motivation of RFC.

@simonbuchan
Copy link
Contributor

FWIW I offhandedly suggested a conflicting(?) interpretation of this syntax in IRLO a short while ago:

https://internals.rust-lang.org/t/phantom-trait/20434/15?u=simonbuchan

TLDR:

struct Foo<T> {
  id: u32,
  _: PhantomData<T>,
}

let foo: Foo<String> { id: 123 }

Where the compiler can implicitly initialize some sensible set of types (Default? Some new Unitary auto trait?) without them needing to be named.

Strictly I guess this use of the syntax doesn't conflict if such types are considered to be empty types somehow?

@Amanieu
Copy link
Member

Amanieu commented May 12, 2024

It seems that implementation work (#121553) is blocked on the lang team nomination, so it would be nice to clarify what the exact question being asked here is.

The libc 1.0 work (rust-lang/libc#3248) is currently blocked on this feature since we would like to make the APIs match C more closely.

This is specifically useful for APIs where the field organization can vary between platforms, where some platforms use nested unions but others use distinct fields.

@fmease
Copy link
Member

fmease commented May 12, 2024

It seems that implementation work (#121553) is blocked on the lang team nomination, so it would be nice to clarify what the exact question being asked here is.

@Amanieu, cc #121270 (comment)

@Amanieu
Copy link
Member

Amanieu commented May 12, 2024

I wasn't aware that this had already been discussed since this issue is still marked as lang-nominated. I know this was a while ago, but it would be nice if @compiler-errors or someone else could summarize the position of the lang team on this feature.

@joshtriplett
Copy link
Member

It sounds like, from the minutes, that some folks would like to see this feature designed differently than it was when it was previously accepted. It wouldn't be the first or last feature to need some design adjustments when lang design met compiler reality. Happy to help with that, so that we can find a design that meets the requirements in the original RFC and any new issues that have arisen since then.

@RalfJung
Copy link
Member

The libc 1.0 work (rust-lang/libc#3248) is currently blocked on this feature since we would like to make the APIs match C more closely.

This is specifically useful for APIs where the field organization can vary between platforms, where some platforms use nested unions but others use distinct fields.

Note that with the proposal as specified in the RFC, this leads to the same code being safe on some platforms and unsafe on others.

IMO some C APIs are just too terrible for us to want to support matching them in Rust. I'm not saying we shouldn't support interop with those platforms -- we always should support interop. We added untagged unions, a major non-trivial language feature, largely to support C interop. So these APIs can be translated to named structs/unions. A consistent API across platforms can be achieved via methods. That's different from what happens in C, but it is not a good idea to copy mistakes (in my eyes) from C just because we want to have APIs that look the same. "It has to match the C API" is a great recipe for being forever stuck with poor choices made decades ago. IMO a new language feature needs stronger motivation than that.

@fmease fmease removed the I-lang-nominated Nominated for discussion during a lang team meeting. label May 13, 2024
@frank-king frank-king removed their assignment Aug 9, 2024
@fmease fmease added the S-tracking-design-concerns Status: There are blocking design concerns. label Sep 14, 2024
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 11, 2024
…s, r=wesleywiser

Retire the `unnamed_fields` feature for now

`#![feature(unnamed_fields)]` was implemented in part in rust-lang#115131 and rust-lang#115367, however work on that feature has (afaict) stalled and in the mean time there have been some concerns raised (e.g.[^1][^2]) about whether `unnamed_fields` is worthwhile to have in the language, especially in its current desugaring. Because it represents a compiler implementation burden including a new kind of anonymous ADT and additional complication to field selection, and is quite prone to bugs today, I'm choosing to remove the feature.

However, since I'm not one to really write a bunch of words, I'm specifically *not* going to de-RFC this feature. This PR essentially *rolls back* the state of this feature to "RFC accepted but not yet implemented"; however if anyone wants to formally unapprove the RFC from the t-lang side, then please be my guest. I'm just not totally willing to summarize the various language-facing reasons for why this feature is or is not worthwhile, since I'm coming from the compiler side mostly.

Fixes rust-lang#117942
Fixes rust-lang#121161
Fixes rust-lang#121263
Fixes rust-lang#121299
Fixes rust-lang#121722
Fixes rust-lang#121799
Fixes rust-lang#126969
Fixes rust-lang#131041

Tracking:
* rust-lang#49804

[^1]: https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Unnamed.20struct.2Funion.20fields
[^2]: rust-lang#49804 (comment)
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 11, 2024
…s, r=wesleywiser

Retire the `unnamed_fields` feature for now

`#![feature(unnamed_fields)]` was implemented in part in rust-lang#115131 and rust-lang#115367, however work on that feature has (afaict) stalled and in the mean time there have been some concerns raised (e.g.[^1][^2]) about whether `unnamed_fields` is worthwhile to have in the language, especially in its current desugaring. Because it represents a compiler implementation burden including a new kind of anonymous ADT and additional complication to field selection, and is quite prone to bugs today, I'm choosing to remove the feature.

However, since I'm not one to really write a bunch of words, I'm specifically *not* going to de-RFC this feature. This PR essentially *rolls back* the state of this feature to "RFC accepted but not yet implemented"; however if anyone wants to formally unapprove the RFC from the t-lang side, then please be my guest. I'm just not totally willing to summarize the various language-facing reasons for why this feature is or is not worthwhile, since I'm coming from the compiler side mostly.

Fixes rust-lang#117942
Fixes rust-lang#121161
Fixes rust-lang#121263
Fixes rust-lang#121299
Fixes rust-lang#121722
Fixes rust-lang#121799
Fixes rust-lang#126969
Fixes rust-lang#131041

Tracking:
* rust-lang#49804

[^1]: https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Unnamed.20struct.2Funion.20fields
[^2]: rust-lang#49804 (comment)
RalfJung pushed a commit to RalfJung/miri that referenced this issue Oct 14, 2024
…eywiser

Retire the `unnamed_fields` feature for now

`#![feature(unnamed_fields)]` was implemented in part in #115131 and #115367, however work on that feature has (afaict) stalled and in the mean time there have been some concerns raised (e.g.[^1][^2]) about whether `unnamed_fields` is worthwhile to have in the language, especially in its current desugaring. Because it represents a compiler implementation burden including a new kind of anonymous ADT and additional complication to field selection, and is quite prone to bugs today, I'm choosing to remove the feature.

However, since I'm not one to really write a bunch of words, I'm specifically *not* going to de-RFC this feature. This PR essentially *rolls back* the state of this feature to "RFC accepted but not yet implemented"; however if anyone wants to formally unapprove the RFC from the t-lang side, then please be my guest. I'm just not totally willing to summarize the various language-facing reasons for why this feature is or is not worthwhile, since I'm coming from the compiler side mostly.

Fixes #117942
Fixes #121161
Fixes #121263
Fixes #121299
Fixes #121722
Fixes #121799
Fixes #126969
Fixes #131041

Tracking:
* rust-lang/rust#49804

[^1]: https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Unnamed.20struct.2Funion.20fields
[^2]: rust-lang/rust#49804 (comment)
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Oct 17, 2024
…eywiser

Retire the `unnamed_fields` feature for now

`#![feature(unnamed_fields)]` was implemented in part in #115131 and #115367, however work on that feature has (afaict) stalled and in the mean time there have been some concerns raised (e.g.[^1][^2]) about whether `unnamed_fields` is worthwhile to have in the language, especially in its current desugaring. Because it represents a compiler implementation burden including a new kind of anonymous ADT and additional complication to field selection, and is quite prone to bugs today, I'm choosing to remove the feature.

However, since I'm not one to really write a bunch of words, I'm specifically *not* going to de-RFC this feature. This PR essentially *rolls back* the state of this feature to "RFC accepted but not yet implemented"; however if anyone wants to formally unapprove the RFC from the t-lang side, then please be my guest. I'm just not totally willing to summarize the various language-facing reasons for why this feature is or is not worthwhile, since I'm coming from the compiler side mostly.

Fixes #117942
Fixes #121161
Fixes #121263
Fixes #121299
Fixes #121722
Fixes #121799
Fixes #126969
Fixes #131041

Tracking:
* rust-lang/rust#49804

[^1]: https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Unnamed.20struct.2Funion.20fields
[^2]: rust-lang/rust#49804 (comment)
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Oct 18, 2024
…eywiser

Retire the `unnamed_fields` feature for now

`#![feature(unnamed_fields)]` was implemented in part in #115131 and #115367, however work on that feature has (afaict) stalled and in the mean time there have been some concerns raised (e.g.[^1][^2]) about whether `unnamed_fields` is worthwhile to have in the language, especially in its current desugaring. Because it represents a compiler implementation burden including a new kind of anonymous ADT and additional complication to field selection, and is quite prone to bugs today, I'm choosing to remove the feature.

However, since I'm not one to really write a bunch of words, I'm specifically *not* going to de-RFC this feature. This PR essentially *rolls back* the state of this feature to "RFC accepted but not yet implemented"; however if anyone wants to formally unapprove the RFC from the t-lang side, then please be my guest. I'm just not totally willing to summarize the various language-facing reasons for why this feature is or is not worthwhile, since I'm coming from the compiler side mostly.

Fixes #117942
Fixes #121161
Fixes #121263
Fixes #121299
Fixes #121722
Fixes #121799
Fixes #126969
Fixes #131041

Tracking:
* rust-lang/rust#49804

[^1]: https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Unnamed.20struct.2Funion.20fields
[^2]: rust-lang/rust#49804 (comment)
github-actions bot pushed a commit to ytmimi/rustfmt that referenced this issue Dec 2, 2024
…eywiser

Retire the `unnamed_fields` feature for now

`#![feature(unnamed_fields)]` was implemented in part in #115131 and #115367, however work on that feature has (afaict) stalled and in the mean time there have been some concerns raised (e.g.[^1][^2]) about whether `unnamed_fields` is worthwhile to have in the language, especially in its current desugaring. Because it represents a compiler implementation burden including a new kind of anonymous ADT and additional complication to field selection, and is quite prone to bugs today, I'm choosing to remove the feature.

However, since I'm not one to really write a bunch of words, I'm specifically *not* going to de-RFC this feature. This PR essentially *rolls back* the state of this feature to "RFC accepted but not yet implemented"; however if anyone wants to formally unapprove the RFC from the t-lang side, then please be my guest. I'm just not totally willing to summarize the various language-facing reasons for why this feature is or is not worthwhile, since I'm coming from the compiler side mostly.

Fixes #117942
Fixes #121161
Fixes #121263
Fixes #121299
Fixes #121722
Fixes #121799
Fixes #126969
Fixes #131041

Tracking:
* rust-lang/rust#49804

[^1]: https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Unnamed.20struct.2Funion.20fields
[^2]: rust-lang/rust#49804 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC E-help-wanted Call for participation: Help is requested to fix this issue. F-unnamed_fields `#![feature(unnamed_fields)]` S-tracking-design-concerns Status: There are blocking design concerns. S-tracking-impl-incomplete Status: The implementation is incomplete. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests