Skip to content

Adjust and slightly generalize operator error suggestion - #101424

Merged
bors merged 5 commits into
rust-lang:masterfrom
compiler-errors:operator-err-sugg
Sep 8, 2022
Merged

Adjust and slightly generalize operator error suggestion#101424
bors merged 5 commits into
rust-lang:masterfrom
compiler-errors:operator-err-sugg

Conversation

@compiler-errors

@compiler-errorscompiler-errors commented Sep 4, 2022

Copy link
Copy Markdown
Contributor

(in no particular order)

  • Stop passing around a whole extra ProjectionPredicate
  • Add spaces around = in Trait<..., Output = Ty> suggestion
  • Some code clean-ups, including
    • add lang_item_for_op to turn a Op into a DefId
    • avoid SourceMap because we don't really need to render an expr
    • Remove TypeParamVisitor in favor of just checking ty.has_param_types_or_consts -- this acts a bit differently, but shouldn't cause erroneous suggestions (actually might generalize them a bit)
  • We now suggest Output = Ty in the where clause suggestion when we fail to add Struct<T> and T.

I can split this out into more PRs if needed, but they're all just miscellaneous generalizations, changes, and nitpicks I saw when messing with this operator code.

@rustbotrustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 4, 2022
@rust-highfive

rust-highfive commented Sep 4, 2022

Copy link
Copy Markdown
Contributor

r? @TaKO8Ki

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 4, 2022
compiler-errors
compiler-errors commented Sep 4, 2022
View reviewed changes
Comment threadcompiler/rustc_middle/src/ty/diagnostics.rs Outdated
compiler-errors
compiler-errors commented Sep 4, 2022
View reviewed changes
Comment threadcompiler/rustc_typeck/src/check/op.rs Outdated
Comment threadcompiler/rustc_typeck/src/check/op.rs Outdated
Comment threadcompiler/rustc_typeck/src/check/op.rs Outdated
Comment threadcompiler/rustc_typeck/src/check/op.rs Outdated

@compiler-errorscompiler-errorsSep 4, 2022

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Peeling the type here is actually wrong -- when we have /* &&i32 */ + /* i32 */ and we deref the LHS, we are adding &i32 and i32, not i32 and i32.

TaKO8Ki
TaKO8Ki reviewed Sep 6, 2022
View reviewed changes

@TaKO8KiTaKO8Ki left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me. r=me after a nit is addressed or not.

Comment threadcompiler/rustc_typeck/src/check/op.rs Outdated

@TaKO8KiTaKO8KiSep 6, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: Option::and_then can simplify this.

Suggested change
let output_def_id = trait_def_id.map_or(None,|def_id| {
let output_def_id = trait_def_id.and_then(|def_id| {

@compiler-errorscompiler-errorsSep 8, 2022•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Do you mean is_some_and?
edit: I was looking at the wrong map_or in that file... 🤦 you're right!

@bors

bors commented Sep 7, 2022

Copy link
Copy Markdown
Collaborator

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

@compiler-errors

compiler-errors commented Sep 8, 2022

Copy link
Copy Markdown
ContributorAuthor

@bors r=TaKO8Ki

@bors

bors commented Sep 8, 2022

Copy link
Copy Markdown
Collaborator

📌 Commit 48281b0 has been approved by TaKO8Ki

It is now in the queue for this repository.

@borsbors 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 Sep 8, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 8, 2022
@Dylan-DPC
Rollup merge ofrust-lang#101424- compiler-errors:operator-err-sugg,…
905e5e4
… r=TaKO8Ki
Adjust and slightly generalize operator error suggestion
(in no particular order)
* Stop passing around a whole extra `ProjectionPredicate`
* Add spaces around `=` in `Trait<..., Output = Ty>` suggestion
* Some code clean-ups, including
* add `lang_item_for_op` to turn a `Op` into a `DefId`
* avoid `SourceMap` because we don't really need to render an expr
* Remove `TypeParamVisitor` in favor of just checking `ty.has_param_types_or_consts` -- this acts a bit differently, but shouldn't cause erroneous suggestions (actually might generalize them a bit)
* We now suggest `Output = Ty` in the `where` clause suggestion when we fail to add `Struct<T>` and `T`.
I can split this out into more PRs if needed, but they're all just miscellaneous generalizations, changes, and nitpicks I saw when messing with this operator code.
This was referenced Sep 8, 2022
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 8, 2022
@bors
Auto merge ofrust-lang#101577- Dylan-DPC:rollup-l9xw7i7, r=Dylan-DPC
8778809
Rollup of 7 pull requests
Successful merges:
- rust-lang#98933 (Opaque types' generic params do not imply anything about their hidden type's lifetimes)
- rust-lang#101041 (translations(rustc_session): migrates rustc_session to use SessionDiagnostic - Pt. 2)
- rust-lang#101424 (Adjust and slightly generalize operator error suggestion)
- rust-lang#101496 (Allow lower_lifetime_binder receive a closure)
- rust-lang#101501 (Allow lint passes to be bound by `TyCtxt`)
- rust-lang#101515 (Recover from typo where == is used in place of =)
- rust-lang#101545 (Remove unnecessary `PartialOrd` and `Ord`)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit b5ffbd3 into rust-lang:masterSep 8, 2022
@rustbotrustbot added this to the 1.65.0 milestone Sep 8, 2022
flip1995 pushed a commit to flip1995/rust that referenced this pull request Oct 6, 2022
@bors
Auto merge ofrust-lang#101577- Dylan-DPC:rollup-l9xw7i7, r=Dylan-DPC
f47a1e2
Rollup of 7 pull requests
Successful merges:
- rust-lang#98933 (Opaque types' generic params do not imply anything about their hidden type's lifetimes)
- rust-lang#101041 (translations(rustc_session): migrates rustc_session to use SessionDiagnostic - Pt. 2)
- rust-lang#101424 (Adjust and slightly generalize operator error suggestion)
- rust-lang#101496 (Allow lower_lifetime_binder receive a closure)
- rust-lang#101501 (Allow lint passes to be bound by `TyCtxt`)
- rust-lang#101515 (Recover from typo where == is used in place of =)
- rust-lang#101545 (Remove unnecessary `PartialOrd` and `Ord`)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
@compiler-errors
compiler-errors deleted the operator-err-sugg branch November 2, 2022 02:59
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TaKO8KiTaKO8KiTaKO8Ki left review comments

Assignees

@TaKO8KiTaKO8Ki

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant 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.

5 participants

@compiler-errors@rust-highfive@bors@TaKO8Ki@rustbot