Replace macro approach with IndirectSignalReceiver to force type inference to work when connecting closures to the signals

[Yarwin]

What does this PR solve?

Allows us to use FnTraits for signal connections instead of generating signature for every single possible function. This approach makes creating signal extensions and whatnot much easier (since we can work with FnTrait instead of doing hoops to change Fn signature into ParamTuple and back again).

This PR does not introduce any changes in user-facing API (unless we missed something in itest 😅)

WHAT ????

Alright, let's start from scratch.

Crab is behaving very silly when it comes to type inference for closures, especially when Fn Traits are anyhow involved – which is ultra annoying and forces users to sprinkle their code with unhealthy amount of noise. To make it worse, type inference is, on first glance, inconsistent – qualified calls in qualified context are fine (ok, in layman terms – doing Trait stuff, like display and whatnot), while invoking methods on individual types doesn't work.

trait Bar {}

impl<F> Bar for F
where 
    F: FnMut(i32) -> i32
{
    
}

fn baz(x: impl Bar) {
    
}


fn main() {
    // + is syntax sugar for std::ops::Add::add(...) so there is a qualified call in qualified context…
    baz(|i| { i + 1 });

    // BUT this won't compile!
    // error[E0282]: type annotations needed
    // let closure = baz(|i| { i.abs() });
    //                    ^    - type must be known at this point

    // Changing it to i32::abs(i) makes it working again.
    baz(|i| { i32::abs(i) });
}

See: rust-lang/rust#63702.

Part of this weirdness can be addressed by using so-called "identity function". It is well known and established hack, mentioned, for example, here: https://users.rust-lang.org/t/type-inference-in-closures/78399.

trait Bar {}

impl<F> Bar for F
where 
    F: FnMut(i32) -> i32
{
    
}

fn identity_function(i: impl FnMut(i32) -> i32) -> impl Bar {
    i
}

fn baz(x: impl Bar) {
    
}


fn main() {
    baz(|i| { i + 1 });

    // Compiles again
    baz(identity_function(|i| { i.abs() }));
}

Why does it work? Well, the answer can be found in this apologetic comment in the compiler:

https://github.com/rust-lang/rust/blob/5ad7454f7503b6af2800bf4a7c875962cb03f913/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs#L306-L317

        // Check the arguments.
        // We do this in a pretty awful way: first we type-check any arguments
        // that are not closures, then we type-check the closures. This is so
        // that we have more information about the types of arguments when we
        // type-check the functions. This isn't really the right way to do this.
        for check_closures in [false, true] {
            // More awful hacks: before we check argument types, try to do
            // an "opportunistic" trait resolution of any trait bounds on
            // the call. This helps coercions.
            if check_closures {
                self.select_obligations_where_possible(|_| {})
            }

Don't worry if you haven't understand anything, we are in the same boat. Long story short, crab behaves ultra silly – if you specify concrete function type (FnMut…), then it is all good, if you specify trait (impl Bar) then crab gets lost and needs guidance.

In our case – specifying callbacks in form of closures for signals – identity function can't be just applied directly. We could make identity function work, doing something along the lines of signal().connect(infer_plspls!(|this| this.add_node(…))) which is equally annoying, creates unnecessary overhead, doesn't work nicely with most IDEs and also comes with its own tradeoffs.

For now, we went with @Houtamelo workaround which was using macro magic to handle all the possible callback types – but it comes with its own trade offs, mainly not being able to easily extend available methods, signal connections and whatnot. It is pretty good workaround, aye, but perfect is the enemy of good!

See also: #1152 (comment)

back on track – crab needs concrete type to infer types in closure. We need some identity function, or type, which would change our closure into trait stuff, right?
If we would work on function pointers then we could do something like Box<dyn FnMut(..)> -> Box<dyn Bar> conversion… But we can't really do Fn(???) -> impl Bar. Or can we? Let's make a step back, to get a better view… Like a view function? Yeah, view function sounds good, let's call it Intermediary.

Let's make concrete type from our Intermediary. Something simple and elegant, such as <&mut Func as Into<Intermediary<'c, Func, Params, Ret>>>. This looks like beautiful concrete type that makes our crab happy!

I mean, really

use std::marker::PhantomData;

trait Bar<Params, Ret> {
    fn call(&mut self, c: Params) -> Ret;
}

impl<F, Params, Ret> Bar<Params, Ret> for F
where
    F: FnMut(Params) -> Ret + 'static,
{
    fn call(&mut self, c: Params) -> Ret {
        self(c)
    }
}

fn baz<Func, Params, Ret>(mut x: Func, p: Params)
where
    Func: Bar<Params, Ret> + 'static,
    for<'c> Intermediary<'c, Func, Params, Ret>: From<&'c mut Func>,
{
    // Yes, <_, _, _> is required. Don't ask too many questions, we are in sillness domain. 
    <&mut Func as Into<Intermediary<_, _, _>>>::into(&mut x)
        .inner
        .call(p);
}

fn main() {
    baz(|i| i + 1, 4);

    // Compiles again
    baz(
        |i| {
            println!("henlo :)");
            let x = i.abs();
            x
        },
        -33i32,
    );
}

struct Intermediary<'c, Func, Params, Ret>
where
    Func: Bar<Params, Ret> + 'static,
{
    inner: &'c mut Func,
    params: PhantomData<Params>,
    ret: PhantomData<Ret>,
}

impl<'c, Func, Params, Ret> From<&'c mut Func> for Intermediary<'c, Func, Params, Ret>
where
    Func: FnMut(Params) -> Ret + Bar<Params, Ret> + 'static,
{
    fn from(value: &'c mut Func) -> Self {
        Intermediary {
            inner: value,
            params: PhantomData,
            ret: PhantomData,
        }
    }
}

it's all so silly…
I wouldn't call this hack elegant, but hey, it works!

Hopefully, the new trait solver will render this whole issue null: https://rustc-dev-guide.rust-lang.org/solve/significant-changes.html#deferred-alias-equality. If next-gen trait solver fixes this issue, we can just get rid of Intermediary, go back to function trait and call it a day (Intermediary is just a view function, so it will be no-brainer).

[GodotRust]

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1179

[Bromeon]

Thanks a lot! This is incredible work 💪
I already gave up hope on a generic approach, before Mr. Traitbender appeared 🧐

Could you set up the commits so they're ready for merge? Can be one or more, however you like (but maybe distinct messages if multiple). Please remove my authorship, I didn't really contribute much here 😅