Suggest removing & when awaiting a reference to a future#154933
Suggest removing & when awaiting a reference to a future#154933rust-bors[bot] merged 1 commit intorust-lang:mainfrom
& when awaiting a reference to a future#154933Conversation
f36f2b9 to
5afb519
Compare
|
r? @JohnTitor rustbot has assigned @JohnTitor. Use Why was this reviewer chosen?The reviewer was selected based on:
|
bff3e3b to
cdd3efa
Compare
bc9d978 to
7e22d2d
Compare
This comment has been minimized.
This comment has been minimized.
7e22d2d to
fe623c3
Compare
| error[E0277]: `&impl Future<Output = ()>` is not a future | ||
| --> $DIR/await-ref-future.rs:9:9 | ||
| | | ||
| LL | fut.await; |
There was a problem hiding this comment.
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)
|
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
|
fe623c3 to
9aed706
Compare
|
Updated to handle the additional cases:
|
| // In async fn, params are desugared: `let fut = __arg0;` | ||
| // with `LocalSource::AsyncFn`. Follow init back to the | ||
| // original parameter. | ||
| hir::Node::LetStmt(local) |
There was a problem hiding this comment.
This is still a fairly ad hoc reimplementation of borrow-removal logic, and it misses cases that the more general machinery should handle. For example, let fut = &&my_async_fn(); fut.await; still falls back to help: remove the .await. I think the better direction is to extract the borrow-peeling/span-finding part of suggest_remove_reference above and reuse it here, instead of open special handling for cases such as AddrOf / LetStmt / AsyncFn cases again.
9aed706 to
1323164
Compare
| } | ||
|
|
||
| // Try removing `&` from the parameter type. | ||
| if let Some(param) = terminal_param { |
There was a problem hiding this comment.
thanks, this version sees better.
this test should not suggest removing from parameter?
async fn ref_param_borrowed_expr(fut: &impl std::future::Future<Output = ()>) {
(&fut).await; //~ ERROR is not a future
}1323164 to
e9856d7
Compare
|
Fixed — guarded param |
|
Thanks! @bors r=chenyukang |
Rollup of 5 pull requests Successful merges: - #153411 (Offload slice support) - #154557 (Make E0284 generic argument suggestions more explicit) - #154933 (Suggest removing `&` when awaiting a reference to a future) - #155524 (Fix LLVM offload install docs to use semicolon-separated CMake lists) - #155568 (Update books)
Rollup of 5 pull requests Successful merges: - rust-lang/rust#153411 (Offload slice support) - rust-lang/rust#154557 (Make E0284 generic argument suggestions more explicit) - rust-lang/rust#154933 (Suggest removing `&` when awaiting a reference to a future) - rust-lang/rust#155524 (Fix LLVM offload install docs to use semicolon-separated CMake lists) - rust-lang/rust#155568 (Update books)
Fixes #87211
When
.awaiting&impl Future, suggest removing the&instead of removing.await.