[builtins] Builtin function doesnt have constructor property

[croraf]

To Reproduce
[].findIndex.constructor

Expected behavior
Should return a Function constructor. Instead undefined is returned.

Custom functions do have constructor property which returns correctly.

[54k1]

I think make_builtin_fn should take even global as parameter (apart from parent), just like make_constructor_fn. Then, we can set the constructor field as "Function" (fetched from global). Something like the following:

pub fn make_builtin_fn<N>(function: NativeFunctionData, name: N, parent: &Value, length: usize, global: &Value)
where
    N: Into<String> {
    ...
    let mut function = Object::function(Function::builtin(Vec::new(), function), Value::null());
    let constructor = Object::function(function, global.get_field("Function"));
    function.insert_property("constructor", Property::data_descriptor(constructor.into(),
        Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::PERMANENT));
    ...
}

@HalidOdat Any thoughts about this?

[HalidOdat]

I think make_builtin_fn should take even global as parameter (apart from parent), just like make_constructor_fn. Then, we can set the constructor field as "Function" (fetched from global).

Yes. this is good solution, maybe we should accept an Interpreter so we have more control and we can call Interpreter::global() which return the global object. Accepting Interpreter can make it easier to implement caching in the future for common objects like Object, Array, Function and there prototypes instead of doing it through .get_field.

@54k1 should I assign you to this issue?

[54k1]

Yes, that would be a good solution to take Interpreter as the parameter. Thanks for pointing it out.
I was however just checking the specification of Function instances. It does not mention anything about the "constructor" field. Is it something left to the implementation?

Sure, I can take this up.

[HalidOdat]

I was however just checking the specification of Function instances. It does not mention anything about the "constructor" field. Is it something left to the implementation?

Good catch! I should have looked at the spec first. What we are trying to do is not spec compliant. the "constructor" field is not a part of the function instance, it is from the Function prototype. So what we need to do is set the prototype (__proto__), not the owned property field "constructor". here there already is a FIXME comment on it, currently the function instance prototype is set to Value::null() this is wrong we should set it to Function.prototype

We don't have to worry about setting "constructor" it is already done it all make_constructor_fn here.

[].findIndex.constructor This works because properties are searched in the prototype chain.

[54k1]

[].findIndex.constructor This works because properties are searched in the prototype chain.

Thanks for pointing this out.