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

Do anonymous lifetimes remapping correctly for nested rpits #103205

Merged
merged 2 commits into from Oct 20, 2022
Merged

Do anonymous lifetimes remapping correctly for nested rpits #103205

merged 2 commits into from Oct 20, 2022

Conversation

spastorino
Copy link
Member

Closes #103141

r? @cjgillot @nikomatsakis

This fixes a stable to stable regression that in my opinion is P-critical so, we probably want to backport it all the way up to stable.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 18, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 18, 2022
@spastorino spastorino changed the title Do lifetimes remapping correctly for nested rpits Do anonymous lifetimes remapping correctly for nested rpits Oct 18, 2022
@spastorino
Copy link
Member Author

@cjgillot, pushed a different change that changes the remappings to always be from the main outermost def_id, the innermost ones. That means that the remapping chain shouldn't be needed.

@spastorino spastorino added beta-nominated Nominated for backporting to the compiler in the beta channel. stable-nominated Nominated for backporting to the compiler in the stable channel. labels Oct 18, 2022
@rustbot rustbot added the A-testsuite Area: The testsuite used to check the correctness of rustc label Oct 19, 2022
@spastorino
Copy link
Member Author

@cjgillot this is ready now. I've just pushed again because I've found that's better to have the same set of "orig" methods and remapped methods, said orig_local_def_id and orig_opt_local_def_id as we have local_def_id and opt_local_def_id. That saves to unwrap in the call site.

@cjgillot
Copy link
Contributor

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented Oct 19, 2022

📌 Commit 49ce8a2 has been approved by cjgillot

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 Oct 19, 2022
@bors
Copy link
Contributor

bors commented Oct 20, 2022

⌛ Testing commit 49ce8a2 with merge ebdde35...

@bors
Copy link
Contributor

bors commented Oct 20, 2022

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing ebdde35 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 20, 2022
@bors bors merged commit ebdde35 into rust-lang:master Oct 20, 2022
@rustbot rustbot added this to the 1.66.0 milestone Oct 20, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (ebdde35): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -2.2% [-2.2%, -2.2%] 1
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

Footnotes

  1. the arithmetic mean of the percent change ↩

  2. number of relevant changes ↩

@apiraino
Copy link
Contributor

apiraino commented Oct 20, 2022 •
edited

Beta backport accepted as per compiler team on Zulip

@rustbot label +beta-accepted

@rustbot rustbot added beta-accepted Accepted for backporting to the compiler in the beta channel. stable-accepted Accepted for backporting to the compiler in the stable channel. labels Oct 20, 2022
@apiraino apiraino removed the stable-accepted Accepted for backporting to the compiler in the stable channel. label Oct 20, 2022
@cuviper cuviper mentioned this pull request Oct 20, 2022
Merged
@cuviper cuviper modified the milestones: 1.66.0, 1.65.0 Oct 20, 2022
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 20, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 21, 2022
@bors
Auto merge of rust-lang#103316 - cuviper:beta-next, r=cuviper
a317055 
[beta] backports

- Use rebind instead of dummy binder in `SameTypeModuloInfer` relation rust-lang#102059
- Add missing space between notable trait tooltip and where clause rust-lang#102107
- Avoid repeated re-initialization of the BufReader buffer rust-lang#102760
- Ensure enum cast moves rust-lang#103016
- Fix `TyKind::is_simple_path` rust-lang#103176
- Do anonymous lifetimes remapping correctly for nested rpits rust-lang#103205
- [beta] Cargo backport 1.65.0 rust-lang#103303
- linker: Fix weak lang item linking with combination windows-gnu + LLD + LTO rust-lang#103092

r? `@ghost`
@spastorino
Copy link
Member Author

This was stable-accepted https://rust-lang.zulipchat.com/#narrow/stream/238009-t-compiler.2Fmeetings/topic/.5Bweekly.5D.202022-10-20/near/305212449

@rustbot label +stable-accepted -stable-nominated

@rustbot rustbot added stable-accepted Accepted for backporting to the compiler in the stable channel. and removed stable-nominated Nominated for backporting to the compiler in the stable channel. labels Oct 21, 2022
@spastorino
Copy link
Member Author

@cuviper this was meant to be stable-accepted, unsure if you need to revisit something here.

@spastorino spastorino mentioned this pull request Oct 21, 2022
Merged
@cuviper
Copy link
Member

cuviper commented Oct 21, 2022

For stable backports, we wait until there's an actual stable point release in motion, but it should keep both labels until then so we know it hasn't been done.

@cuviper cuviper added the stable-nominated Nominated for backporting to the compiler in the stable channel. label Oct 21, 2022
@apiraino apiraino added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 21, 2022
@cuviper
Copy link
Member

cuviper commented Oct 21, 2022

I already included this one in beta, #103316.

@rustbot label -beta-nominated

@rustbot rustbot removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 21, 2022
@apiraino apiraino mentioned this pull request Nov 9, 2022
Closed
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
@bors
Auto merge of rust-lang#103205 - spastorino:fix-rpits-lifetime-remapp…
1cc4e3c 
…ing, r=cjgillot

Do anonymous lifetimes remapping correctly for nested rpits

Closes rust-lang#103141

r? `@cjgillot` `@nikomatsakis`

This fixes a stable to stable regression that in my opinion is `P-critical` so, we probably want to backport it all the way up to stable.
@Mark-Simulacrum Mark-Simulacrum removed stable-nominated Nominated for backporting to the compiler in the stable channel. stable-accepted Accepted for backporting to the compiler in the stable channel. labels Mar 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjgillot cjgillot cjgillot left review comments

Assignees

@cjgillot cjgillot

Labels
A-testsuite Area: The testsuite used to check the correctness of rustc beta-accepted Accepted for backporting to the compiler in the beta channel. merged-by-bors This PR was explicitly merged by bors. 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.65.0
Development

Successfully merging this pull request may close these issues.

Rust update from 1.63 to 1.64 introduce lifetime problem on build
9 participants
@spastorino @cjgillot @bors @rust-timer @apiraino @cuviper @Mark-Simulacrum @rust-highfive @rustbot