-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
First version of the interpreter of Mulang #202
Conversation
spec/InterpreterSpec.hs
Outdated
|
||
it "condition is false then evals second branch" $ do | ||
lastRef (run' [text| | ||
if(false){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is Nice, but it is not how JS behaves
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I'm not testing against the value returned by the expression, but rather against the last reference created by the interpreter. So if the value is true, it will create a reference for the value in the first branch, and if false for the one in the second. That does not mean that that is the result of an expression.
spec/InterpreterSpec.hs
Outdated
lastRef (run "'123' == '123'") `shouldReturn` MuBool True | ||
|
||
it "is false when values are of different types" $ do | ||
lastRef (run "'123' == 123") `shouldReturn` MuBool False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one also returns True in JS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True. I actually already commented this to @flbulgarelli and we decided that's not really important, since it's more of a quirk of JS rather than standard procedural / object-oriented behavior, which is what we actually aim to teach.
…iscard TypeFamilies and ConstraintKinds extensions
That is awesome. Now this interpreter has also a very basic support for python, and potentially other imperative languages. There are some things still missing, that we should address in the future:
That said, I think this PR is ready to be merged. |
README.md
Outdated
``` | ||
|
||
```haskell | ||
TestGroup (MuString "succ") (Lambda [] ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should remove those lambdas. Also we should document python test syntax
src/Interpreter/Runner.hs
Outdated
| Failure String | ||
deriving (Show, Eq) | ||
|
||
runTestsForDir :: String -> IO () |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should remove this function, or parameterize it, so that is not .js
-files-specific.
src/Language/Mulang/Parsers/Java.hs
Outdated
@@ -91,7 +91,7 @@ muStmt (Do body cond) = M.While (muStmt body) (muExp cond) | |||
muStmt (Return exp) = M.Return $ fmapOrNone muExp exp | |||
muStmt (ExpStmt exp) = muExp exp | |||
muStmt Empty = None | |||
muStmt (Assert exp _) = SimpleSend Self "assert" [muExp exp] | |||
--muStmt (Assert exp _) = muAssertion $ muExp exp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should remove this line
normalizeTests expr = normalizeTest expr | ||
|
||
normalizeTest func@(M.SimpleProcedure name _ body) = case isPrefixOf "test_" name of | ||
True -> M.Test (M.MuString name) body |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to use guards here or even a local function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or an if
expression
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, that actually makes more sense. Whoops!
src/Interpreter/Mulang.hs
Outdated
createReference :: Value -> Executable Reference | ||
createReference value = do | ||
nextReferenceId :: Reference <- gets (fromMaybe (Reference 10) . fmap incrementRef . getMaxKey . globalObjects) | ||
modify (updateGlobalObjects $ Map.insert nextReferenceId value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why reference 10
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh yeah, I saw that too. I don't think it has any special meaning, I think we could just replace that with a fromJust. I think globalObjects should never be empty.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already talked with @julian-berbel and that 10 is useless, I think I replaced it with an exception but I never pushed that code. Initially objectSpace was empty but then I added contexts as objects and there is always at least one object alive so that 10 is never used
src/Interpreter/Mulang.hs
Outdated
|
||
putRef :: Reference -> Value -> Executable () | ||
putRef ref val = do | ||
modify $ updateGlobalObjects $ Map.insert ref val |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to avoid overuse of application operation, and favour composition operation instead. E.g.:
modify . updateGlobalObjects $ Map.insert ref val
modify . updateGlobalObjects . Map.insert ref $ val
My rule of thumb is one $
per line. Mulang code follows it quite strictly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, Mulang code tends to favor point-free style and avoid unnecessary do
-blocks:
putRef ref = modify . updateGlobalObjects . Map.insert ref
src/Interpreter/Mulang.hs
Outdated
updateGlobalObjects f context = | ||
context { globalObjects = f $ globalObjects context | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd recommend one-lining simple expressions that don't require multiple span - do
-expressions or really long applications.
I particularly enforce this with solo-chars:
updateGlobalObjects f context =
context { globalObjects = f $ globalObjects context }
updateGlobalObjects f context = context { globalObjects = f $ globalObjects context }
No description provided.