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

feature/additional parameters #498

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

TobiasWienand
Copy link
Contributor

I decided to close PR #449 simply because the diff to main became too high.

This PR adds support for compiling ArrayPattern, ObjectPattern, and RestElement constructs, along with related tests.

Changes to the codebase have been kept as minimal as possible since, in most cases, we only need functions with plain parameters. That is why we now provide two constructors for the Parameters structure. The old constructor remains unchanged, while the new one includes an additional argument—patterns—which stores the signature’s "structure" used for type inference and lifting.

If I understand correctly, JSTyper.swift performs type inference both on the function's signature and on its inner outputs. In each case, a different view is required: A callee should see the parameters in the strict sense. For example, in

f([a, b], {foo: c, bar: d})

the function has two parameters: one is an array pattern and the other is an object pattern. However, within the function's scope, the parameters should be viewed in their destructured form—in this example, as a, b, c, and d. These destructured parameters are still typed as either plain (a single value), rest (an iterable), or opt (possibly undefined). These two views are toggled on and off using the outerView parameter in inferSubroutineParameterList.

CodeGenerators remain unchanged, simply the use case for these new Signatures is mainly for compilation, not for generation. I could write CodeGenerators that generates these functions along with suitable arguments if that is wanted.

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.

1 participant