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

Remove variances_of on RPITIT GATs, remove its one use-case #113427

Merged
merged 1 commit into from
Jul 8, 2023
Merged

Remove variances_of on RPITIT GATs, remove its one use-case #113427

merged 1 commit into from
Jul 8, 2023

Conversation

compiler-errors
Copy link
Member

It doesn't make sense to implement variances on a GAT anyways, since we don't relate GATs with variance:

rust/compiler/rustc_middle/src/ty/relate.rs

Lines 569 to 579 in 85bf079

(&ty::Alias(a_kind, a_data), &ty::Alias(b_kind, b_data)) => {
// FIXME(-Zlower-impl-trait-in-trait-to-assoc-ty): This if can be removed
// and the assert uncommented once the new desugaring is stable.
if a_kind == b_kind {
let alias_ty = relation.relate(a_data, b_data)?;
// assert_eq!(a_kind, b_kind);
Ok(Ty::new_alias(tcx, a_kind, alias_ty))
} else {
Err(TypeError::Sorts(expected_found(relation, a, b)))
}
}

r? @spastorino

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 7, 2023
compiler-errors
compiler-errors commented Jul 7, 2023
View reviewed changes
compiler/rustc_hir_analysis/src/variance/mod.rs
@@ -59,13 +59,6 @@ fn variances_of(tcx: TyCtxt<'_>, item_def_id: LocalDefId) -> &[ty::Variance] {
DefKind::OpaqueTy | DefKind::ImplTraitPlaceholder => {
return variance_of_opaque(tcx, item_def_id);
}
DefKind::AssocTy => {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is unreachable given the deleted code below.

compiler/rustc_hir_analysis/src/variance/mod.rs
@@ -125,7 +118,8 @@ fn variance_of_opaque(tcx: TyCtxt<'_>, item_def_id: LocalDefId) -> &[ty::Varianc
// FIXME(-Zlower-impl-trait-in-trait-to-assoc-ty) check whether this is necessary
// at all for RPITITs.
ty::Alias(_, ty::AliasTy { def_id, substs, .. })
if self.tcx.is_impl_trait_in_trait(*def_id) =>
if self.tcx.is_impl_trait_in_trait(*def_id)
&& !self.tcx.lower_impl_trait_in_trait_to_assoc_ty() =>
Copy link
Member Author

Choose a reason for hiding this comment

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

In the new lowering scheme, we should never need to consider the variances on a RPITIT GAT.

compiler/rustc_infer/src/infer/opaque_types.rs
@@ -473,17 +473,6 @@ where
}
}

ty::Alias(ty::Projection, proj) if self.tcx.is_impl_trait_in_trait(proj.def_id) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

In the new lowering scheme, we should never need to consider the variances of a RPITIT GAT.

@spastorino
Copy link
Member

spastorino commented Jul 7, 2023 •
edited
Loading

r=me with CI green

@bors rollup

@compiler-errors compiler-errors changed the title Remove variances_of on RPITIT gats, remove its one use-case Remove variances_of on RPITIT GATs, remove its one use-case Jul 7, 2023
@compiler-errors
Copy link
Member Author

@bors r=spastorino

@bors
Copy link
Contributor

bors commented Jul 7, 2023

📌 Commit ca8202d has been approved by spastorino

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 Jul 7, 2023
@spastorino spastorino mentioned this pull request Jul 7, 2023
Closed
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jul 7, 2023
@GuillaumeGomez
Rollup merge of rust-lang#113427 - compiler-errors:no-variances-of-rp…
ba711ba 
…itit-gat, r=spastorino

Remove `variances_of` on RPITIT GATs, remove its one use-case

It doesn't make sense to implement variances on a GAT anyways, since we don't relate GATs with variance:

https://github.com/rust-lang/rust/blob/85bf07972a1041b9e25393b803d0e006bec3eaaf/compiler/rustc_middle/src/ty/relate.rs#L569-L579

r? `@spastorino`
@GuillaumeGomez GuillaumeGomez mentioned this pull request Jul 7, 2023
Closed
@compiler-errors
Copy link
Member Author

@bors rollup

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jul 7, 2023
@GuillaumeGomez
Rollup merge of rust-lang#113427 - compiler-errors:no-variances-of-rp…
d8de054 
…itit-gat, r=spastorino

Remove `variances_of` on RPITIT GATs, remove its one use-case

It doesn't make sense to implement variances on a GAT anyways, since we don't relate GATs with variance:

https://github.com/rust-lang/rust/blob/85bf07972a1041b9e25393b803d0e006bec3eaaf/compiler/rustc_middle/src/ty/relate.rs#L569-L579

r? `@spastorino`
@GuillaumeGomez GuillaumeGomez mentioned this pull request Jul 7, 2023
Closed
@compiler-errors compiler-errors mentioned this pull request Jul 8, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 8, 2023
@bors
Auto merge of rust-lang#113474 - compiler-errors:rollup-07x1up7, r=co…
ce519c5 
…mpiler-errors

Rollup of 8 pull requests

Successful merges:

 - rust-lang#113413 (Add needs-triage to all new issues)
 - rust-lang#113426 (Don't ICE in `resolve_bound_vars` when associated return-type bounds are in bad positions)
 - rust-lang#113427 (Remove `variances_of` on RPITIT GATs, remove its one use-case)
 - rust-lang#113441 (miri: check that assignments do not self-overlap)
 - rust-lang#113453 (Remove unused from_method from rustc_on_unimplemented)
 - rust-lang#113456 (Avoid calling report_forbidden_specialization for RPITITs)
 - rust-lang#113466 (Update cargo)
 - rust-lang#113467 (Fix comment of `fn_can_unwind`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 751dcac into rust-lang:master Jul 8, 2023
11 checks passed
@rustbot rustbot added this to the 1.72.0 milestone Jul 8, 2023
@compiler-errors compiler-errors deleted the no-variances-of-rpitit-gat branch August 11, 2023 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@spastorino spastorino

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

Successfully merging this pull request may close these issues.
4 participants
@compiler-errors @spastorino @bors @rustbot