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

Recover from lifetimes with default lifetimes in generic args #107580

Merged
merged 1 commit into from
Feb 6, 2023
Merged

Recover from lifetimes with default lifetimes in generic args #107580

merged 1 commit into from
Feb 6, 2023

Conversation

lenko-d
Copy link
Contributor

@lenko-d lenko-d commented Feb 2, 2023

Fixes #107492

@rustbot
Copy link
Collaborator

rustbot commented Feb 2, 2023

r? @WaffleLapkin

(rustbot has picked a reviewer for you, use r? to override)

@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 Feb 2, 2023
@lenko-d lenko-d marked this pull request as ready for review February 2, 2023 04:08
compiler-errors
compiler-errors reviewed Feb 2, 2023
View reviewed changes
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

The general approach looks fine, but I have some changes.

Can you give this PR a more descriptive name? Instead of explaining the problem, it should explain the solution... maybe "recover from lifetimes with default lifetimes in generic args", or something else.

compiler/rustc_parse/src/parser/generics.rs Outdated Show resolved Hide resolved
compiler/rustc_parse/src/parser/generics.rs Outdated Show resolved Hide resolved
@lenko-d lenko-d changed the title Default value for a lifetime generic parameter produces confusing diagnostics Recover from lifetimes with default lifetimes in generic args Feb 2, 2023
@lenko-d
Copy link
Contributor Author

lenko-d commented Feb 3, 2023

@rustbot label -S-waiting-on-review +S-waiting-on-author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 3, 2023
@rustbot
Copy link
Collaborator

rustbot commented Feb 4, 2023

rustc_error_messages was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki

compiler-errors
compiler-errors reviewed Feb 4, 2023
View reviewed changes
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

Thanks, one last change then I think this is ready to approve. Maybe we could also suggest removing the default lifetime param, but I don't think it's super necessary.

compiler/rustc_parse/src/parser/generics.rs Outdated Show resolved Hide resolved
tests/ui/lifetimes/issue-107492-default-value-for-lifetime.rs Outdated Show resolved Hide resolved
@compiler-errors
Copy link
Member

Also, please rebase off of the merge commit 338aa5cfd4e263303de24e7419f39120f1f67d9e, the repository has a "no merge commit" policy.

@lenko-d lenko-d force-pushed the default_value_for_a_lifetime_generic_parameter_produces_confusing_diagnostic branch from aac30a0 to c6934bb Compare February 4, 2023 04:30
@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-style Relevant to the style team, which will review and decide on the PR/issue. labels Feb 4, 2023
@rustbot
Copy link
Collaborator

rustbot commented Feb 4, 2023

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in src/doc/style-guide

cc @rust-lang/style

These commits modify compiler targets.
(See the Target Tier Policy.)

rustc_macros::diagnostics was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @Folyd, @jsha

@oli-obk
Copy link
Contributor

oli-obk commented Feb 4, 2023

I think your rebase hit the case described in https://rustc-dev-guide.rust-lang.org/git.html?highlight=rebase#git-is-trying-to-rebase-commits-i-didnt-write

This should resolve itself automatically by rebasing over the latest rust-lang/rust master commit again

@lenko-d lenko-d force-pushed the default_value_for_a_lifetime_generic_parameter_produces_confusing_diagnostic branch 2 times, most recently from 92883e2 to e58f9a8 Compare February 4, 2023 17:23
@lenko-d lenko-d force-pushed the default_value_for_a_lifetime_generic_parameter_produces_confusing_diagnostic branch from e58f9a8 to d9f6005 Compare February 4, 2023 23:11
@lenko-d
Copy link
Contributor Author

lenko-d commented Feb 5, 2023

@rustbot label -S-waiting-on-author +S-waiting-on-review

@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 Feb 5, 2023
compiler-errors
compiler-errors approved these changes Feb 6, 2023
View reviewed changes
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

Cool!

@compiler-errors
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 6, 2023

📌 Commit d9f6005 has been approved by compiler-errors

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 Feb 6, 2023
@Dylan-DPC Dylan-DPC mentioned this pull request Feb 6, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 6, 2023
@bors
Auto merge of rust-lang#107727 - Dylan-DPC:rollup-b1yexcl, r=Dylan-DPC
7ff69b4 
Rollup of 5 pull requests

Successful merges:

 - rust-lang#107553 (Suggest std::ptr::null if literal 0 is given to a raw pointer function argument)
 - rust-lang#107580 (Recover from lifetimes with default lifetimes in generic args)
 - rust-lang#107669 (rustdoc: combine duplicate rules in ayu CSS)
 - rust-lang#107685 (Suggest adding a return type for async functions)
 - rust-lang#107687 (Adapt SROA MIR opt for aggregated MIR)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 8ddbfad into rust-lang:master Feb 6, 2023
@rustbot rustbot added this to the 1.69.0 milestone Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@compiler-errors compiler-errors compiler-errors approved these changes

Assignees

@compiler-errors compiler-errors

Labels
A-testsuite Area: The testsuite used to check the correctness of rustc A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic 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. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-style Relevant to the style team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.69.0
Development

Successfully merging this pull request may close these issues.

Attempt to set a default value for a lifetime generic parameter produces confusing diagnostic
6 participants
@lenko-d @rustbot @compiler-errors @oli-obk @bors @WaffleLapkin