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

Jscompilation #145

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Jscompilation #145

wants to merge 8 commits into from

Conversation

nscarcella
Copy link

@nscarcella nscarcella commented Jan 12, 2018

So...

I'm making this PR with a first draft of an implementation requested by @flbulgarelli . Basically, the idea is to generate ES5 source code from a Mulang AST so it can be executed with, hopefully, similar semantics to the original source code.

As discussed with Franco, there is really no way to produce code with the exactly same behavior as the original, because some semantics are lost on parsing, and there is no way to know how the original language would behave (for example, an if in some OO languages is an expression and returns a result, while in others is just a non-returning sentence) but, in general terms, it is possible to default these aspects to handle most cases.

I did not try to compile ALL Mulang expressions. Things like Facts and other Logic Oriented abstractions would require some extra work to execute (it's not that hard, but it would require some sort of querying interface).

The generated code needs to be preppended with a small set of primitive functions that acts as a Runtime Environment (I call it MRE because I'm super original) and will probably differ from language to language. On the code comments are described all the primitives needed on the MRE, but I guess I can provide an example if needed.

I tried to be as clear as possible and add documentation to all functions, but I'm not used to your code conventions and have no good IDE for Haskell so I hope is good enough.

Tests. Well... There are tests, but they are not the tests that I would like. I am currently checking the generated strings to match the expected text, but this is IN NO WAY the right way to test this kind of thing. Ideally, the tests should run the generated code on a node instance and check the result, problem with this is that I found no way to do this without messing with the cabal configuration and it felt a bit too invasive for a first PR.

As final, minor note, I think there are a couple of rough edges on the model that could be improved. Some of which makes it hard to understand and, in consequence, hard to compile to other language. I left some TODOs with inquiries and marked points where I made educated guesses about how things work.

Fixes #55

@ghost ghost added the pending-review label Jan 12, 2018
@nscarcella
Copy link
Author

What does Travis mean with: "The constructor ‘Switch’ should have 3 arguments, but has been given 2"? In AST the Switch structure is defined as: Switch Expression [(Expression, Expression)]

@flbulgarelli
Copy link
Member

What does Travis mean with: "The constructor ‘Switch’ should have 3 arguments, but has been given 2"? In AST the Switch structure is defined as: Switch Expression [(Expression, Expression)]

Weird, I will check

@flbulgarelli
Copy link
Member

As discussed with Franco, there is really no way to produce code with the exactly same behavior as the original, because some semantics are lost on parsing, and there is no way to know how the original language would behave (for example, an if in some OO languages is an expression and returns a result, while in others is just a non-returning sentence) but, in general terms, it is possible to default these aspects to handle most cases.

👍

I did not try to compile ALL Mulang expressions. Things like Facts and other Logic Oriented abstractions would require some extra work to execute (it's not that hard, but it would require some sort of querying interface).

👍 It is fine. For this proof of concept, handling logic programming is far beyond scope.

The generated code needs to be preppended with a small set of primitive functions that acts as a Runtime Environment (I call it MRE because I'm super original) and will probably differ from language to language. On the code comments are described all the primitives needed on the MRE, but I guess I can provide an example if needed.

Agree. I was thinking about an even smaller MRE - for example, for a first proof of concept I wouldn't have wrapped numbers. But I agree with you it would have been necessary at some point.

I tried to be as clear as possible and add documentation to all functions, but I'm not used to your code conventions and have no good IDE for Haskell so I hope is good enough.

No worries, I will have a more in-depth look now. I will do some extra commits if necessary.

Tests. Well... There are tests, but they are not the tests that I would like. I am currently checking the generated strings to match the expected text, but this is IN NO WAY the right way to test this kind of thing. Ideally, the tests should run the generated code on a node instance and check the result, problem with this is that I found no way to do this without messing with the cabal configuration and it felt a bit too invasive for a first PR.

I agree. Right now - since last week -, we are also embedding the mulang gem wrapper in the same repository under the wrapper directory, so we can also access to a ruby environment if more js dependencies are required.

As final, minor note, I think there are a couple of rough edges on the model that could be improved. Some of which makes it hard to understand and, in consequence, hard to compile to other language. I left some TODOs with inquiries and marked points where I made educated guesses about how things work.

Awesome! Docs are not our best feature - we extended our README during the last few days, but it is never enough - and definitely the OOP support still needs work - e.g. we introduced the MuNil element just a few weeks ago. See:

We are planning to improve the AST on mulang 4, the next major release we are now developing in master. This new brand JSCompiler will come with it.

I will look at your comments and open issues when needed.

)

it "EntryPoint" $ do
(EntryPoint "foo" (MuBool True)) `shouldBeCompiledTo` "function foo() { return new MuBool(true) }"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it is an entry point, and JS does not actually have a main program, we should also execute this code, or register into the MRE so it is called after all the code is loaded.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, never mind, we should definitely no execute the entry points, since it could have many and it is resposibility of the code running system to call it 😅

it "Record" $ do
(Record "foo") `shouldBeCompiledTo` ""

it "Function" $ do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a side note: I prefer to declare those groups of tests as a describe with multiple its in order to keep tests separated.

(Record "foo") `shouldBeCompiledTo` ""

it "Function" $ do
(Function "f" []) `shouldBeCompiledTo` "function f() {try { throw new MuPatternMatchError() }catch($error) { if($error.constructor === MuReturn) { return $error.value } else { throw $error } } }"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd use SimpleFunction instead of plain Functions, which is an individual case for functions that have just one unguarded equation.

(Function "f" []) `shouldBeCompiledTo` "function f() {try { throw new MuPatternMatchError() }catch($error) { if($error.constructor === MuReturn) { return $error.value } else { throw $error } } }"
(Function "f" [Equation [] (UnguardedBody (Return (MuNumber 5)))]) `shouldBeCompiledTo` (
"function f() {try { " ++
"if(arguments.length === 0){ return function(){ throw new MuReturn(new MuNumber(5.0)) }() } " ++
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! We definitely must check the arguments count.

"function f() {try { " ++
"if(arguments.length === 0){ return function(){ throw new MuReturn(new MuNumber(5.0)) }() } " ++
"throw new MuPatternMatchError() " ++
"}catch($error) { if($error.constructor === MuReturn) { return $error.value } else { throw $error } } }"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why you are dealing with normal application scenario using exceptions for control flow?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sight... Here is the issue: Since some constructions that many languages use as expressions are not expressions in JS (if, try/catch, etc.) I need to wrap them in a function call to make them return values. Problem is, that stops the return statements from leaving the current context. So, for example, if you had some java:

int f (){
if(x) { return 1 };
return 2;
}

The first return would exit the if function context but not the f function itself as expected.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it

it "MuNull" $ do
MuNull `shouldBeCompiledTo` "new MuNull()"

it "Sequence" $ do pending
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that a sequence should be just the join of the translated expressions using a ;. For example, the given code

Sequence [Send (Reference "x") (Reference "foo") [], Send (Reference "x") (Reference "bar") []]

should map to:

x.foo() ; x.bar()

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sequences are being compiled, the tests are the ones pending.

I did pretty much that, but prepended a return to the last element so it would leave the context with that value, because I wasn't sure if the parsers where inserting Return statements on languages with default return.

it "Send" $ do
(Send (Reference "o") (Reference "m") []) `shouldBeCompiledTo` "o['m']()"
(Send (Reference "o") (Reference "m") [Reference "x", Reference "y"]) `shouldBeCompiledTo` "o['m'](x, y)"
(Send (Send (Reference "o") (Reference "m") [Reference "x"]) (Reference "n") [Reference "y"]) `shouldBeCompiledTo` "o['m'](x)['n'](y)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side note: while I was reading the PR, I was wondering.... #149

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hehe, I guess that depends on what crazy cases you are planning on parsing :p

I guess if you had JS that does obj["foo" + "bar"] you could on this model call that a send and would need the identifier to be more elaborated than an identifier...

Still, I think I preffer the identifier.

-- | It might be reasonable to drop this whole implementation in the future and replace it with a
-- | direct interpretation of the Ast.
-- |
-- | TODO: A proper implementation of this would escape all JS reserved (or context relevant) words on the AST, (for
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

-- | Notice that, since some semantics of the original code's language are lost during the parsing,
-- | executing this code might not be equivalent to executing the original sources.
-- | It might be reasonable to drop this whole implementation in the future and replace it with a
-- | direct interpretation of the Ast.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it worth the effort? Which benefits would it yield?

If performance is a concern - It is not right now actually, or not that much - what about using http://asmjs.org/ as an intermediate approach? It is a lower level target JavaScript but still a source-to-source approach.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, it's not about performance.

I guess it all ends up being a matter of preferences, but my point is that js is going to be a nice(ish) container for (some) OO model, but not so much for functional or logic. In my experience it's not that much harder to interpret an AST than compile it and it lets you have direct control on what should happen instead of having to worry about how JS will behave. It is also much cleaner to read :P and does not introduce dependencies to node to run tests properly.

If you really want src to src I don't think it matters what is the target language. JS is simple enough to be a good choice.

In any case not my call to make, but thought I should mention it.

-- Generates a JS assignation statement that replaces the current context's constructor's prototype with a copy of
-- itself merged with the mixin's prototype.
-- It also calls the mixin initialization on the new instance.
compile (Include _name) = do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend not using _identifiers, since it means a non interesting value but that still has a name

cases <- compileAll _cases " else "
return [text| $cases throw new MuPatternMatchError() |]

type Guard = (Expression, Expression)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we could move the Guard and Catch aliases to Ast.hs

Copy link
Author

@nscarcella nscarcella Jan 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you could make them constructors instead of aliases? :D

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

Successfully merging this pull request may close these issues.

2 participants