Deprecate `intrinsics::drop_in_place` and `collections::Bound`, which accidentally weren't deprecated by bstrie · Pull Request #82122 · rust-lang/rust

bstrie · 2021-02-14T23:27:56Z

Fixes#82080.

I've taken the liberty of updating the since values to 1.52, since an unobservable deprecation isn't much of a deprecation (even the detailed release notes never bothered to mention these deprecations).

As mentioned in the issue I'm pretty sure that using a type alias for Bound is semantically equivalent to the re-export; the reference implies that type aliases only observably differ from types when used on unit structs or tuple structs, whereas Bound is an enum.

rust-highfive · 2021-02-14T23:27:59Z

r? @dtolnay

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

bstrie · 2021-02-14T23:29:14Z

Note that this might not necessarily "fix" #82080 if you happen to think that rustc_deprecated should "just work" on re-exports, but this is a far more expedient fix to the current problem.

dtolnay

Thanks!

dtolnay · 2021-02-14T23:44:22Z

@bors r+

bors · 2021-02-14T23:44:23Z

📌 Commit dfae9f0077de752ec9c00f6625c65cab202366b3 has been approved by dtolnay

dtolnay · 2021-02-14T23:45:26Z

It looks like tidy failed in CI:

2021-02-14T23:40:28.5030915Z tidy check2021-02-14T23:40:28.8226072Z * 624 error codes2021-02-14T23:40:28.8227914Z * highest error code: E07812021-02-14T23:40:28.8228436Z Checking which error codes lack tests...2021-02-14T23:40:29.0131586Z Found 436 error codes2021-02-14T23:40:29.0132273Z Found 0 error codes with no tests2021-02-14T23:40:29.0132741Z Done!2021-02-14T23:40:30.1846989Z tidy error: /checkout/library/core/src/intrinsics.rs:72: undocumented unsafe2021-02-14T23:40:32.0135064Z some tidy checks failed

@bors r-

jyn514 · 2021-02-15T00:00:44Z

Note that this might not necessarily "fix" #82080 if you happen to think that rustc_deprecated should "just work" on re-exports, but this is a far more expedient fix to the current problem.

Could you open a separate issue for making rustc_deprecated work on re-exports (since it sounds like 82080 is mostly tracking drop_in_place)?

dtolnay · 2021-02-15T00:02:30Z

@bors r+

bors · 2021-02-15T00:02:31Z

📌 Commit c813b38ec04eae0108cfb14312be8e4348dc0d27 has been approved by dtolnay

bstrie · 2021-02-15T00:07:27Z

I've opened #82123 to track fixing the behavior of #[rustc_deprecated].

bstrie · 2021-02-15T00:18:19Z

Oof, I'll take a look at that.

jyn514 · 2021-02-15T00:18:41Z

@bors r-

bstrie · 2021-02-15T03:44:58Z

Ok, when I first saw that this patch was changing the output of some UI tests I was concerned that codegen was secretly referring to intrinsics::drop_in_place somewhere, and that this was just exposing that. However I tried deleting intrinsics::drop_in_place and found that the tests all passed, so my new hypothesis is just that the diagnostic output is smartly recognizing that there are two distinct drop_in_place symbols and is just trying to disambiguate by referring to the fully-qualified path of ptr::drop_in_place. Does that sound plausible? If so, I've blessed those UI tests.

jyn514 · 2021-02-15T03:49:28Z

However I tried deleting intrinsics::drop_in_place and found that the tests all passed, so my new hypothesis is just that the diagnostic output is smartly recognizing that there are two distinct drop_in_place symbols and is just trying to disambiguate by referring to the fully-qualified path of ptr::drop_in_place. Does that sound plausible?

That sounds right, yeah, I think that's what was implemented in #73996.

bstrie · 2021-02-15T04:22:10Z

Ok, so I assume that the mir output being diffed here is only for human consumption, and that the new fully-qualified path being shown for ptr::drop_in_place is just an artifact of the textual representation and not actually any sort of codegen regression?

But fascinatingly, one of the failing MIR tests does contain an explicit reference to intrinsics::drop_in_place... but it's failing because this PR causes it to start referring to ptr::drop_in_place, somehow???

While we're at it, I'm just grepping for intrinsics::drop_in_place now and there's a bunch of codegen tests with that exact string. Why? The lang item has long since been directly on ptr::drop_in_place, so codegen should always be referring to that. True enough, these tests start failing when I run the test suite locally. Is it okay to bless these?

Fixesrust-lang#82080

Dylan-DPC-zz · 2021-03-10T00:49:23Z

@jyn514 this was in the error as well:

[RUSTC-TIMING] semver_parser test:false 3.186
error: use of deprecated type alias `std::collections::Bound`: moved to `std::ops::Bound`
|
30 | use std::collections::Bound;
| ^^^^^^^^^^^^^^^^^^^^^^^

bstrie · 2021-03-16T21:53:50Z

I've updated Clippy; ready for re-approval.

Dylan-DPC-zz · 2021-03-16T21:55:14Z

@bors r=dtolnay

bors · 2021-03-16T21:55:16Z

📌 Commit 49aa79e has been approved by dtolnay

bors · 2021-03-17T19:39:07Z

⌛ Testing commit 49aa79e with merge 36f1f04...

bors · 2021-03-17T22:22:38Z

☀️ Test successful - checks-actions
Approved by: dtolnay
Pushing 36f1f04 to master...

rust-highfive · 2021-03-17T22:23:47Z

📣 Toolstate changed by #82122!

Tested on commit 36f1f04.
Direct link to PR: #82122

💔 miri on windows: test-pass → build-fail (cc @eddyb @oli-obk @RalfJung).
💔 miri on linux: test-pass → build-fail (cc @eddyb @oli-obk @RalfJung).

@eddyb

Tested on commit rust-lang/rust@36f1f04. Direct link to PR: <rust-lang/rust#82122> 💔 miri on windows: test-pass → build-fail (cc @eddyb @oli-obk @RalfJung). 💔 miri on linux: test-pass → build-fail (cc @eddyb @oli-obk @RalfJung).

Replace deprecated `collections::Bound` with `ops::Bound` Fixesrust-lang/rust#83242 , which resulted from rust-lang/rust#82122 .

Replace deprecated `collections::Bound` with `ops::Bound` Cc rust-lang/rust#83242 , which resulted from rust-lang/rust#82122 .

RalfJung · 2021-03-17T23:21:58Z

+#[rustc_deprecated(reason = "moved to `std::ops::Bound`", since = "1.52.0")]
 #[doc(hidden)]
-pub use crate::ops::Bound;
+pub type Bound<T> = crate::ops::Bound<T>;


FWIW, this is technically a breaking change if someone did use std::collections::Bound::*; (like Miri did)

Oh, I assumed miri had broken due to denying deprecation warnings. I wasn't aware that type aliases to enums couldn't glob-import; it looks like even concrete importing like use collections::Bound::Unbounded isn't supported. :\ This is a distressing limitation of type alises that I wasn't aware of; I trusted that the limitations laid out in the reference were exhaustive. I'm not sure what to do about this. At the very least I'll file an issue about supporting this, since it really does seem like a bug in type aliases.

I've filed #83248 to track importing variants through type aliases. I'll also ping the libs team on Zulip to see what they think should be done.

Zulip thread at https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/unavoidable.20breakage.20when.20deprecating.20an.20enum.3F .

I trusted that the limitations laid out in the reference were exhaustive.
Which part of the reference is that? Might be worth reporting a bug (or submitting a PR).

This page of the reference: https://doc.rust-lang.org/reference/items/type-aliases.html , at the bottom where it talks about where type aliases cannot be used. I'll submit a PR.

Reference PR at rust-lang/reference#984

Revert the second deprecation of collections::Bound Per the review at rust-lang#82122 (comment) and the decision at https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/unavoidable.20breakage.20when.20deprecating.20an.20enum.3F , revert this small portion of rust-lang#82122 for the time being. This doesn't affect the other components of that patch, i.e. `intrinsics::drop_in_place` is still deprecated-for-real, and uses of `collections::Bound` remain removed from the repo.

Deprecate `intrinsics::drop_in_place` and `collections::Bound`, which accidentally weren't deprecated Fixesrust-lang#82080. I've taken the liberty of updating the `since` values to 1.52, since an unobservable deprecation isn't much of a deprecation (even the detailed release notes never bothered to mention these deprecations). As mentioned in the issue I'm *pretty* sure that using a type alias for `Bound` is semantically equivalent to the re-export; [the reference implies](https://doc.rust-lang.org/reference/items/type-aliases.html) that type aliases only observably differ from types when used on unit structs or tuple structs, whereas `Bound` is an enum.

rust-highfive assigned dtolnay Feb 14, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 14, 2021

This comment has been minimized.
Sign in to view

dtolnay approved these changes Feb 14, 2021
View reviewed changes

bors 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 Feb 14, 2021

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 14, 2021

bstrie force-pushed the dep4real branch from dfae9f0 to c813b38Compare February 14, 2021 23:56

bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 15, 2021

This comment has been minimized.
Sign in to view

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 15, 2021

bstrie force-pushed the dep4real branch from c813b38 to e8a0327Compare February 15, 2021 03:40

This comment has been minimized.
Sign in to view

bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 9, 2021

Deprecate items that accidentally weren't deprecated
49aa79e
Fixesrust-lang#82080

bstrie force-pushed the dep4real branch from 3e305f6 to 49aa79eCompare March 16, 2021 20:44

bstrie added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 16, 2021

bors 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 16, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 17, 2021

bors merged commit 36f1f04 into rust-lang:masterMar 17, 2021

rustbot added this to the 1.52.0 milestone Mar 17, 2021

rust-highfive mentioned this pull request Mar 17, 2021
miri no longer builds after rust-lang/rust#82122 #83242
Closed

bstrie mentioned this pull request Mar 17, 2021
Replace deprecated collections::Bound with ops::Boundrust-lang/miri#1746
Merged

bors mentioned this pull request Mar 17, 2021
Use ValTree in all type level constants #83234
Closed

bors added a commit to rust-lang/miri that referenced this pull request Mar 17, 2021
Auto merge of#1746- bstrie:depfix, r=RalfJung
65b348c
Replace deprecated `collections::Bound` with `ops::Bound` Fixesrust-lang/rust#83242 , which resulted from rust-lang/rust#82122 .

bors added a commit to rust-lang/miri that referenced this pull request Mar 17, 2021
Auto merge of#1746- bstrie:depfix, r=RalfJung
12dac5c
Replace deprecated `collections::Bound` with `ops::Bound` Cc rust-lang/rust#83242 , which resulted from rust-lang/rust#82122 .

RalfJung reviewed Mar 17, 2021
View reviewed changes

This was referenced Mar 17, 2021
Support importing enum variants through type aliases #83248
Open
Revert the second deprecation of collections::Bound #83269
Merged

CDirkx mentioned this pull request May 4, 2021
Update RELEASES.md with missing deprecation notes. #84914
Closed

Uh oh!

Uh oh!

Conversation

bstrie commented Feb 14, 2021

Uh oh!

rust-highfive commented Feb 14, 2021

Uh oh!

bstrie commented Feb 14, 2021

Uh oh!

This comment has been minimized.

dtolnay left a comment

Choose a reason for hiding this comment

Uh oh!

dtolnay commented Feb 14, 2021

Uh oh!

bors commented Feb 14, 2021

Uh oh!

dtolnay commented Feb 14, 2021

Uh oh!

jyn514 commented Feb 15, 2021

Uh oh!

dtolnay commented Feb 15, 2021

Uh oh!

bors commented Feb 15, 2021

Uh oh!

bstrie commented Feb 15, 2021

Uh oh!

This comment has been minimized.

bstrie commented Feb 15, 2021

Uh oh!

jyn514 commented Feb 15, 2021

Uh oh!

bstrie commented Feb 15, 2021

Uh oh!

jyn514 commented Feb 15, 2021

Uh oh!

This comment has been minimized.

bstrie commented Feb 15, 2021

Uh oh!

Dylan-DPC-zz commented Mar 10, 2021

Uh oh!

bstrie commented Mar 16, 2021

Uh oh!

Dylan-DPC-zz commented Mar 16, 2021

Uh oh!

bors commented Mar 16, 2021

Uh oh!

bors commented Mar 17, 2021

Uh oh!

bors commented Mar 17, 2021

Uh oh!

rust-highfive commented Mar 17, 2021

Uh oh!

RalfJungMar 17, 2021

Choose a reason for hiding this comment

Uh oh!

bstrieMar 17, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bstrieMar 17, 2021

Choose a reason for hiding this comment

Uh oh!

bstrieMar 18, 2021

Choose a reason for hiding this comment

Uh oh!

RalfJungMar 18, 2021

Choose a reason for hiding this comment

Uh oh!

bstrieMar 18, 2021

Choose a reason for hiding this comment

Uh oh!

bstrieMar 18, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

bstrieMar 17, 2021•
edited
Loading