Always return undefined from functions that do not return

[raskad]

This Pull Request fixes/closes #672.

It changes the following:

• Always return undefined from functions that do not return
• Fix JSON.parse test

[raskad]

The test test/built-ins/JSON/parse/reviver-get-name-err.js fails now because of an error in the JSON.parse implementation.

Test262 conformance changes:

Test result	master count	PR count	difference
Total	80,685	80,685	0
Passed	32,670	32,700	+30
Ignored	15,818	15,818	0
Failed	32,197	32,167	-30
Panics	0	0	0
Conformance	40.49%	40.53%	+0.04%

Fixed tests (32):

test/built-ins/Date/value-symbol-to-prim-invocation.js [strict mode] (previously Failed)
test/built-ins/Date/value-symbol-to-prim-invocation.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-570.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-570.js (previously Failed)
test/built-ins/RegExp/prototype/Symbol.replace/fn-invoke-args-empty-result.js [strict mode] (previously Failed)
test/built-ins/RegExp/prototype/Symbol.replace/fn-invoke-args-empty-result.js (previously Failed)
test/built-ins/RegExp/prototype/Symbol.replace/fn-invoke-args.js [strict mode] (previously Failed)
test/built-ins/RegExp/prototype/Symbol.replace/fn-invoke-args.js (previously Failed)
test/built-ins/Array/prototype/findIndex/predicate-called-for-each-array-property.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/findIndex/predicate-called-for-each-array-property.js (previously Failed)
test/built-ins/Array/prototype/findIndex/predicate-call-parameters.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/findIndex/predicate-call-parameters.js (previously Failed)
test/built-ins/Array/prototype/find/predicate-called-for-each-array-property.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/find/predicate-called-for-each-array-property.js (previously Failed)
test/built-ins/Array/prototype/find/predicate-call-parameters.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/find/predicate-call-parameters.js (previously Failed)
test/built-ins/Array/prototype/concat/is-concat-spreadable-get-order.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/concat/is-concat-spreadable-get-order.js (previously Failed)
test/built-ins/JSON/stringify/value-tojson-arguments.js [strict mode] (previously Failed)
test/built-ins/JSON/stringify/value-tojson-arguments.js (previously Failed)
test/language/expressions/arrow-function/statement-body-requires-braces-must-return-explicitly-missing.js [strict mode] (previously Failed)
test/language/expressions/arrow-function/statement-body-requires-braces-must-return-explicitly-missing.js (previously Failed)
test/language/expressions/equals/coerce-symbol-to-prim-invocation.js [strict mode] (previously Failed)
test/language/expressions/equals/coerce-symbol-to-prim-invocation.js (previously Failed)
test/language/expressions/addition/get-symbol-to-prim-err.js [strict mode] (previously Failed)
test/language/expressions/addition/get-symbol-to-prim-err.js (previously Failed)
test/language/expressions/addition/coerce-symbol-to-prim-invocation.js [strict mode] (previously Failed)
test/language/expressions/addition/coerce-symbol-to-prim-invocation.js (previously Failed)
test/language/statements/function/S13.2.1_A9.1_T1.js [strict mode] (previously Failed)
test/language/statements/function/S13.2.1_A9.1_T1.js (previously Failed)
test/language/statements/function/S13.2.1_A9.1_T2.js [strict mode] (previously Failed)
test/language/statements/function/S13.2.1_A9.1_T2.js (previously Failed)

Broken tests (2):

test/built-ins/JSON/parse/reviver-get-name-err.js [strict mode] (previously Passed)
test/built-ins/JSON/parse/reviver-get-name-err.js (previously Passed)

[jedel1043]

Huh. Maybe we don't change the executor to InterpreterState::Return when throwing?
Edit: Yep.

boa/boa/src/syntax/ast/node/throw/mod.rs

Lines 51 to 56 in ae73c26

	impl Executable for Throw {
	#[inline]
	fn run(&self, context: &mut Context) -> JsResult<JsValue> {
	Err(self.expr().run(context)?)
	}
	}

I don't know if setting the executor to Return will break anything. If it does, we can create a new Throw state.
Or maybe it has nothing to do with this and it really is an error on JSON.parse

[jedel1043]

Just an additional comment. The correct fix in this case would be to check every statement here and every declaration here and return the corresponding Completion Record, but knowing we haven't implemented Completion Records to propagate state at execution, this is possibly the best we can do right now.

Maybe we should start thinking about refactoring the executor to return Completion Records instead of JsValues.

[Razican]

Nice catch!

[jedel1043]

I think we should create an issue with this, yes. Not sure if this is the parser's job or the executor's job, though.

Ah sorry, it's the executor's job.

[jedel1043]

Nice, Github is bugging lol

[jedel1043]

Does setting the interpreter inside the throw ast to InterpreterState::Return work for fixing the failing tests? If not, you can ignore this.

[raskad]

Does setting the interpreter inside the throw ast to InterpreterState::Return work for fixing the failing tests? If not, you can ignore this.

I tried that, it does not fix the failing test, but some others fail and some pass. We definitely have to investigate how we implement completion records. I will try to find a good example and open an issue based on that.

[jedel1043]

Does setting the interpreter inside the throw ast to InterpreterState::Return work for fixing the failing tests? If not, you can ignore this.

I tried that, it does not fix the failing test, but some others fail and some pass. We definitely have to investigate how we implement completion records. I will try to find a good example and open an issue based on that.

Okay, let me approve then :)

[jedel1043]

Completion Records should fix the two failing tests. Good work!