-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Retire the unnamed_fields
feature for now
#131045
Conversation
r? @fee1-dead rustbot has assigned @fee1-dead. Use |
Some changes occurred in src/tools/rustfmt cc @rust-lang/rustfmt |
I'm nominating this for T-lang somewhat out of courtesy; my understanding is that whether or not an RFC-approved unstable implementation of a feature moves forwards or backwards really is a T-compiler judgement call, but I would like to hear T-lang's take on this. I generally think that "let's go back to the drawing board" is the right call here, and in the mean time, I'd like to remove this from the compiler since there's a fair chance that a future design could be implemented some other way, and it's always easier to add things back into the compiler -- and I would be glad to either mentor or implement that work myself when that time comes, of course. @rustbot label: +I-lang-nominated |
This comment has been minimized.
This comment has been minimized.
3dd4c4b
to
6c409ea
Compare
Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
This comment has been minimized.
This comment has been minimized.
currently capacity deficient, so r? compiler |
6c409ea
to
6628bba
Compare
Especially given that the tracking issue for this is marked |
@rustbot labels -I-lang-nominated We discussed this in the meeting today, and we appreciated the courtesy, and agree that it's OK as far as lang is concerned to rip this out. |
r? compiler (cc rust-lang/team#1565) |
I agree, removing this implementation is well motivated. Thanks @compiler-errors for doing this and also notifying T-lang of the removal. @bors r+ |
…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)
☀️ Test successful - checks-actions |
Finished benchmarking commit (f496659): comparison URL. Overall result: ❌✅ regressions and improvements - no action needed@rustbot label: -perf-regression Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (secondary 0.6%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (primary -2.0%, secondary 3.5%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 780.481s -> 780.255s (-0.03%) |
…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)
#![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.12) about whetherunnamed_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:
Footnotes
https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Unnamed.20struct.2Funion.20fields ↩
https://github.com/rust-lang/rust/issues/49804#issuecomment-1972619108 ↩