Skip to content

Deprecate RustcEncodable and RustcDecodable. - #83160

Merged
bors merged 1 commit into
rust-lang:masterfrom
m-ou-se:deprecate-rustc-serialize-derives
Mar 16, 2021
Merged

Deprecate RustcEncodable and RustcDecodable.#83160
bors merged 1 commit into
rust-lang:masterfrom
m-ou-se:deprecate-rustc-serialize-derives

Conversation

@m-ou-se

Copy link
Copy Markdown
Member

We can't remove the RustcEncodable and RustcDecodable derive macros from the prelude, but we can deprecate them.

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @kennytm

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 15, 2021
@m-ou-se

m-ou-se commented Mar 15, 2021

Copy link
Copy Markdown
MemberAuthor

r? @petrochenkov

@m-ou-sem-ou-se added the T-libs-api [DEPRECATED; DO NOT USE] label Mar 15, 2021
@m-ou-se

m-ou-se commented Mar 15, 2021

Copy link
Copy Markdown
MemberAuthor

I suppose this technically requires an FCP.

As far as I understand, these macros are ancient left-overs from builtin support for rustc-serialize. We don't even show these in the nightly docs anymore. Let's mark them as deprecated.

@rfcbot merge

@rfcbot

rfcbot commented Mar 15, 2021

Copy link
Copy Markdown

Team member @m-ou-se has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbotrfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Mar 15, 2021
@jyn514

Copy link
Copy Markdown
Member

@rust-log-analyzer

This comment has been minimized.

@m-ou-se
m-ou-seforce-pushed the deprecate-rustc-serialize-derives branch from 852d04e to 924e522CompareMarch 15, 2021 19:16
@petrochenkovpetrochenkov added S-waiting-on-team and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 15, 2021
@BurntSushi

Copy link
Copy Markdown
Member

Wow. I'm surprised these haven't been deprecated yet.

These were indeed ancient leftovers from the pre-serde era of automatic type driven serialization. IIRC, their names were specifically prefixed with Rustc because we knew that they would eventually be replaced by something better. (And indeed, they were, with many thanks to @erickt and @dtolnay!)

@rfcbot

Copy link
Copy Markdown

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbotrfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Mar 15, 2021
@eddyb

Copy link
Copy Markdown
Contributor

Wow. I'm surprised these haven't been deprecated yet.

Likely only due to technical limitations, I seem to recall @petrochenkov making it so all of the builtin macros have a presence in the standard library (even w/o any body), at some point in the last couple years.

@m-ou-se

Copy link
Copy Markdown
MemberAuthor

Seems a bit overkill to wait for a 10 day FCP (and miss the release cycle) for a simple long-overdue deprecation. Marking the FCP as finished.

@m-ou-sem-ou-se added finished-final-comment-period The final comment period is finished for this PR / Issue. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-team final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Mar 15, 2021
@eddyb

Copy link
Copy Markdown
Contributor

Just to be sure, can we have a test that shows a deprecation warning? I'm worried the current system will only warn on a reexport (or at least an implicit import), not on any use of the derives.

@m-ou-se

Copy link
Copy Markdown
MemberAuthor
warning: use of deprecated macro `RustcEncodable`: rustc-serialize is deprecated and no longer supported
--> src/main.rs:1:10
|
1 | #[derive(RustcEncodable)]
| ^^^^^^^^^^^^^^
|
= note: `#[warn(deprecated)]` on by default

@eddyb

eddyb commented Mar 15, 2021

Copy link
Copy Markdown
Contributor

Thanks! Feels a bit strange that there were no preexisting tests that changed output, but maybe that's because such a test would need to get the library from crates.io (or at least some mock version of it)?

(compiler/rustc_serialize no longer has a compatible interface to the old derives being deprecated here, it was changed for the custom derives in compiler/rustc_macros/src/serialize.rs, and should probably be renamed anyway)

@petrochenkov

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Mar 15, 2021

Copy link
Copy Markdown
Collaborator

📌 Commit 924e522 has been approved by petrochenkov

@borsbors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 15, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 16, 2021
…erives, r=petrochenkov
Deprecate RustcEncodable and RustcDecodable.
We can't remove the `RustcEncodable` and `RustcDecodable` derive macros from the prelude, but we can deprecate them.
@JohnTitorJohnTitor mentioned this pull request Mar 16, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 16, 2021
Rollup of 10 pull requests
Successful merges:
- rust-lang#81822 (Added `try_exists()` method to `std::path::Path`)
- rust-lang#83072 (Update `Vec` docs)
- rust-lang#83077 (rustdoc: reduce GC work during search)
- rust-lang#83091 (Constify `copy` related functions)
- rust-lang#83156 (Fall-back to sans-serif if Arial is not available)
- rust-lang#83157 (No background for code in portability snippets)
- rust-lang#83160 (Deprecate RustcEncodable and RustcDecodable.)
- rust-lang#83162 (Specify *.woff2 files as binary)
- rust-lang#83172 (More informative diagnotic from `x.py test` attempt atop beta checkout)
- rust-lang#83196 (Use delay_span_bug instead of panic in layout_scalar_valid_range)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit 39af66f into rust-lang:masterMar 16, 2021
@rustbotrustbot added this to the 1.52.0 milestone Mar 16, 2021
@m-ou-se
m-ou-se deleted the deprecate-rustc-serialize-derives branch March 17, 2021 20:01
@rfcbotrfcbot added the to-announce Announce this issue on triage meeting label Mar 25, 2021
@apirainoapiraino removed the to-announce Announce this issue on triage meeting label Apr 4, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

disposition-mergeThis issue / PR is in PFCP or FCP with a disposition to merge it.finished-final-comment-periodThe final comment period is finished for this PR / Issue.S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-libs-api[DEPRECATED; DO NOT USE]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

12 participants

@m-ou-se@rust-highfive@rfcbot@jyn514@rust-log-analyzer@BurntSushi@eddyb@petrochenkov@bors@kennytm@apiraino@rustbot