Make BoxFuture and LocalBoxFuture a concrete type with #[must_use]

[andll]

Currently BoxFuture is type alias for Pin<Box<...>>, so it can not have #[must_wait]. That leads to accidental mistakes, when user does not call await/poll on result of function that returns BoxFuture. We had multiple problems with that

Some notes on PR:

• I understand that for some projects this might be breaking change - if someone have hardcoded Pin<Box<...>> as a type of variable returned by .boxed, they would have to change it BoxedFuture once this diff lands, but I believe this is reasonable trade off.

• I am not sure what to do with Debug implementation for BoxFuture - ok to just suppress warning for this type?

• I don't fully understand what UnsafeFutureObj is, I mostly copied implementation from Pin<Box<...>>, but please take a look at it carefully to make sure I did not miss something.

[Nemo157]

This seems like a big limitation in the #[must_use] for traits support, that should somehow be inherited through Box, I left a comment on the implementing issue.

[andll]

Note that this is Pin<Box<..>>, not just Box<...>
I think (confirming with author of commit), rust-lang/rust#55506 only handles Box<..>, so it won't help our case

[andll]

So my feeling that rust-lang/rust#62262 could take a while (and might not even happen, some people voice concerns about propagating must_use through structs).

What are your thoughts on merging this diff to solve the problem of unused boxed futures? I think pretty much all other future wrappers (AndThen, etc) already are concrete types, is there reason not to convert BoxFuture?

[andll]

@Nemo157 what do you think? Would it make sense to merge this?

[Nemo157]

I'm on the fence with this, it makes sense if we're not going to have must_use working on the alias soon, but it is definitely going to be annoying dealing with a concrete type in some places.

I think one thing that would help is to add

impl<'a, T: Future + 'a> BoxFuture<'a, T> {
    pub fn pin(x: T) -> Self {
        Self::new(Box::pin(x))
    }
}

that way code that is currently

Box::pin(async move { ... })

can transition to

BoxFuture::pin(async { ... })

(using .boxed() along with async { ... } results in some pretty bad formatting).

@cramertj you've probably got a better idea how badly moving this to a concrete type would affect large codebases.

[cramertj]

I'm unsure about this. It does seem unfortunate that #[must_use] doesn't propagate correctly, but once we have that fixed (and I'm assuming we will fix it at some point), we would want to shift it back to a type alias. It's easy to imagine breaking existing uses of this type by shifting it back into a type alias-- there are some things we could do to try and make the APIs intercompatible like using BoxFuture::pin rather than BoxFuture::new etc., but I'm feeling pretty uneasy about it.

[andll]

@cramertj for my education - why type alias is more preferred then concrete type, e.g. why would we want to shift back to alias?
As far as I understand, there is no allocation/performance overhead for having concrete types like this

[cramertj]

Primarily that it's a simpler mental model for users-- it's more transparent, and easy to see that it's "just" a boxed trait object rather than something special.

[taiki-e]

Even if this PR is merged, considering that must_use does not work with Pin<&mut impl Future>, I think that the advantage of merging this PR is small (but thanks for finding this issue @andll).

[cramertj]

I agree with @taiki-e. I'm going to go ahead and close this PR-- I don't think this is something we want to do.Thanks for the PR, though-- I appreciate you calling attention to this.