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

.await does not perform autoref or autoderef #111546

Open
Jules-Bertholet opened this issue May 13, 2023 · 13 comments
Open

.await does not perform autoref or autoderef #111546

Jules-Bertholet opened this issue May 13, 2023 · 13 comments
Labels
A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue. WG-async Working group: Async & await

Comments

@Jules-Bertholet
Copy link
Contributor

Jules-Bertholet commented May 13, 2023

I tried this code:

use core::future::IntoFuture;
use futures::future::{ready, Ready};
struct Foo;

impl IntoFuture for &Foo {
    type Output = u32;
    type IntoFuture = Ready<u32>;

    fn into_future(self) -> Self::IntoFuture {
        ready(2)
    }
}

#[tokio::main]
async fn main() {
    assert_eq!(Foo.await, 2);
}

I expected to see this happen: Compiles and runs successfully. The .await performs auto-ref, and <&Foo as IntoFuture>::into_future() is called.

Instead, this happened:

error[E0277]: `Foo` is not a future
  --> src/main.rs:16:19
   |
16 |     assert_eq!(Foo.await, 2);
   |                   ^^^^^^
   |                   |
   |                   `Foo` is not a future
   |                   help: remove the `.await`
   |
   = help: the trait `futures::Future` is not implemented for `Foo`
   = note: Foo must be a future or must implement `IntoFuture` to be awaited
   = help: the trait `std::future::IntoFuture` is implemented for `&Foo`
   = note: required for `Foo` to implement `std::future::IntoFuture`

Meta

rustc --version --verbose:

1.71.0-nightly (2023-05-12 4a59ba4d54a3ec0d8ea1)

@rustbot label A-async-await

@Jules-Bertholet Jules-Bertholet added the C-bug Category: This is a bug. label May 13, 2023
@rustbot rustbot added the A-async-await Area: Async & Await label May 13, 2023
@compiler-errors
Copy link
Member

Is there a reason why you want this behavior? Just for consistency? Or do you have real code that relies on it?

@Jules-Bertholet
Copy link
Contributor Author

Jules-Bertholet commented May 13, 2023

This came up for me with the IntoFuture impls for async_once_cell::Lazy. Autoref would make using that type significantly more ergonomic.

@Jules-Bertholet Jules-Bertholet changed the title .await does not perform autoref or auto-deref .await does not perform autoref or autoderef May 14, 2023
@clarfonthey
Copy link
Contributor

clarfonthey commented May 19, 2023

Weird question: was this present before the move to IntoFuture? i.e., would this have still failed if &T implemented Future and the value offered was type T?

I'm assuming not, because otherwise this could have potentially broken things that worked before.

@compiler-errors
Copy link
Member

@clarfonthey no, afaict it never did autoderef

@eholk
Copy link
Contributor

eholk commented May 22, 2023

We discussed this in @rust-lang/wg-async's triage meeting today.

Overall, we'd like to see more motivation for this behavior, especially since on the PR (#111773) it looks like this change could break existing code.

As I understand it, this would let async_once_cell do shared.await instead of shared.as_ref().await. Am I understanding that correctly? Could you give some more examples of what would be possible with this change?

There could be an argument for consistency here, if user's would find it surprising that .await doesn't autoref. As far as I know this is the first case where this has been surprising, but I could easily believe we are missing a lot of cases.

@Jules-Bertholet
Copy link
Contributor Author

Jules-Bertholet commented May 22, 2023

As I understand it, this would let async_once_cell do shared.await instead of shared.as_ref().await.

Basically, yes. Which doesn't sound huge on its own—but let's compare with the sync version of Lazy.

//! Sync version
use once_cell::sync::Lazy; // or `std::sync::LazyLock`

static USER_IDS: Lazy<HashMap<String, u32>> = Lazy::new(|| /* sync initialization code */);

fn johns_id() -> Option<u32> {
    // No `await` and no method call. `Lazy`'s `Deref` impl allows accessing the value directly.
    USER_IDS.get("John Doe").copied()
}
//! Async version
use async_lazy::Lazy; // or `async_once_cell::Lazy`

static USER_IDS: Lazy<HashMap<String, u32>> = Lazy::const_new(|| Box::pin(async {/* async initialization code */}));

fn johns_id_no_autoref() -> Option<u32> {
    // `.await` and extra method call. +14 characters compared to sync version of this code
    USER_IDS.force().await.get("John Doe").copied()
}

fn johns_id_with_autoref() -> Option<u32> {
    // `.await`, but no extra method call. Much closer to sync version, +6 characters
    USER_IDS.await.get("John Doe").copied()
}

There could be an argument for consistency here, if user's would find it surprising that .await doesn't autoref.

Personally, I had always thought that ". means auto(de)ref", and was surprised to learn otherwise. I think this hasn't come up in the past because IntoFuture is not widely implemented, especially on references; without #![feature(impl_trait_in_assoc_type)], it's hard to write the impls. But that feature should stabilize soon.

@tmandry
Copy link
Member

tmandry commented Aug 29, 2023

We discussed this in a recent wg-async meeting (notes). The consensus was that we thought the change was well-motivated. At the same time, we want to be cautious about introducing problems (namely backwards compatibility).

There should probably be a crater run of this change, and we should also work through any problematic interactions that could be caused by this change. (@rust-lang/types should probably weigh in.)

The main motivation for the change is the analogy to .method(), as well as to wanting async and sync to feel similarly convenient in most cases.

Note that there is another analogy that works against this, the analogy to IntoIterator, where the lang-effect form (for _ in foo {}) does not do autoref/autoderef. However, given that this looks very different from foo.await, and taking a reference with that form is significantly more convenient (for x in &foo or for x in foo.iter() vs (&foo).await), it seemed the analogy was stretched pretty thin. So we elected to put more weight on the above two considerations.

That being said, this change would need lang team signoff. You can consider this comment wg-async's official recommendation to the lang team.

@eholk eholk added the AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. label Sep 25, 2023
@compiler-errors compiler-errors added T-lang Relevant to the language team, which will review and decide on the PR/issue. I-lang-nominated Nominated for discussion during a lang team meeting. labels Sep 25, 2023
@tmandry tmandry added I-types-nominated Nominated for discussion during a types team meeting. T-types Relevant to the types team, which will review and decide on the PR/issue. labels Sep 25, 2023
@traviscross
Copy link
Contributor

@rustbot labels -I-types-nominated

This was discussed at a recent T-types meeting. The consensus was that this is a question for T-lang. If T-lang has a specific question for T-types, please renominate.

@rustbot rustbot removed the I-types-nominated Nominated for discussion during a types team meeting. label Nov 6, 2023
@estebank
Copy link
Contributor

estebank commented Nov 6, 2023

In the meantime we could change the error to check for the same steps that autoderef would follow and suggest the right combination of operations (suggest (&Foo).await here).

@WaffleLapkin
Copy link
Member

While I sympathize with the example of async Lazy, I don't think this is a good solution. Or, rather, that seems like a too radical approach, for a small problem. I would not expect .await to auto ref (the same as with for loop) and I don't think we need to complicate .await with that.

What I would rather see is postfix address of operator, which would let you take the reference without adding parenthesis: lazy.&.await. Compiler then can suggest adding it, as @estebank suggested above.

(I'm a bit biased here cause I want postfix addr-of anyway, but making .await do autoref really does not sound like a good idea to me...)

@estebank estebank added A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` labels Nov 7, 2023
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 11, 2023
Perform autoref/autoderef on `.await`

This PR adds support for autoref/autoderef of the reciever of an `.await` (before calling `IntoFuture::into_future()`).

This PR is not ready to merge yet—the feature works, but diagnostics are regressed and clippy is broken. I would like to get some feedback on my current approach, before investing more effort into it.

Fixes rust-lang#111546.
@tmandry
Copy link
Member

tmandry commented Apr 4, 2024

Given that there's been a crater run and we haven't looked through the results yet, unnominating until @rust-lang/wg-async has time to do so.

@rustbot label: -I-lang-nominated

@rustbot rustbot removed the I-lang-nominated Nominated for discussion during a lang team meeting. label Apr 4, 2024
@traviscross traviscross added the WG-async Working group: Async & await label Apr 4, 2024
@tmandry
Copy link
Member

tmandry commented Apr 4, 2024

I'd like to use this issue to collect more use cases in the meantime. Right now this seems like a risky change to make, when so far we only know of one instance where it would be useful.

@clarfonthey
Copy link
Contributor

I definitely think that this should be something that happens, even if it has to happen in a future edition. (I'd imagine it's probably too late to nudge into the 2024 edition, given the amount of work required and the short deadline.)

I also believe that . should just always do auto-deref, and that includes for cases like .await. Probably the only future exception might be postfix macros (I wouldn't expect x.macro!() to auto-deref since it's a syntactical thing, and it wouldn't know what to deref to in general) but I don't think there's any existing cases where it doesn't auto-deref.

.& might itself be useful in combination with .* and postfix macros though. Perhaps .* and .& could be paired together in an RFC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue. WG-async Working group: Async & await
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants