Implement Display for function objects

[kvnvelasco]

This Pull Request fixes/closes #361.

Hello!

This is my first attempt at a contribution here let me know if I'm being dumb. I decided to go ahead and stick the most naive implementation I could think of as a beginning of a discussion while I get to grips with the codebase. I also have questions.

Thanks!

It changes the following:

• Adds a special case in Value::to_string that handles function objects (for now)
• Adds a bunch of questions for implementation details

Questions:

• Do all function values have a name? Right now I'm operating on the assumption that get_field("name") can totally fail and if it does, we proceed without a name.

• How much of the naive code in the implementation do you think should go into impl Display for Function?

  • Because the name of the function is located in the Value that contains it, not the function itself, i was thinking of exposing something like display_function_body instead or something.

• How do I get at the actual backing source code that generated something? Testing this out in the browser It seems like the console.log implementation sticks the actual defined source code in there including formatting (newlines, spaces, etc).

  • Is it even sane to try to emulate that in any meaningful way?

• Why do const x = () => {} functions not have names?

• Is there a way to get at the backing Declr for any of the function objects? It seems like that's the only place where arrow, expression, and function definitions are differentiated.

Tested with

function normalFunc(arg, argb) {
   let x = 3;
}

function inline() { let x = 3 }
console.log(inline)

const arrowFunc = (arg1, arg2) => {}

console.log(normalFunc)
console.log(arrowFunc);
console.log(String)
console.log(() => {})

Outputs

inline() {let x = 3}
normalFunc(arg, argb) {let x = 3;}
(arg1, arg2) {}
function String() {
 [native Code]
}
() {}
undefined

[Razican]

Test262 conformance changes:

Test result	master count	PR count	difference
Total	78,897	78,897	0
Passed	26,857	26,867	+10
Ignored	15,628	15,628	0
Failed	36,412	36,402	-10
Panics	0	0	0
Conformance	34.04%	34.05%	+0.01%

Fixed tests:

test/built-ins/Function/prototype/toString/S15.3.4.2_A12.js [strict mode] (previously Failed)
test/built-ins/Function/prototype/toString/S15.3.4.2_A12.js (previously Failed)
test/built-ins/Function/prototype/toString/S15.3.4.2_A14.js [strict mode] (previously Failed)
test/built-ins/Function/prototype/toString/S15.3.4.2_A14.js (previously Failed)
test/built-ins/Function/prototype/toString/S15.3.4.2_A13.js [strict mode] (previously Failed)
test/built-ins/Function/prototype/toString/S15.3.4.2_A13.js (previously Failed)
test/built-ins/Function/prototype/toString/S15.3.4.2_A16.js [strict mode] (previously Failed)
test/built-ins/Function/prototype/toString/S15.3.4.2_A16.js (previously Failed)
test/built-ins/Object/getOwnPropertyDescriptor/15.2.3.3-4-35.js [strict mode] (previously Failed)
test/built-ins/Object/getOwnPropertyDescriptor/15.2.3.3-4-35.js (previously Failed)

[kvnvelasco]

Fixed 2 of the 4 broken tests. @Razican how does one run the conformance suite and get that diff? I'm able to run the suite itself but I don't know how to get which tests no longer work (and maybe the broken assertions). Nevermind found it.

Ran
test/built-ins/String/S15.5.2.1_A1_T11.js
test/built-ins/String/S15.5.2.1_A1_T8.js

Both pass locally in strict and non strict mode but it seems like 2 tests are broken somewhere else and I can't find them

Results:
Total tests: 78897
Passed tests: 26799
Ignored tests: 15628
Failed tests: 36470 (panics: 8)
Conformance: 33.97%

Resolved
Ran the conformance test on master and the test on the branch with two separate output logfiles
Diffed both files

[kvnvelasco]

Results:
Total tests: 78897
Passed tests: 26807
Ignored tests: 15628
Failed tests: 36462 (panics: 0)
Conformance: 33.98%

[Razican]

You seem to not be using the this variable here, and it's causing a clippy error. You can change it to _this to avoid it.

[Razican]

Results:
Total tests: 78897
Passed tests: 26807
Ignored tests: 15628
Failed tests: 36462 (panics: 0)
Conformance: 33.98%

I updated the conformance. This is generated in the CI and we have to manually copy it here (until we find a better solution)

[RageKnify]

Phrasing suggestion, overall looks ok.