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

Validation #49

Closed
4 tasks done
ballercat opened this issue Dec 27, 2017 · 8 comments
Closed
4 tasks done

Validation #49

ballercat opened this issue Dec 27, 2017 · 8 comments

Comments

@ballercat
Copy link
Owner

ballercat commented Dec 27, 2017

Goal

Make it easier to extend the syntax and make compiler easier to work with.

Overview

This should unblock #28

Currently, the individual node parsing methods perform in-place validation on the syntax. For example, the identifier parser may throw an undefined variable syntax error if it cannot identify the variable.

ctx.handleUndefinedIdentifier(ctx.token.value);

This is OK, but the more syntax is extended the harder it is to extend the behavior. In the above code, the undefined variable handler is overwritten to silence the error while parsing function arguments(while parsing the parent node). Instead of doing the above, let's have another step before the generator, a static type/validator checker.

The final pipeline will look like this:

Tokenizer -> tokens -> Parser -> ast -> Static Checker -> ast/dag -> Generator -> ir -> Emitter -> bytecode

Initial implementation would only handle Identifier nodes. This means the static checker would do the following:

  • Ensure the identifier is valid/in-scope
  • Patch nodes to attach correct metadata, like global/local indexes, type definitions, function indexes and table pointers
  • throw syntax errors if the identifier is undefined and only do so in correct context. For example, it should be fine for an identifier to be undefined in a function definition because that is the first node it's defined in.
  • Code generation needs to be moved entirely to its own step. Currently, functions generate IR in-place but this will need to change.
@iddan
Copy link

iddan commented Dec 27, 2017

Maybe the validation should work on Flow AST?

@baransu
Copy link
Contributor

baransu commented Dec 27, 2017

@iddan Could you provide more information what do you have in mind?

@iddan
Copy link

iddan commented Dec 27, 2017

If the pipeline is:

Tokenizer -> tokens -> Parser -> ast -> Static Checker -> ast/dag -> Generator -> ir -> Emitter -> bytecode

Then we can replace the Tokenizer -> tokens -> Parser -> ast part with flow-parser or Babylon
so our pipe would like:

Babylon | flow-parser -> Static Checker -> ast/dag -> Generator -> ir -> Emitter -> bytecode

This will reduce the scope that Walt has to deal with and allow any valid JS/Flow syntax in the AST level

@baransu
Copy link
Contributor

baransu commented Dec 27, 2017

So you propose that we move from our custom parser and tokenizer to babylon/flow-parser and then produce WASM output?

@iddan
Copy link

iddan commented Dec 27, 2017

Exactly as the tokens and syntax is the same only the meaning of them change

@ballercat
Copy link
Owner Author

I think once the project is mature and stable enough something like that should be possible. Right now I'd like to keep the discussion focused on implementing the changes outlined in the issue.

@ballercat
Copy link
Owner Author

I've updated the description as this is actually two pieces of work. #51 is added as a prerequisite.

While (#51) should be resolved before this is implemented, it is possible to resolve both issues in parallel, this entire thing can be implemented and tested with a few unit tests.

@ballercat ballercat assigned ballercat and unassigned ballercat Dec 30, 2017
@ballercat ballercat changed the title Static Checker/Validation step Validation Jan 4, 2018
@ballercat
Copy link
Owner Author

I've unblocked this with #56 and implemented a basic validation step.

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

No branches or pull requests

3 participants