search

rustasync / runtime

Empowering everyone to build asynchronous software
https://docs.rs/runtime
Apache License 2.0
862 stars 28 forks source link

improved error message for unused futures #32

Closed ibaryshnikov closed 5 years ago

ibaryshnikov commented 5 years ago

Follows https://github.com/rustasync/runtime/pull/31#issuecomment-492593832 I guess this variant was merged in rust-lang/rust. What about message for streams?

yoshuawuyts commented 5 years ago

@ibaryshnikov the same message can be used for streams

#[must_use = "streams do nothing unless you `.await` or poll them"]
taiki-e commented 5 years ago

@yoshuawuyts

#[must_use = "streams do nothing unless you `.await` or poll them"]

I think the message may be misleading, as streams cannot use .await directly.

Related: https://github.com/rust-lang-nursery/futures-rs/pull/1600

yoshuawuyts commented 5 years ago

@taiki-e ah yeah, fair -- I guess the way we want to go is for x.await in y {} loops, but not sure how to mention that. "polled" is probably accurate (poll_next), but await... is currently only sort of accurate? Do you have a suggestion for something better?

taiki-e commented 5 years ago

@yoshuawuyts I don't know what the more desirable error message is. For example, .next().await may be more convenient, but this may not be appropriate because next is not a method of Stream trait. Also, we use must_use for impl Sink types, so I don't know if the message related to a specific method name is really good. This also depends on how Stream trait is added in the language, so, for now, I think it is better to postpone the stream must_use message problem.

taiki-e commented 5 years ago

By the way, "improwed" in the commit message and the title looks like a typo.

yoshuawuyts commented 5 years ago

for now, I think it is better to postpone the stream must_use message problem.

@taiki-e that sounds reasonable (: