Skip to content

Rework borrowing suggestions to use Expr instead of just Span #143742

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

Merged
merged 3 commits into from
Jul 11, 2025

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Jul 10, 2025

In the suggestion machinery for borrowing expressions and types, always use the available obligation Span to find the appropriate Expr to perform appropriateness checks no the ExprKind instead of on the textual snippet corresponding to the Span. (We were already doing this, but only for a subset of cases.) This now better handles situations where parentheses and <> are needed for correct syntax (&(foo + bar), (&foo).bar(), <&Foo>::bar(), etc.).

Unify the logic for the case where & and &mut are appropriate with the logic for only one of those cases. (Instead of having two branches for emitting the suggestion, we now have a single one, using Diag::multipart_suggestions always.)

Handle the case when S::foo() should have been <&S>::foo() (instead of suggesting the prior &S::foo(). Fix #143393.

Make Diag::multipart_suggestions always verbose. CC #141973.

estebank added 3 commits July 10, 2025 17:23
In the suggestion machinery for borrowing expressions and types, always use the available obligation `Span` to find the appropriate `Expr` to perform appropriateness checks no the `ExprKind` instead of on the textual snippet corresponding to the `Span`.

Unify the logic for the case where `&` *and* `&mut` are appropriate with the logic for only one of those cases.

Handle the case when `S::foo()` should have been `<&S>::foo()` (instead of suggesting the prior `&S::foo()`.
@rustbot
Copy link
Collaborator

rustbot commented Jul 10, 2025

r? @fee1-dead

rustbot has assigned @fee1-dead.
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 Jul 10, 2025
@estebank
Copy link
Contributor Author

I recommend using "hide whitespace" option in the diff view, as some of the changes are only on the indentation level.

@compiler-errors
Copy link
Member

r? compiler-errors @bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jul 10, 2025

📌 Commit d692444 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 Jul 10, 2025
tgross35 added a commit to tgross35/rust that referenced this pull request Jul 10, 2025
…iler-errors

Rework borrowing suggestions to use `Expr` instead of just `Span`

In the suggestion machinery for borrowing expressions and types, always use the available obligation `Span` to find the appropriate `Expr` to perform appropriateness checks no the `ExprKind` instead of on the textual snippet corresponding to the `Span`. (We were already doing this, but only for a subset of cases.) This now better handles situations where parentheses and `<>` are needed for correct syntax (`&(foo + bar)`, `(&foo).bar()`, `<&Foo>::bar()`, etc.).

Unify the logic for the case where `&` *and* `&mut` are appropriate with the logic for only one of those cases. (Instead of having two branches for emitting the suggestion, we now have a single one, using `Diag::multipart_suggestions` always.)

Handle the case when `S::foo()` should have been `<&S>::foo()` (instead of suggesting the prior `&S::foo()`. Fix rust-lang#143393.

Make `Diag::multipart_suggestions` always verbose. CC rust-lang#141973.
tgross35 added a commit to tgross35/rust that referenced this pull request Jul 10, 2025
…iler-errors

Rework borrowing suggestions to use `Expr` instead of just `Span`

In the suggestion machinery for borrowing expressions and types, always use the available obligation `Span` to find the appropriate `Expr` to perform appropriateness checks no the `ExprKind` instead of on the textual snippet corresponding to the `Span`. (We were already doing this, but only for a subset of cases.) This now better handles situations where parentheses and `<>` are needed for correct syntax (`&(foo + bar)`, `(&foo).bar()`, `<&Foo>::bar()`, etc.).

Unify the logic for the case where `&` *and* `&mut` are appropriate with the logic for only one of those cases. (Instead of having two branches for emitting the suggestion, we now have a single one, using `Diag::multipart_suggestions` always.)

Handle the case when `S::foo()` should have been `<&S>::foo()` (instead of suggesting the prior `&S::foo()`. Fix rust-lang#143393.

Make `Diag::multipart_suggestions` always verbose. CC rust-lang#141973.
bors added a commit that referenced this pull request Jul 10, 2025
Rollup of 8 pull requests

Successful merges:

 - #140136 (Add an aarch64-msvc build running on ARM64 Windows)
 - #143642 (stdarch subtree update)
 - #143679 (Preserve the .debug_gdb_scripts section)
 - #143707 (Fix `--skip-std-check-if-no-download-rustc`)
 - #143722 (Make some "safe" llvm ops actually sound)
 - #143728 (Resolve refactor: extraction of `finalize_module_binding` and `single_import_can_define_name`)
 - #143742 (Rework borrowing suggestions to use `Expr` instead of just `Span`)
 - #143744 (Properly track the depth when expanding free alias types)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Jul 11, 2025
Rollup of 7 pull requests

Successful merges:

 - #140136 (Add an aarch64-msvc build running on ARM64 Windows)
 - #143642 (stdarch subtree update)
 - #143707 (Fix `--skip-std-check-if-no-download-rustc`)
 - #143722 (Make some "safe" llvm ops actually sound)
 - #143728 (Resolve refactor: extraction of `finalize_module_binding` and `single_import_can_define_name`)
 - #143742 (Rework borrowing suggestions to use `Expr` instead of just `Span`)
 - #143744 (Properly track the depth when expanding free alias types)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 58e5c26 into rust-lang:master Jul 11, 2025
11 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jul 11, 2025
rust-timer added a commit that referenced this pull request Jul 11, 2025
Rollup merge of #143742 - estebank:borrow-suggestion, r=compiler-errors

Rework borrowing suggestions to use `Expr` instead of just `Span`

In the suggestion machinery for borrowing expressions and types, always use the available obligation `Span` to find the appropriate `Expr` to perform appropriateness checks no the `ExprKind` instead of on the textual snippet corresponding to the `Span`. (We were already doing this, but only for a subset of cases.) This now better handles situations where parentheses and `<>` are needed for correct syntax (`&(foo + bar)`, `(&foo).bar()`, `<&Foo>::bar()`, etc.).

Unify the logic for the case where `&` *and* `&mut` are appropriate with the logic for only one of those cases. (Instead of having two branches for emitting the suggestion, we now have a single one, using `Diag::multipart_suggestions` always.)

Handle the case when `S::foo()` should have been `<&S>::foo()` (instead of suggesting the prior `&S::foo()`. Fix #143393.

Make `Diag::multipart_suggestions` always verbose. CC #141973.
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.

Borrow suggestion on unmet obligation doesn't account for types in type-relative paths
5 participants