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.

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

Rudimentary heuristic to insert parentheses when needed for RPIT overcaptures lint #134142

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

compiler-errors
Copy link
Member

We don't have basically any preexisting machinery to detect when parentheses are needed for types. AFAICT, all of the diagnostics we have for opaques just... fail when they suggest + 'a when that's ambiguous.

Fixes #132853

@rustbot
Copy link
Collaborator

rustbot commented Dec 10, 2024

r? @jieyouxu

rustbot has assigned @jieyouxu.
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 Dec 10, 2024
Copy link
Member

@fmease fmease Dec 10, 2024

Choose a reason for hiding this comment

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

We don't have basically any preexisting machinery to detect when parentheses are needed for types

We have hir::Generics::bounds_span_for_suggestions() for bounds (on generic params I guess?). I don't know if you can leverage it here in some way or generalize it. I haven't looked at it for a while.

Edit: And yeah, it checks for TyKind::Ref but not for TyKind::Ptr, I've been meaning to fix that for some time.

cc #120929

Copy link
Member Author

Choose a reason for hiding this comment

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

That's more about ambiguities introduced by the bound of the RPIT themselves. This fixes ambiguities having to do with the parent type of the RPIT. I think both could use fixing and unification, but I don't think I'll do it here.

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks. This probably will still have more edge cases involving... more interesting combination of syntax, but this is a very nice improvement in the meantime.

@jieyouxu
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 11, 2024

📌 Commit e134c74 has been approved by jieyouxu

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 Dec 11, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 11, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#134079 (Add a note saying that `{u8,i8}::from_{be,le,ne}_bytes` is meaningless)
 - rust-lang#134105 (Validate self in host predicates correctly)
 - rust-lang#134136 (Exercise const trait interaction with default fields)
 - rust-lang#134139 ([AIX] keep profile-rt symbol alive)
 - rust-lang#134141 (Remove more traces of anonymous ADTs)
 - rust-lang#134142 (Rudimentary heuristic to insert parentheses when needed for RPIT overcaptures lint)
 - rust-lang#134158 (Rename `projection_def_id` to `item_def_id`)
 - rust-lang#134160 (Add vacation entry for myself in triagebot.toml)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 11, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#134079 (Add a note saying that `{u8,i8}::from_{be,le,ne}_bytes` is meaningless)
 - rust-lang#134105 (Validate self in host predicates correctly)
 - rust-lang#134136 (Exercise const trait interaction with default fields)
 - rust-lang#134139 ([AIX] keep profile-rt symbol alive)
 - rust-lang#134141 (Remove more traces of anonymous ADTs)
 - rust-lang#134142 (Rudimentary heuristic to insert parentheses when needed for RPIT overcaptures lint)
 - rust-lang#134158 (Rename `projection_def_id` to `item_def_id`)
 - rust-lang#134160 (Add vacation entry for myself in triagebot.toml)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f103076 into rust-lang:master Dec 11, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 11, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 11, 2024
Rollup merge of rust-lang#134142 - compiler-errors:paren-sug, r=jieyouxu

Rudimentary heuristic to insert parentheses when needed for RPIT overcaptures lint

We don't have basically any preexisting machinery to detect when parentheses are needed for *types*. AFAICT, all of the diagnostics we have for opaques just... fail when they suggest `+ 'a` when that's ambiguous.

Fixes rust-lang#132853
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

2024 impl_trait_overcaptures ambiguous +
5 participants