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

TryFutureExt conflicts with futures #371

Open
bjchambers opened this issue Dec 16, 2022 · 2 comments
Open

TryFutureExt conflicts with futures #371

bjchambers opened this issue Dec 16, 2022 · 2 comments
Labels
found in the field A user of SNAFU found this when trying to use it help wanted Extra attention is needed

Comments

@bjchambers
Copy link

bjchambers commented Dec 16, 2022

I'd like to be able to use methods from both traits, but am unable to use both since they have the same name:

use futures::TryFutureExt;
use snafu::TryFutureExt;

It's possible to work around this:

use futures::TryFutureExt;
use snafu::TryFutureExt as SnafuTryFutureExt;

But this won't be suggested by rust-analyzer, etc. Ideally, it would be possible to use both simultaneously. It might be helpful to rename the trait in snafu (possibly providing an alias for backwards compatibility). Alternatively, I could just use await.context(...) (eg., use ResultExt instead of TryFutureExt), but that raises the question of why there needs to be a TryFutureExt at all.

@8573
Copy link

8573 commented Dec 18, 2022

(I'm just a user, not a maintainer!)

But this won't be suggested by rust-analyzer, etc.

This sounds to me like R-A's problem, not Snafu's. Using traits (or other items) that need to be renamed1 on import is something Rust users need to be prepared to do, so, if Rust users depend on R-A to do it for them, R-A needs to be prepared to do so.

Footnotes

  1. or de-named with use ... as _;

@shepmaster shepmaster changed the title snfau's TryFutureExt conflicts with futures TryFutureExt conflicts with futures Dec 19, 2022
@shepmaster
Copy link
Owner

I agree that this isn't really something that libraries should work around. I recommend using the _ import, and that's what the snafu::prelude. Ideally, all of the SNAFU docs will use this form (please let me know if you find any that don't!).

Also, we've raised an issue with rust-analyzer to suggest importing a prelude when present, which would help here.

Alternatively, I could just use await.context(...) (eg., use ResultExt instead of TryFutureExt), but that raises the question of why there needs to be a TryFutureExt at all.

Absolutely! SNAFU's future combinators were added a long while ago — I think before async and await were added to the language. When writing async code, you need to be careful of when you place your .awaits so that you don't lose concurrency. For example:

let a = a.await.context(...);
let b = b.await.context(...); // b will happen *after* a

// vs

let a = a.context(...);
let b = b.context(...);
let (a, b) = join!(a, b); // a and b will happen concurrently 

// or without SNAFU's combinators

let a = async { a.await.context(...) };
let b = async { b.await.context(...) };
let (a, b) = join!(a, b); // a and b will happen concurrently 

@shepmaster shepmaster added help wanted Extra attention is needed found in the field A user of SNAFU found this when trying to use it labels Dec 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
found in the field A user of SNAFU found this when trying to use it help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants