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

Rules around call resolution on fn_traits are confusing #120022

Open
Kolsky opened this issue Jan 16, 2024 · 2 comments
Open

Rules around call resolution on fn_traits are confusing #120022

Kolsky opened this issue Jan 16, 2024 · 2 comments
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. F-unboxed_closures `#![feature(unboxed_closures)]` requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Kolsky
Copy link

Kolsky commented Jan 16, 2024

I tried this code:

#![feature(fn_traits, unboxed_closures)]

pub struct Foo<F>(F); // a simple newtype over closure object, nothing more

impl<F: FnOnce(A) -> B, A, B> FnOnce<(A,)> for Foo<F> {
    type Output = B;

    extern "rust-call" fn call_once(self, (a,): (A,)) -> B {
        (self.0)(a)
    }
}

impl<F: FnMut(A) -> B, A, B> FnMut<(A,)> for Foo<F> {
    extern "rust-call" fn call_mut(&mut self, (a,): (A,)) -> B {
        (&mut self.0)(a)
    }
}

impl<F: Fn(A) -> B, A, B> Fn<(A,)> for Foo<F> {
    extern "rust-call" fn call(&self, (a,): (A,)) -> B {
        (&self.0)(a)
    }
}

fn main() {
    let mut s = String::new();
    
    // This works
    {
        let mut closure = |()| s.push_str("");
        (move |x| closure(x))(());
    }
    
    // This doesn't!
    {
        let closure = |()| s.push_str("");
        (Foo(closure))(());
    }
}

(playground)

I expected to see this happen: FnMut gets picked up as the next candidate fitting for closure call, code compiles succesfully.

Instead, this happened: the closest candidate in method resolution should be Self, if anything, but, evidently, &Self is preferred instead. It would be justifiable for closures specifically if the code could be compiled. It doesn't seem to align with anything known so far about either closures or method call expressions. Removing Fn impl makes the code compile successfully, but it shouldn't be considered at all due to unsatisfied F: Fn bound.

Meta

rustc version: 1.76.0 nightly

Main tracking issue: #29625

@Kolsky Kolsky added the C-bug Category: This is a bug. label Jan 16, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 16, 2024
@fmease fmease added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. F-unboxed_closures `#![feature(unboxed_closures)]` labels Jan 16, 2024
@compiler-errors
Copy link
Member

The only reason this works for closures is because we defer closure call resolution to the end of type checking, and we don't apply autoref or autoderef on closure calls until we've determined the closure's captures and kind.

It doesn't seem to align with anything known so far about either closures or method call expressions.

Given that the Fn traits are currently unstable -- and not really in shape for friendly user-facing usage yet, to be honest -- we don't really guarantee anything about their behavior except for what you can witness on stable code.

If you do want to understand how these things work under the hood, my best recommendation is to just read the compiler code. You can look for CallStep::DeferredClosure and deferred_call_resolutions to find some of the machinery that allows closures to select the right Fn-family trait without yet knowing what it implements.

@compiler-errors compiler-errors removed the C-bug Category: This is a bug. label Jan 16, 2024
@fmease fmease added C-discussion Category: Discussion or questions that doesn't represent real issues. requires-nightly This issue requires a nightly compiler in some way. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jan 16, 2024
@ijackson
Copy link
Contributor

ijackson commented Jan 23, 2024

I don't think it's all that bad :-). It's true that there's some magic about automatic selection of the right trait for closures. But. in many realistic scenarios, you're passing the implementor of the Fn traits to some other function that knows what it needs

Eg, this works:

fn call_fnmut<A>(mut cl: impl FnMut(A), a: A) {
    cl(a);
}
    {
        let mut closure = |()| s.push_str("");
        call_fnmut(move |x| closure(x), ());
    }
    {
        let closure = |()| s.push_str("");
        call_fnmut(Foo(closure), ());
    }

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=40754397ad9fce96cbf1c52528e1231d

Likewise if the newtype knows which fn traits it implements. https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=3058e46a99343243aaa8eef01b3351fd

The problems seem to come if you want the closure trait selection (type inference) to pass through generics on your newtype. Or to put it another way, the generics on the newtype act as a barrier for fn trait selection - an inference barrier between the definition of an actual closure, and place where the newtype is called.

(I think this is no more than a "Rust can't compile your program yet" problem, assuming you don't mind that Rust sometimes picks Fn when FnMut or FnOnce would have been possible too.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. F-unboxed_closures `#![feature(unboxed_closures)]` requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants