Skip to content

Optimized Rest Parameter -> Spread Operator#628

Closed
tomblind wants to merge 7 commits intomasterfrom
feature/elipsis-forward
Closed

Optimized Rest Parameter -> Spread Operator#628
tomblind wants to merge 7 commits intomasterfrom
feature/elipsis-forward

Conversation

@tomblind
Copy link
Copy Markdown
Collaborator

@tomblind tomblind commented Jun 9, 2019

When a rest parameter is referenced with a spread operator, the transformed code packs and unpacks it:

function foo(this: void, ...args: unknown[]) {
    console.log(...args);
}

=>

function foo(...)
    local args = ({...})
    print(unpack(args))
end

This PR optimizes this so that the elipsis operator is used instead. It also prevents generation of the temp local if it is not needed in the function.

Update
Due to issues with the optimization being applied inside generated IIFE's, this PR has been updated to instead provide an explicit @elipsisForward directive to apply this optimization.

/** @elipsisForward */
declare function elipsisForward<A extends unknown[]>(args?: A): A;

function foo(this: void, ...args: unknown[]) {
    console.log(...elipsisForward(args));
}

=>

function foo(...)
    print(...)
end

The new directive can only be used on a function, and that function can only be used in a spread expression, and passed a rest argument.

You can also pass no argument, which is useful for accessing the file-scope elipsis:

const arg = [...elipsisForward()];

=>

local arg = {...}

@ark120202
Copy link
Copy Markdown
Contributor

This optimization shouldn't be applied if it's used in the inner function:

function foo(...a: any[]) {
    function bar() {
        print(...a);
    }
}
function foo(self, ...)
    local function bar(self)
        print(...)
    end
end
3: cannot use '...' outside a vararg function near '...'

@ark120202
Copy link
Copy Markdown
Contributor

It also shouldn't optimize it in try-catch

function foo(...a: any[]) {
    try {
        print(...a);
    } catch {}
}

and expressions that require IIFE

function foo(...a: any[]) {
    const _ = (0, print(...a));
}

@tomblind
Copy link
Copy Markdown
Collaborator Author

closing to open as a fresh PR since it's changed so radically

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants