[Merged by Bors] - Fix super call execution order

[raskad]

This Pull Request fixes/closes #2672.

It changes the following:

• Get the super constructor and the new target before executing arguments in super calls.

[github-actions]

Test262 conformance changes

Test result	main count	PR count	difference
Total	94,277	94,277	0
Passed	71,071	71,071	0
Ignored	17,324	17,324	0
Failed	5,882	5,882	0
Panics	14	14	0
Conformance	75.39%	75.39%	0.00%

[codecov]

Codecov Report

Merging #2724 (9a79ce8) into main (9b53859) will increase coverage by 0.38%.
The diff coverage is 78.57%.

@@            Coverage Diff             @@
##             main    #2724      +/-   ##
==========================================
+ Coverage   50.45%   50.84%   +0.38%     
==========================================
  Files         395      398       +3     
  Lines       39810    39942     +132     
==========================================
+ Hits        20085    20307     +222     
+ Misses      19725    19635      -90     

Impacted Files	Coverage Δ
boa_engine/src/vm/code_block.rs	46.95% <ø> (+2.97%)	⬆️
boa_engine/src/vm/flowgraph/mod.rs	0.00% <ø> (ø)
boa_engine/src/vm/opcode/mod.rs	45.45% <ø> (ø)
boa_engine/src/vm/opcode/environment/mod.rs	51.18% <77.77%> (+28.60%)	⬆️
boa_engine/src/bytecompiler/expression/mod.rs	66.84% <100.00%> (+5.55%)	⬆️

... and 24 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[HalidOdat]

Looks good to me! :)

[jedel1043]

Tested this case with V8 and SM but V8 throws instead of returning true, and seeing that there are no test262 tests affected by this change, it seems like this behaviour is not specified by the spec. Should we open an issue on ecma262 before committing to this behaviour?

[raskad]

Tested this case with V8 and SM but V8 throws instead of returning true, and seeing that there are no test262 tests affected by this change, it seems like this behaviour is not specified by the spec. Should we open an issue on ecma262 before committing to this behaviour?

The two tests that I added result in the expected results in both SM and V8 for me.
The spec is also pretty good here. The section is in SuperCall : super Arguments https://tc39.es/ecma262/#sec-super-keyword-runtime-semantics-evaluation:

...
3. Let func be GetSuperConstructor().
4. Let argList be ? ArgumentListEvaluation of Arguments.
5. If IsConstructor(func) is false, throw a TypeError exception.
6. Let result be ? Construct(func, argList, newTarget).
...

We currently do step 4 before we fo the rest and that causes the panics / wrong results.

[jedel1043]

Ah, so the spec does specify this behaviour! Then it seems like SpiderMonkey V8 has a bug, probably...

SpiderMonkey:

V8:

The spec is also pretty good here. The section is in SuperCall : super Arguments

Then wouldn't that invalidate the second test, since it is not throwing when calling the constructor?

EDIT: Realized the behaviour is the opposite, it shouldn't throw because it gets the constructor before calling setPrototypeOf, so it's most certainly a V8 bug instead.

[jedel1043]

Good catch!

[raskad]

bors r+

[bors]

Pull request successfully merged into main.

Build succeeded:

• Build and Test (macos-latest)
• Build and Test (windows-latest)
• Coverage
• Misc