Skip to content

Suggest removing & when awaiting a reference to a future#154933

Open
fru1tworld wants to merge 1 commit intorust-lang:mainfrom
fru1tworld:fix-87211-ref-future-diagnostic
Open

Suggest removing & when awaiting a reference to a future#154933
fru1tworld wants to merge 1 commit intorust-lang:mainfrom
fru1tworld:fix-87211-ref-future-diagnostic

Conversation

@fru1tworld
Copy link
Copy Markdown
Contributor

@fru1tworld fru1tworld commented Apr 7, 2026 •
edited by rustbot
Loading

Fixes #87211

When .awaiting &impl Future, suggest removing the & instead of removing .await.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 7, 2026
@fru1tworld fru1tworld force-pushed the fix-87211-ref-future-diagnostic branch from f36f2b9 to 5afb519 Compare April 7, 2026 08:20
@fru1tworld fru1tworld marked this pull request as ready for review April 7, 2026 08:25
@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 7, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 7, 2026

r? @JohnTitor

rustbot has assigned @JohnTitor.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 69 candidates
  • Random selection from 11 candidates

chenyukang
chenyukang reviewed Apr 7, 2026
View reviewed changes
compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs
&& self
.type_implements_trait(future_trait, [inner_ty], obligation.param_env)
.must_apply_modulo_regions()
{
Copy link
Copy Markdown
Member

@chenyukang chenyukang Apr 7, 2026

Choose a reason for hiding this comment

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

maybe you want to use span_suggestion_verbose with Applicability::MaybeIncorrect.

chenyukang
chenyukang reviewed Apr 7, 2026
View reviewed changes
compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs
&& let future_trait =
self.tcx.require_lang_item(LangItem::Future, obligation.cause.span)
&& self
.type_implements_trait(future_trait, [inner_ty], obligation.param_env)
Copy link
Copy Markdown
Member

@chenyukang chenyukang Apr 7, 2026

Choose a reason for hiding this comment

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

This branch should not suggest removing & based only on inner_ty: Future. That also matches &dyn Future, where removing & is not a valid fix. Please reuse the existing span-based borrow-removal logic (see suggest_remove_reference or extract it into a helper) and only emit the fix-it when there is an actual borrow in the source.

test code for this is like:

async fn ref_param(fut: &impl Future<Output = ()>) {
    fut.await;
}

async fn dyn_ref_param(fut: &dyn Future<Output = ()>) {
    fut.await;
}

Copy link
Copy Markdown
Contributor Author

@fru1tworld fru1tworld Apr 7, 2026

Choose a reason for hiding this comment

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

Added !matches!(inner_ty.kind(), ty::Dynamic(..)) to skip &dyn Future and added a test.

@fru1tworld fru1tworld marked this pull request as draft April 7, 2026 12:03
@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 7, 2026
@fru1tworld fru1tworld force-pushed the fix-87211-ref-future-diagnostic branch from bff3e3b to cdd3efa Compare April 7, 2026 12:08
@fru1tworld fru1tworld marked this pull request as ready for review April 7, 2026 12:12
@fru1tworld fru1tworld requested a review from chenyukang April 7, 2026 12:12
@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 7, 2026
chenyukang
chenyukang reviewed Apr 7, 2026
View reviewed changes
tests/ui/async-await/await-ref-future.stderr Outdated
|
= help: the trait `Future` is not implemented for `&impl std::future::Future<Output = ()>`
= note: &impl std::future::Future<Output = ()> must be a future or must implement `IntoFuture` to be awaited
= help: a reference to a future is not a future; consider removing the leading `&`-reference
Copy link
Copy Markdown
Member

@chenyukang chenyukang Apr 7, 2026

Choose a reason for hiding this comment

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

the check does not works since it still suggesting here.

Copy link
Copy Markdown
Contributor Author

@fru1tworld fru1tworld Apr 7, 2026

Choose a reason for hiding this comment

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

Fixed - it now only suggests when there's an actual '&' in the source

@JohnTitor JohnTitor assigned chenyukang and unassigned JohnTitor Apr 7, 2026
@fru1tworld fru1tworld marked this pull request as draft April 7, 2026 12:49
@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 7, 2026
@fru1tworld fru1tworld force-pushed the fix-87211-ref-future-diagnostic branch from bc9d978 to 7e22d2d Compare April 7, 2026 23:17
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@fru1tworld fru1tworld force-pushed the fix-87211-ref-future-diagnostic branch from 7e22d2d to fe623c3 Compare April 7, 2026 23:33
@fru1tworld fru1tworld marked this pull request as ready for review April 7, 2026 23:37
@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 7, 2026
@fru1tworld fru1tworld requested a review from chenyukang April 7, 2026 23:37
chenyukang
chenyukang reviewed Apr 8, 2026
View reviewed changes
tests/ui/async-await/await-ref-future.stderr
error[E0277]: `&impl Future<Output = ()>` is not a future
--> $DIR/await-ref-future.rs:9:9
|
LL | fut.await;
Copy link
Copy Markdown
Member

@chenyukang chenyukang Apr 8, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

View changes since the review

this test is the code from original issue, now suggest remove & not working for it.

as I referred, you may need to reuse the existing span-based borrow-removal logic (see suggest_remove_reference or extract it into a helper)

@chenyukang
Copy link
Copy Markdown
Member

chenyukang commented Apr 8, 2026

nit:

it's not necessary to request review after each commit, reviewer can receive notification from your git push, and will review it when time available(it's normal to wait several days since a lot of us working in free time). https://rustc-dev-guide.rust-lang.org/contributing.html?highlight=reviewer#waiting-for-reviews

request review may makes them feel too pushy.

@fru1tworld fru1tworld force-pushed the fix-87211-ref-future-diagnostic branch from fe623c3 to 9aed706 Compare April 8, 2026 03:14
@fru1tworld
Copy link
Copy Markdown
Contributor Author

fru1tworld commented Apr 8, 2026

Updated to handle the additional cases:

  • let fut = &async_fn(); fut.await → suggest removing & from init
  • async fn bar(fut: &impl Future) → suggest removing & from param type (traces through async fn desugaring)
  • &dyn Future excluded (unsized)
  • Fallback help when specific span suggestion is unavailable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chenyukang chenyukang chenyukang left review comments

Assignees

@chenyukang chenyukang

Labels

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.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Confusing error: Immutable reference to future is not a future

5 participants

@fru1tworld @rustbot @rust-log-analyzer @chenyukang @JohnTitor