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

don't touch closure_substs.parent_substs() in borrowck #100375

Closed
wants to merge 1 commit into from
Closed

don't touch closure_substs.parent_substs() in borrowck #100375

wants to merge 1 commit into from

Conversation

aliemjay
Copy link
Member

@aliemjay aliemjay commented Aug 10, 2022 •
edited
Loading

... They bite!

An alternative implementation of #98835 that doesn't add any additional constraints on closure_substs.parent_substs in order to not change the behavior of opaque type inference. See #100372.

See the comment on the test for more details.

Fixes #100267.

r? types

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 10, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 10, 2022
@aliemjay aliemjay force-pushed the ugly-closure-substs branch from aab2c8a to 2b9a220 Compare August 10, 2022 14:25
@spastorino
Copy link
Member

r? types

@rust-highfive rust-highfive assigned jackh726 and unassigned spastorino Aug 10, 2022
@aliemjay aliemjay mentioned this pull request Aug 13, 2022
Closed
@jackh726 jackh726 mentioned this pull request Aug 18, 2022
Merged
@bors
Copy link
Contributor

bors commented Sep 10, 2022

☔ The latest upstream changes (presumably #98559) made this pull request unmergeable. Please resolve the merge conflicts.

jackh726
jackh726 reviewed Sep 12, 2022
View reviewed changes
compiler/rustc_borrowck/src/region_infer/opaque_types.rs Outdated
Comment on lines 110 to 111
// FIXME: This is a troublesome back-compat hack. See #100372.
.unwrap_or(infcx.tcx.lifetimes.re_root_empty),
Copy link
Member

Choose a reason for hiding this comment

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

Now that ReEmpty has removed, does this PR work?

@PureWhiteWu
Copy link

Hi, is there any progress on this bugfix?
This bug has caused us unable to use the latest nightly version for one and a half month.
Thank you all for your work!

@apiraino
Copy link
Contributor

I think this is waiting on @aliemjay to check this comment. Please request another review with @rustbot ready, thanks!

@rustbot author

@rustbot rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 26, 2022
@jackh726 jackh726 force-pushed the ugly-closure-substs branch from 2b9a220 to 4f32381 Compare November 7, 2022 00:10
@jackh726
Copy link
Member

jackh726 commented Nov 7, 2022

I went ahead and rebased this. It's not clear we should land this at this point. The only behavior difference I see is to the src/test/ui/type-alias-impl-trait/closure_wf_outlives.rs test, where we no longer require bounds on the GAT parameters (except for T: 'static).

@jackh726
Copy link
Member

jackh726 commented Nov 7, 2022

Let me go ahead and nominate.

@jackh726 jackh726 added the I-types-nominated Nominated for discussion during a types team meeting. label Nov 7, 2022
@lcnr lcnr removed the I-types-nominated Nominated for discussion during a types team meeting. label Jan 18, 2023
@lcnr
Copy link
Contributor

lcnr commented Jan 18, 2023

https://rust-lang.zulipchat.com/#narrow/stream/326132-t-types.2Fmeetings/topic/2023-01-18.20meeting/near/322078253

unnominating with the expectation that the PR is now unnecessary as the added tests already seem to pass. If not feel free to update the PR description and renominate 🤔

@jackh726
Copy link
Member

@oli-obk can you look at src/test/ui/type-alias-impl-trait/closure_wf_outlives.rs and let me you think this change here is good or not?

@oli-obk
Copy link
Contributor

oli-obk commented Feb 21, 2023

looks good, but the now-passing parts should be pulled out into a separate check-pass test

@Dylan-DPC
Copy link
Member

@aliemjay any updates on this?

@JohnCSimon
Copy link
Member

@aliemjay

Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this.
Note: if you are going to continue please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks for your contribution.

@rustbot label: +S-inactive

@JohnCSimon JohnCSimon closed this Dec 17, 2023
@rustbot rustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Dec 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jackh726 jackh726 jackh726 left review comments

Assignees

@jackh726 jackh726

Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Prove closure bound for TAIT, lifetime bound not satisfied