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

Soft-destabilize RustcEncodable & RustcDecodable, remove from prelude in next edition #272

Closed
jhpratt opened this issue Sep 20, 2023 · 2 comments
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@jhpratt
Copy link
Member

jhpratt commented Sep 20, 2023

Proposal

Problem statement

The RustcEncodable and RustcDecoadable derives serve no purpose on their own and are deprecated, undocumented, and widely unused. The derives produce code for rustc-serialize, an external crate which has not had a release since April 2017 and is officially deprecated in favor of serde.

The rustc-serialize crate will, in its current state, fail to compile at some point in the future, as it contains code with a future compatibility warning:

> warning: impl method assumes more implied bounds than the corresponding trait method
>     --> /home/jhpratt/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustc-serialize-0.3.24/src/serialize.rs:1155:41
>      |
> 1155 |     fn decode<D: Decoder>(d: &mut D) -> Result<Cow<'static, T>, D::Error> {
>      |                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace this type to make the impl signature compatible: `Result<Cow<'a, T>, <D as serialize::Decoder>::Error>`
>      |
>      = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
>      = note: for more information, see issue #105572 <https://github.com/rust-lang/rust/issues/105572>
>      = note: `#[allow(implied_bounds_entailment)]` on by default
> 

Solution sketch

The RustcEncodable and RustcDecodable derives are to be soft-destabilized, as mentioned in rust-lang/rust#92594. This turns any use of them into a deny-by-default lint, which is stronger than the warn-by-default deprecation warning that is currently present.

Additionally, the RustcEncodable and RustcDecodable traits are to be removed from the standard library's preludes in all future editions (i.e. ≥2024). rustfix will change existing uses to use the path to the previous prelude.

Alternatives

A full destabilization is not proposed due to breakage of existing code. If/when rustc-serialize fails to compile on stable as mentioned above, a full destabilization would make more sense.

While this proposes both soft-destabilizing and removing the derives from the prelude, there is no reason one could not be accepted without the other if it is so desired.

Links and related work

@jhpratt jhpratt added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Sep 20, 2023
@dtolnay
Copy link
Member

dtolnay commented Sep 20, 2023

Seconded; let's make it happen. The soft-destabilize approach described in rust-lang/rust#92594 (review) sounds great to me.

@dtolnay dtolnay added the ACP-accepted API Change Proposal is accepted (seconded with no objections) label Sep 20, 2023
@dtolnay dtolnay closed this as completed Sep 20, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 23, 2024
…olnay

Soft-destabilize `RustcEncodable` & `RustcDecodable`, remove from prelude in next edition

cc rust-lang/libs-team#272

Any use of `RustcEncodable` and `RustcDecodable` now triggers a deny-by-default lint. The derives have been removed from the 2024 prelude. I specifically chose **not** to document this in the module-level documentation, as the presence in existing preludes is not documented (which I presume is intentional).

This does not implement the proposed change for `rustfix`, which I will be looking into shortly.

With regard to the items in the preludes being stable, this should not be an issue because rust-lang#15702 has been resolved.

r? libs-api
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 23, 2024
…olnay

Soft-destabilize `RustcEncodable` & `RustcDecodable`, remove from prelude in next edition

cc rust-lang/libs-team#272

Any use of `RustcEncodable` and `RustcDecodable` now triggers a deny-by-default lint. The derives have been removed from the 2024 prelude. I specifically chose **not** to document this in the module-level documentation, as the presence in existing preludes is not documented (which I presume is intentional).

This does not implement the proposed change for `rustfix`, which I will be looking into shortly.

With regard to the items in the preludes being stable, this should not be an issue because rust-lang#15702 has been resolved.

r? libs-api
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Mar 26, 2024
Soft-destabilize `RustcEncodable` & `RustcDecodable`, remove from prelude in next edition

cc rust-lang/libs-team#272

Any use of `RustcEncodable` and `RustcDecodable` now triggers a deny-by-default lint. The derives have been removed from the 2024 prelude. I specifically chose **not** to document this in the module-level documentation, as the presence in existing preludes is not documented (which I presume is intentional).

This does not implement the proposed change for `rustfix`, which I will be looking into shortly.

With regard to the items in the preludes being stable, this should not be an issue because rust-lang#15702 has been resolved.

r? libs-api
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 27, 2024
Soft-destabilize `RustcEncodable` & `RustcDecodable`, remove from prelude in next edition

cc rust-lang/libs-team#272

Any use of `RustcEncodable` and `RustcDecodable` now triggers a deny-by-default lint. The derives have been removed from the 2024 prelude. I specifically chose **not** to document this in the module-level documentation, as the presence in existing preludes is not documented (which I presume is intentional).

This does not implement the proposed change for `rustfix`, which I will be looking into shortly.

With regard to the items in the preludes being stable, this should not be an issue because rust-lang#15702 has been resolved.

r? libs-api
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Mar 29, 2024
Soft-destabilize `RustcEncodable` & `RustcDecodable`, remove from prelude in next edition

cc rust-lang/libs-team#272

Any use of `RustcEncodable` and `RustcDecodable` now triggers a deny-by-default lint. The derives have been removed from the 2024 prelude. I specifically chose **not** to document this in the module-level documentation, as the presence in existing preludes is not documented (which I presume is intentional).

This does not implement the proposed change for `rustfix`, which I will be looking into shortly.

With regard to the items in the preludes being stable, this should not be an issue because #15702 has been resolved.

r? libs-api
@RalfJung
Copy link
Member

I made an issue to track the implementation progress of this: rust-lang/rust#134301

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

3 participants