Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement arrow functions #601

Merged
merged 17 commits into from
Mar 3, 2019
Merged

Conversation

sam-lord
Copy link
Contributor

This PR fully implements arrow functions.

As part of getting the test suite to pass, I've done a little playing around with where we call SetFunctionName.

It boils down to this test:

const { cover = (function () {}), xCover = (0, function() {})  } = {};
assert.sameValue(cover.name, 'cover');
assert.notSameValue(xCover.name, 'xCover');

And a similar one with array binding patterns.

I'd consider these relevant changes to the PR as this area did need modifying to work with JintArrowFunctionExpressions anyway.

@lahma
Copy link
Collaborator

lahma commented Feb 25, 2019

I hope I'm not opening Pandora's box here, but have you tried what happens if you add the more extensive arrow function test suite? Test case below should enable them, they are not run at the moment.

using Xunit;

namespace Jint.Tests.Test262
{
    public class ExpressionTests : Test262Test
    {
        [Theory(DisplayName = "language\\expressions\\arrow-function")]
        [MemberData(nameof(SourceFiles), "language\\expressions\\arrow-function", false)]
        [MemberData(nameof(SourceFiles), "language\\expressions\\arrow-function", true, Skip = "Skipped")]
        protected void ArrowFunction(SourceFile sourceFile)
        {
            RunTestInternal(sourceFile);
        }
    }
}

These usually bring out the corner cases of spec 🙂

@lahma lahma added the es6 label Feb 25, 2019
@sam-lord
Copy link
Contributor Author

Hadn't spotted that. There are a few (163) failures, I'll get back to you once I've taken a look through them & spotted any immediate bugs.

@sam-lord
Copy link
Contributor Author

sam-lord commented Mar 1, 2019

Updated with my latest work. This is now a WIP branch that I expect will need some help to get 100% of the tests passing.

@sam-lord sam-lord changed the title Implement arrow functions WIP - Implement arrow functions Mar 1, 2019
* skip strict mode tests let variables
* skip obsolete old ECMA tests
* fix function name setting precondition
* fix function length calculation and property flags
* capture this binding when arrow function is instantiated
@lahma
Copy link
Collaborator

lahma commented Mar 3, 2019

@AmateurMemetics @sebastienros almost there, only two test cases failing, both share the same reason. I'm running out of time for now so if either one of you could help and check with fresh eyes. Just can't figure out where the last scope isolation belongs.

language/expressions/arrow-function/scope-paramsbody-var-open.js

var x = 'outside';
var probeParams, probeBody;

(function(_ = probeParams = function() { return x; }) {
  var x = 'inside';
  probeBody = function() { return x; };
}());

assert.sameValue(probeParams(), 'outside');
assert.sameValue(probeBody(), 'inside');
Assert.Null() Failure
Expected: (null)
Actual:   Test262Error: Expected SameValue(«inside», «outside») to be true

@lahma lahma changed the title WIP - Implement arrow functions Implement arrow functions Mar 3, 2019
@sebastienros sebastienros merged commit 644795e into sebastienros:dev Mar 3, 2019
@kevinroast
Copy link

whoop whoop! this is great work guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants