Inconsistency in trait item resolution between `Box<Trait>` and `impl Trait`

[petrochenkov]

Consider this example:

#![feature(conservative_impl_trait)]

// use m::Tr;

mod m {
    pub trait Tr {
        fn method(&self) {}
    }
    impl Tr for u8 {}
    
    pub fn dynamic_tr() -> Box<Tr> { Box::new(0) }
    pub fn static_tr() -> impl Tr { 0u8 }
}

fn main() {
    m::dynamic_tr().method(); // OK
    m::static_tr().method(); // ERROR: no method named `method` found for type `impl m::Tr` in the current scope
}

Here we are trying to call methods of traits that are not in scope.
Typically such methods are not resolved, but there's an exception - trait objects.
Trait objects are magic - when you call a method of Trait on a value of type Trait then Trait is automatically considered in scope (or maybe this method is treated as inherent, I haven't looked how exactly the implementation works). This is the reason why m::dynamic_tr().method() works. Even if this is a special case, it's a reasonable special case - if you have a value of type Trait in hands, you typically want to call some trait methods on it and it would be a nuisance to require Trait imported in scope for this.
I don't know what RFC or PR introduced these exact rules, but the fact is that they are used in practice.

All the logic above is applicable to impl Trait, but this special case doesn't apply to it and m::static_tr().method() reports an "unresolved method" error.

[hniksic]

As documented in #54610, I noticed this while removing boxed returns from the subprocess crate, where I had to update tests after the change of return type from Box<Write> to impl Write. The code in tests was of the kind that one would have expected to keep working; something like:

let mut stream = ...;
stream.write_all(b"foo").unwrap();

Note that the return type was never named - and yet the change of stream from Box<Write> to impl Write requires the caller to use std::io::Write which was not required before.

[SNCPlay42]

Adding to the inconsistency, this only affects existential impl trait, not universal impl trait (or the mostly-equivalent trait bound):

mod not_in_scope {
    pub trait Foo {
        fn foo(&self);
    }
    
    impl Foo for () {
        fn foo(&self) {}
    }
}

fn existential_impl_trait() {
    fn f() -> impl not_in_scope::Foo {
        ()
    }
    
    f().foo();
}

fn universal_impl_trait(x: impl not_in_scope::Foo) {
    x.foo();
}

fn dyn_trait(x: &dyn not_in_scope::Foo) {
    x.foo();
}

fn trait_bound<T: not_in_scope::Foo>(x: T) {
    x.foo();
}

pub fn main() {
    existential_impl_trait();
    dyn_trait(&());
    universal_impl_trait(());
    trait_bound(());
}

Errors on existential_impl_trait (playground), but if existential_impl_trait and the call to it is commented out, the code compiles and runs without any errors from the other functions.

[LunaBorowska]

I would argue that not having to import a trait into scope for Box<dyn Tr> is a bug, however because of backwards compatibility it's here to stay - it's relatively harmless in this case and programs probably already depend on it.

I think extending it to existential impl Tr would be a big mistake as that would mean that replacing impl Tr with a concrete type could be a breaking change. Consider replacing impl Tr with u8 in your example - if existential impl Tr were to have inherent methods then changing the type to u8 would cause breakage.

[Starwort]

I think extending it to existential impl Tr would be a big mistake as that would mean that replacing impl Tr with a concrete type could be a breaking change.

I feel like that's a more-intuitive (and less-common) breakage to have though; Box<dyn Trait> and impl Trait both convey the same semantic meaning; 'the thing I'm giving you/taking needs to have these abilities, and it doesn't really matter what the specific type is' - it seems intuitive that an impl Trait has Trait's methods as inherent, as giving an opaque type limits what you can do with it to only the methods of the trait specified; if I provide a concrete type that happens to implement that trait (e.g. u8) the semantic meaning is closer to 'the thing I'm giving you/taking matters what the specific type is, and the type I chose has these abilities'

Returning impl Trait gives the API user an idea of what they're intended to do with the returned value, and by extension an idea of the purpose of the function (although to be fair naming and a doc comment is better for this).

Additionally, unlike moving from Box<dyn Trait> to impl Trait, impl Trait to doesn't actually change the mechanical meaning of the code in any way; it doesn't give the compiler any extra information and (IMO) serves only to confuse the user of the API.

I hope I communicated my thoughts clearly, please let me know if there's anything you'd like me to clear up. Hopefully I didn't come off as harsh ^^'

[LunaBorowska]

Perhaps I should make my old argument more clear.

If a public function function declares a function that returns Box<dyn Trait> then it cannot change that type without semver breakage. Users of a library could depend on that after all, there is a possibility that they stored return value from the library in a variable of Box<dyn Trait> type.

However, if a public function returns impl Trait then it should be possible to change that type into something more concrete without breaking the users.

For instance, imagine a crate that declares the following function.

pub fn do_stuff() -> impl std::future::Future<Output = ()> {
    todo!()
}

It should be possible to change impl std::future::Future<Output = ()> into a concrete type without breaking backwards compatibility.

pub struct DoStuff(());

impl std::future::Future for DoStuff {
    type Output = ();
    fn poll(self: std::pin::Pin<&mut Self>, cx: &mut std::task::Context<'_>) -> std::task::Poll<()> {
        todo!()
    }
}

pub fn do_stuff() -> DoStuff {
    DoStuff(())
}

However, if impl Trait would allow to access Trait methods without importing the trait then it would allow users to not import the trait. The problem with that is that if crate users did wrote code like pin!(do_stuff()).poll(...) without importing std::future::Future then their programs would break after that change making it a semver hazard.

[Starwort]

Yes, that does make more sense when applied to specifically future; that said, I'm still not quite seeing the reason why anyone would change impl Trait to a specific T without meaning the associated semantic change with it; especially since using impl Trait means you can change the internals without changing the signature (and therefore not a semver hazard AFAIK).

Your future example definitely moves me closer to an undecided stance, but honestly I think treating trait methods as inherent would make sense (if I return impl io::Write, why should you need to import the trait in order to do anything with the value I returned?) IMO changing the semantic meaning of a function signature should be as much of a semver hazard as changing its types, but I respect that others may not agree