Boa: `Context::to_ordinary_primitive` panics when passed a function

[vgel]

Description
Trying to convert a function object (such as function test() {}) to a primitive (such as via the Number constructor) panics, because of a mismatch of assumptions between Value::to_primitive and Context::ordinary_to_primitive.

To Reproduce

$ RUST_BACKTRACE=1 cargo run --bin boa
    Finished dev [unoptimized + debuginfo] target(s) in 0.06s
     Running `target/debug/boa`
>> function test() {}
undefined
>> Number(test)
thread 'main' panicked at 'assertion failed: o.get_type() == Type::Object', boa/src/context.rs:379:9
stack backtrace:
# ....... snipped panic machinery ......
  11: std::panicking::begin_panic
             at /home/vogel/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/panicking.rs:456
  12: boa::context::Context::ordinary_to_primitive
             at boa/src/context.rs:379
  13: boa::value::Value::to_primitive
             at boa/src/value/mod.rs:600
  14: boa::value::Value::to_numeric_number
             at boa/src/value/mod.rs:914
  15: boa::builtins::number::Number::make_number
             at boa/src/builtins/number/mod.rs:134
  16: boa::object::gcobject::GcObject::call
             at boa/src/object/gcobject.rs:176
  17: boa::context::Context::call
             at boa/src/context.rs:141
  18: <boa::syntax::ast::node::expression::call::Call as boa::exec::Executable>::run
             at boa/src/syntax/ast/node/expression/call/mod.rs:95
  19: <boa::syntax::ast::node::Node as boa::exec::Executable>::run
             at boa/src/syntax/ast/node/mod.rs:262
  20: <boa::syntax::ast::node::statement_list::StatementList as boa::exec::Executable>::run
             at boa/src/syntax/ast/node/statement_list/mod.rs:70
  21: boa::context::Context::eval
             at boa/src/context.rs:499
  22: boa::main
             at boa_cli/src/main.rs:188
 # .... snipped rust boot ....
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Context
Value::to_primitive checks if its own variant is Object, and if so passes the value over to Context::ordinary_to_primitive:
https://github.com/boa-dev/boa/blob/3e60d2c55472053ff8f8d6fb7159ff69fb136167/boa/src/value/mod.rs#L595-L609

But Context::to_ordinary_primitive checks if get_type() returns Type::Object:
https://github.com/boa-dev/boa/blob/87d9e9cea82c1ea675082063f73296538bb3f46f/boa/src/context.rs#L377-L383

These two don't match up, because Value::get_type checks if the object is callable, and if so returns function:
https://github.com/boa-dev/boa/blob/59d328e67cbfb81692daadb174e09598cb1ea9ce/boa/src/value/type.rs#L49-L54

I believe the correct thing to do by the standard here is to treat functions the same as objects. https://tc39.es/ecma262/#sec-ordinarytoprimitive says Type of O should be Object, and in that section it only lists the Object type. Function as a type only comes up in https://tc39.es/ecma262/#sec-typeof-operator .

Build environment (please complete the following information):

• OS: Ubuntu
• Version: Linux marina 5.4.0-7634-generic #38~1592497129~20.04~9a1ea2e-Ubuntu SMP Fri Jun 19 22:43:37 UTC x86_64 x86_64 x86_64 GNU/Linux
• Target triple: x86_64-unknown-linux-gnu
• Rustc version: rustc 1.46.0 (04488afe3 2020-08-24)

[vgel]

Oh, also this doesn't panic in release mode -- it just does the wrong thing.

>> function test() {}
undefined
>> Number(test)
Uncaught: "TypeError": "cannot convert object to primitive value"

It's getting down here somehow: https://github.com/boa-dev/boa/blob/87d9e9cea82c1ea675082063f73296538bb3f46f/boa/src/context.rs#L412-L413

Which I'm not sure why... the function should have a toString key.

[RageKnify]

@Razican and @Lan2u you were the ones who took care of #435 / #441 , looking at the codebase I don't think Type::Function is actually used, and it seems like the spec just sees functions as objects. Do you see any reason against removing Type::Function?

An object can still be identified as a function cince its ObjectData variant will be Function.

[Razican]

@Razican and @Lan2u you were the ones who took care of #435 / #441 , looking at the codebase I don't think Type::Function is actually used, and it seems like the spec just sees functions as objects. Do you see any reason against removing Type::Function?

An object can still be identified as a function cince its ObjectData variant will be Function.

I think it can be removed, there is no such thing as a "function" type in JavaScript, they are callable objects.

[RageKnify]

I did just check in chrome, and:

function x() {}
typeof x

Does return "function", so we need to keep that functionality...

[HalidOdat]

Does return "function", so we need to keep that functionality...

can't we check if its an object with Value::is_object, this gives true for any object including function

[vgel]

Should be incidentally fixed by #777, as ordinary_to_primitive is now associated with GcObject and no longer takes a Value, so it doesn't check the type.

[HalidOdat]

@vgel this has been fixed, right?

[RageKnify]

Yes, I just tested on master, function t() {}; Number(t) return NaN, just like Chrome does.