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

Fix incorrect suggestion for undeclared hrtb lifetimes in where clauses. #123122

Merged
merged 1 commit into from
May 21, 2024
Merged

Fix incorrect suggestion for undeclared hrtb lifetimes in where clauses. #123122

merged 1 commit into from
May 21, 2024

Conversation

surechen
Copy link
Contributor

For poly-trait-ref like for<'a> Trait<T> in T: for<'a> Trait<T> + 'b { }.
We should merge the hrtb lifetimes: existed for<'a> and suggestion for<'b> or will get err: [E0316] nested quantification of lifetimes

fixes #122714

@rustbot
Copy link
Collaborator

rustbot commented Mar 27, 2024

r? @oli-obk

rustbot has assigned @oli-obk.
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 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 Mar 27, 2024
fmease
fmease reviewed Mar 28, 2024
View reviewed changes
compiler/rustc_resolve/src/late.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/late/diagnostics.rs Outdated Show resolved Hide resolved
@surechen surechen force-pushed the fix_122714 branch 2 times, most recently from 2941db4 to db9b023 Compare March 28, 2024 07:22
@surechen surechen requested a review from fmease March 28, 2024 11:41
@fmease fmease assigned fmease and unassigned oli-obk Apr 7, 2024
oli-obk
oli-obk reviewed Apr 8, 2024
View reviewed changes
compiler/rustc_resolve/src/late.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/late/diagnostics.rs Outdated Show resolved Hide resolved
@fmease fmease assigned oli-obk and unassigned fmease Apr 8, 2024
@surechen surechen requested a review from oli-obk April 9, 2024 10:57
@surechen surechen requested a review from oli-obk April 9, 2024 12:02
fmease
fmease requested changes Apr 9, 2024
View reviewed changes
compiler/rustc_resolve/src/late.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/late.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/late/diagnostics.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/late/diagnostics.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/late/diagnostics.rs Outdated Show resolved Hide resolved
@oli-obk
Copy link
Contributor

oli-obk commented Apr 9, 2024

r? @fmease

@rustbot rustbot assigned fmease and unassigned oli-obk Apr 9, 2024
@surechen surechen requested a review from fmease April 10, 2024 01:22
fmease
fmease requested changes Apr 23, 2024
View reviewed changes
Copy link
Member

@fmease fmease left a comment

Choose a reason for hiding this comment

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

Apologies for the delay! I've potentially found some smaller issues. Furthermore, I'm not super convinced by the current approach which adds an entirely new field to the resolver just to fix a niche diagnostic bug. I go into more detail in some of my review comments.

compiler/rustc_resolve/src/late.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/late.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/late/diagnostics.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/late/diagnostics.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/late/diagnostics.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/late/diagnostics.rs Show resolved Hide resolved
@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 Apr 23, 2024
@surechen
Copy link
Contributor Author

Apologies for the delay! I've potentially found some smaller issues. Furthermore, I'm not super convinced by the current approach which adds an entirely new field to the resolver just to fix a niche diagnostic bug. I go into more detail in some of my review comments.

Thank you very much. Your review is very detailed and helps me a lot. I will spend some time processing it.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@surechen surechen requested a review from fmease April 29, 2024 01:09
@rustbot rustbot 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 Apr 29, 2024
fmease
fmease approved these changes May 18, 2024
View reviewed changes
Copy link
Member

@fmease fmease left a comment

Choose a reason for hiding this comment

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

Thanks a lot, that looks great! ❤️ I apologize for taking so long (yet again!) 😞.

Almost ready, I have a couple of small suggestions. Then I will put into the merge queue!

compiler/rustc_resolve/src/late/diagnostics.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/late/diagnostics.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/late/diagnostics.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/late/diagnostics.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/late/diagnostics.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/late/diagnostics.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/late/diagnostics.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/late/diagnostics.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/late/diagnostics.rs Outdated Show resolved Hide resolved
@fmease fmease 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 May 18, 2024
@surechen surechen requested a review from fmease May 21, 2024 01:22
@rustbot rustbot 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 May 21, 2024
@surechen
Copy link
Contributor Author

Thanks a lot, that looks great! ❤️ I apologize for taking so long (yet again!) 😞.

Almost ready, I have a couple of small suggestions. Then I will put into the merge queue!

Thank you very much. Please help me review again.

fmease
fmease approved these changes May 21, 2024
View reviewed changes
Copy link
Member

@fmease fmease left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@fmease
Copy link
Member

fmease commented May 21, 2024

@bors r+ rollup

@bors
Copy link
Contributor

bors commented May 21, 2024

📌 Commit 4ebbb5f has been approved by fmease

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 May 21, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 21, 2024
@matthiaskrgr
Rollup merge of rust-lang#123122 - surechen:fix_122714, r=fmease
48f0e23 
Fix incorrect suggestion for undeclared hrtb lifetimes in where clauses.

For poly-trait-ref like `for<'a> Trait<T>`   in  `T: for<'a> Trait<T> + 'b { }`.
We should merge the hrtb lifetimes: existed `for<'a>` and suggestion `for<'b>` or will get err: [E0316] nested quantification of lifetimes

fixes rust-lang#122714
@matthiaskrgr matthiaskrgr mentioned this pull request May 21, 2024
Closed
fmease added a commit to fmease/rust that referenced this pull request May 21, 2024
@fmease
Rollup merge of rust-lang#123122 - surechen:fix_122714, r=fmease
43bf82a 
Fix incorrect suggestion for undeclared hrtb lifetimes in where clauses.

For poly-trait-ref like `for<'a> Trait<T>`   in  `T: for<'a> Trait<T> + 'b { }`.
We should merge the hrtb lifetimes: existed `for<'a>` and suggestion `for<'b>` or will get err: [E0316] nested quantification of lifetimes

fixes rust-lang#122714
@fmease fmease mentioned this pull request May 21, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request May 21, 2024
@bors
Auto merge of rust-lang#125371 - matthiaskrgr:rollup-poo7wsl, r=matth…
2b45282 
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#122665 (Add some tests for public-private dependencies.)
 - rust-lang#123122 (Fix incorrect suggestion for undeclared hrtb lifetimes in where clauses.)
 - rust-lang#125276 (Fix parsing of erroneously placed semicolons)
 - rust-lang#125310 (Move ~100 tests from tests/ui to subdirs)
 - rust-lang#125357 (Migrate `run-make/rustdoc-scrape-examples-multiple` to `rmake.rs`)
 - rust-lang#125369 (Don't do cc detection for synthetic targets)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr matthiaskrgr mentioned this pull request May 21, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 21, 2024
@bors
Auto merge of rust-lang#125379 - matthiaskrgr:rollup-6149w01, r=matth…
39e02f1 
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#123122 (Fix incorrect suggestion for undeclared hrtb lifetimes in where clauses.)
 - rust-lang#123492 (add pull request template asking for relevant tracking issues)
 - rust-lang#125276 (Fix parsing of erroneously placed semicolons)
 - rust-lang#125310 (Move ~100 tests from tests/ui to subdirs)
 - rust-lang#125357 (Migrate `run-make/rustdoc-scrape-examples-multiple` to `rmake.rs`)
 - rust-lang#125369 (Don't do cc detection for synthetic targets)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6009cb7 into rust-lang:master May 21, 2024
6 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 21, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 21, 2024
@rust-timer
Unrolled build for rust-lang#123122
22a1275 
Rollup merge of rust-lang#123122 - surechen:fix_122714, r=fmease

Fix incorrect suggestion for undeclared hrtb lifetimes in where clauses.

For poly-trait-ref like `for<'a> Trait<T>`   in  `T: for<'a> Trait<T> + 'b { }`.
We should merge the hrtb lifetimes: existed `for<'a>` and suggestion `for<'b>` or will get err: [E0316] nested quantification of lifetimes

fixes rust-lang#122714
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fmease fmease fmease approved these changes

@oli-obk oli-obk Awaiting requested review from oli-obk

Assignees

@fmease fmease

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.80.0
6 participants
@surechen @rustbot @oli-obk @rust-log-analyzer @fmease @bors