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.

Sign up for GitHub

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

Jump to bottom

Implement phantom variance markers #135807

Merged
merged 1 commit into from
Jan 27, 2025
Merged

Implement phantom variance markers #135807

merged 1 commit into from
Jan 27, 2025

Conversation

jhpratt
Copy link
Member

@jhpratt jhpratt commented Jan 21, 2025

ACP accepted rust-lang/libs-team#488

Tracking issue #135806

@jhpratt jhpratt added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jan 21, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jan 21, 2025

r? @scottmcm

rustbot has assigned @scottmcm.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 21, 2025
@jhpratt jhpratt mentioned this pull request Jan 21, 2025
Open
4 tasks
@rust-log-analyzer

This comment has been minimized.

Sign in to view
Sky9x
Sky9x reviewed Jan 22, 2025
View reviewed changes
Copy link
Contributor

@Sky9x Sky9x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct me if I'm wrong, but I think these should be the other way around

library/core/src/marker/variance.rs Outdated Show resolved Hide resolved
library/core/src/marker/variance.rs Outdated Show resolved Hide resolved
@jhpratt
Copy link
Member Author

jhpratt commented Jan 22, 2025

Swapped the descriptions so as to be correct, reworded them a bit for clarity, and added an example (contrived as it may be).

@scottmcm
Copy link
Member

I never remember which variance is which, so hopefully a random roll will find someone who does 🤞

r? libs

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jan 23, 2025
@rustbot rustbot assigned Amanieu and unassigned scottmcm Jan 23, 2025
@Amanieu
Copy link
Member

Amanieu commented Jan 23, 2025

In the ACP, we suggested making these types work like PhantomData by allowing them to be constructed without any fields. This doesn't have to happen in this PR, but it would eliminate the need for the trait and constructor method, and would bring these types in line with PhantomData and PhantomPinned. At the very least it should be listed as a blocker for stabilization in the tracking issue.

@jhpratt
Copy link
Member Author

jhpratt commented Jan 24, 2025 •
edited
Loading

Why should that be a blocker? It's backwards compatible to eliminate the need for ::new() by turning it into a lang item. The trait only exists so people don't have to explicit state the variance when constructing a value, regardless of whether ::new() is needed.

@Amanieu
Copy link
Member

Amanieu commented Jan 25, 2025

While it is backwards-compatible, we are generally reluctant to stabilize functions that we known are going to be deprecated eventually.

In any case, this PR is fine for an initial implementation.

@jhpratt
Copy link
Member Author

jhpratt commented Jan 26, 2025

Gotcha. I'll add that as an unresolved question in the tracking issue.

@Amanieu
Copy link
Member

Amanieu commented Jan 26, 2025

@bors r+

@bors
Copy link
Contributor

bors commented Jan 26, 2025

📌 Commit b3a5d0a has been approved by Amanieu

It is now in the queue for this repository.

@bors 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 Jan 26, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Jan 27, 2025
@jhpratt
Rollup merge of rust-lang#135807 - jhpratt:phantom-variance, r=Amanieu
e49701b 
Implement phantom variance markers

ACP accepted rust-lang/libs-team#488

Tracking issue rust-lang#135806
@jhpratt jhpratt mentioned this pull request Jan 27, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 27, 2025
@bors
Auto merge of rust-lang#136111 - jhpratt:rollup-p9wdze6, r=jhpratt
d21b9c2 
Rollup of 5 pull requests

Successful merges:

 - rust-lang#135807 (Implement phantom variance markers)
 - rust-lang#136091 (Add some tracing to core bootstrap logic)
 - rust-lang#136094 (Upgrade elsa to the newest version.)
 - rust-lang#136097 (rustc_ast: replace some len-checks + indexing with slice patterns etc.)
 - rust-lang#136101 (triagebot: set myself on vacation)

r? `@ghost`
`@rustbot` modify labels: rollup
@GuillaumeGomez GuillaumeGomez mentioned this pull request Jan 27, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 27, 2025
@bors
Auto merge of rust-lang#136135 - GuillaumeGomez:rollup-1ik636d, r=Gui…
ebcf860 
…llaumeGomez

Rollup of 10 pull requests

Successful merges:

 - rust-lang#135773 (Clarify WindowsMut (Lending)Iterator)
 - rust-lang#135807 (Implement phantom variance markers)
 - rust-lang#135876 (fix doc for std::sync::mpmc)
 - rust-lang#135988 (Add a workaround for parallel rustc crashing when there are delayed bugs)
 - rust-lang#136037 (Mark all NuttX targets as tier 3 target and support the standard library)
 - rust-lang#136064 (Add a suggestion to cast target_feature fn items to fn pointers.)
 - rust-lang#136082 (Incorporate `iter_nodes` into `graph::DirectedGraph`)
 - rust-lang#136112 (Clean up all dead files inside `tests/ui/`)
 - rust-lang#136114 (Use identifiers more in diagnostics code)
 - rust-lang#136118 (Change `collect_and_partition_mono_items` tuple return type to a struct)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 314a695 into rust-lang:master Jan 27, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Jan 27, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 27, 2025
@rust-timer
Unrolled build for rust-lang#135807
692dbd4 
Rollup merge of rust-lang#135807 - jhpratt:phantom-variance, r=Amanieu

Implement phantom variance markers

ACP accepted rust-lang/libs-team#488

Tracking issue rust-lang#135806
@jhpratt jhpratt deleted the phantom-variance branch February 5, 2025 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Amanieu Amanieu Amanieu left review comments

@RustyYato RustyYato RustyYato left review comments

@scottmcm scottmcm scottmcm left review comments

@juntyr juntyr juntyr left review comments

@Sky9x Sky9x Sky9x left review comments

Assignees

@Amanieu Amanieu

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.86.0
Development

Successfully merging this pull request may close these issues.
9 participants
@jhpratt @rustbot @rust-log-analyzer @scottmcm @Amanieu @bors @RustyYato @juntyr @Sky9x