Use #[non_exhaustive] when possible. #47122
Conversation
|
r? @BurntSushi (rust_highfive has picked a reviewer for you, use r? to override) |
|
CC #44109 and @davidtwco, who implemented the RFC |
This gets rid of the `future_atomic_orderings` and `io_error_internals` feature gates, as they are no longer needed.
clarfonthey
force-pushed
the
non_exhaustive_std
branch
from
a8397bb to
9bf5c5b
Compare
| pub struct LexError { | ||
| _inner: (), | ||
| } | ||
| #[non_exhaustive] |
There was a problem hiding this comment.
[00:07:40] error: non exhaustive is an experimental feature (see issue #44109)
[00:07:40] --> /checkout/src/libproc_macro/diagnostic.rs:18:1
[00:07:40] |
[00:07:40] 18 | #[non_exhaustive]
[00:07:40] | ^^^^^^^^^^^^^^^^^
[00:07:40] |
[00:07:40] = help: add #![feature(non_exhaustive)] to the crate attributes to enable
There was a problem hiding this comment.
I totally thought I added this but I guess not.
src/liballoc/boxed.rs
Outdated
| _force_singleton: (), | ||
| } | ||
| #[non_exhaustive] | ||
| pub struct ExchangeHeapSingleton { } |
There was a problem hiding this comment.
No need to use the braced form for non_exhaustive structs, more usual pub struct ExchangeHeapSingleton; can be used.
There was a problem hiding this comment.
Done; I mostly did this because I wasn't sure if this had been implemented yet. Although in honesty, the goal should be to make sure this feature works by first testing it in the standard library.
kennytm
added
the
S-waiting-on-author
|
☔ The latest upstream changes (presumably #47088) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Thanks! I'd personally prefer to hold off on this though for libstd until this feature stabilizes. These are all done specifically for backwards compatibility and "good documentation" of libstd and I'd just want to make sure that we have all the bugs fleshed out of this attribute before using it in the standard library (where a mistake can be very costly) |
|
@alexcrichton Which feature is this PR blocked on? How long until that feature is likely to stabilize? That is, should we leave this PR open with S-blocked (for a short amount of time) or close it (if it's going to be a long time)? |
|
@clarcharr what do you think about blocking this on the @carols10cents I don't know personally, I'm not following the feature. I'd prefer to close if we don't want to merge this in the meantime. |
kennytm
added
the
T-libs-api
|
Hi @clarcharr, what do you think about @alexcrichton's comment in #47122 (comment)? |
|
I think that while we shouldn't have to wait until that's stabilised, I think that we should wait until some of the bugs listed in the tracking issue are ironed out. I don't think that the issue should be closed yet though. |
|
Ok, sounds like the issue should be left open but this PR should be closed. If I've misunderstood, please reopen! |
This gets rid of the
future_atomic_orderingsandio_error_internalsfeature gates, as they are no longer needed.