feat(ecmascript): Callable

[yossydev]

I have created Callable as a new implementation. I copied the contents of function.rs and added Proxy to each function.

Now, I’m thinking of rewriting is_callable as follows:

pub(crate) fn is_callable<'a, 'b>(
    argument: impl TryInto<Function<'b>>,
    gc: NoGcScope<'a, '_>,
) -> Option<Callable<'a>> {
    // 1. If argument is not an Object, return false.
    let Ok(argument) = argument.try_into() else {
        return None;
    };
    // 2. If argument has a [[Call]] internal method, return true.
    is_callable(argument, gc)
}

However, if I do this, it will cause compilation errors in all the places where is_callable is currently being used. Since the existing functions mostly accept Function, this behavior seems natural.

At this stage, does this implementation approach for Callable align with the intended design?

[aapoalas]

This is exactly what I was thinking, yes.

... But now that I see it and read the spec a little more, it might be that we should just rename Function to Callable and add Proxy there. The spec says this about callable objects:

an object is a member of the built-in type Object; and a function is a callable object.

and

In addition, some objects are callable; these are referred to as functions or function objects

ie. The spec is saying that "whenever I say 'function', what I mean is any kind of actual function or a callable Proxy". And this is validated by how the spec is written. So the Function enum doesn't actually correspond to anything in the spec... It does make sense from a human standpoint but it might not make actual sense from the code standpoint...

is_callable definitely should return Callable, and basically every place where we have Function as a parameter or saved in a struct should indeed have Callable... The only question is if there will remain any places whatsoever where the Function enum would remain at all?

[aapoalas]

nitpick: Should be Proxy({:?).

[yossydev]

thank you! fixed to 2e8bba3

[aapoalas]

nitpick: This should be unreachable as well.

[yossydev]

thank you! return None 91f9ae7

[aapoalas]

Just call unreachable!()

[yossydev]

Got it, I’ve made the changes! d491dbb

[yossydev]

… But now that I see it and read the spec a little more, it might be that we should just rename Function to Callable and add Proxy there. The spec says this about callable objects:

Yeah, I was also wondering if that might be the right approach while trying to refactor this. I was a bit unsure about when we would actually need to treat Function and Callable separately.

Should we go ahead and rewrite function.rs as callable.rs, or should we merge this PR and gradually replace the places using Function with Callable? If we decide to go with the former, I think we can go ahead and close this PR. What do you think? 👀

[aapoalas]

… But now that I see it and read the spec a little more, it might be that we should just rename Function to Callable and add Proxy there. The spec says this about callable objects:

Yeah, I was also wondering if that might be the right approach while trying to refactor this. I was a bit unsure about when we would actually need to treat Function and Callable separately.

Should we go ahead and rewrite function.rs as callable.rs, or should we merge this PR and gradually replace the places using Function with Callable? If we decide to go with the former, I think we can go ahead and close this PR. What do you think? 👀

I'm a bit unsure. If we add Proxy to Function and rename it to Callable, then we'll get basically all we want the easy way. But! It might still be that there's a place somewhere where the spec does actually rule out callable Proxies, and if that is the case then we'll have removed the enum that would've been used to represent that. The worst thing is that we won't see that case in our code if we just rename Function to Callable.

But, if we do go ahead with a separate Callable then we're going to do a lot of work that might end up being entirely unnecessary :/

[aapoalas]

A quick search through the spec suggests that Proxies are never categorically separated from callable objects, so a closing this PR and just renaming Function to Callable seems proper.