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

Match on platform #263

Merged
merged 2 commits into from
Jul 18, 2018
Merged

Match on platform #263

merged 2 commits into from
Jul 18, 2018

Conversation

hcnelson99
Copy link
Contributor

Makes match statements work on platform objects (require platform) in the interpreter and javascript backend.

For example, you could use it to select a platform-specific implementation of some API as I do in backend's main.wyv:

require platform

val plt: system.Platform = platform

val support = match plt:
    java: system.Java => support_java.apply(java)
    javascript: system.JavaScript => support_js.apply(javascript)

Platform has to be cast up to type system.Platform for the match because require platform creates a platform object with the precise type of the current platform (e.g. system.JavaScript or system.Java). This could be changed but it breaks the behavior of existing tests and doesn't play well with platform-specific imports. Here's the problem:

require platform

import stdout

val stdout = stdout.apply(platform)

stdout.print("Hello, world!\n")

If we change the behavior of require platform so that the platform object has type system.Platform, this code will not compile because import stdout brings in some module depending on the platform (maybe stdlib/platform/java/stdout.wyv) which has a module def stdout(java : Java) : Stdout so it will not like being passed a system.Platform. I can think of a few different ways to fix this, but I'm not sure what the best way is. Thoughts?

This patch changes how the system object is handled by the compiler in a big way. Instead of adding it to the StandardEvalContext, we add it before every module in the actual SeqExpr so that when bytecode is emitted the system object is present and tag information can be generated. Otherwise, a simple if statement in expression/Match.wyv and getTag() method in expression/FFI.java were all that were added. Thoughts on this? This is one of the more intrusive changes I've made to the compiler.

This patch also fixes a bug where match statements take the last case that matches instead of the first (missing break statement). Nearly all match statements in examples/algebra.wyv were reordered to fix this behavior. I confirmed with Justin and tested that this patch doesn't effect algebra.wyv's behavior.

@JonathanAldrich
Copy link
Member

The match statement ordering fix is great.

I think the functionality to check tags on platforms is important, too. But I'm a bit ambivalent about adding it to every module. It's probably OK in the short term, but in the longer term, where I really want to move the system object is to the prelude. Right now that has the same effect, but eventually the prelude should be compiled to its own bytecode file, which will factor the system object out of other bytecode files.

Henry, do you think that accepting this patch will make it harder to go in that direction in the long term, or easier, or about the same?

@JonathanAldrich
Copy link
Member

Update: I see you've added it implicitly as part of the prelude. That probably matches well with my comment above, but let's discuss briefly today.

@hcnelson99
Copy link
Contributor Author

I've added a commit to this PR that allows the use of platform imports outside of the stdlib. It also changes the backend to use platform imports instead of match because it doesn't need to do runtime dispatch. It isn't a separate PR because it relies on a refactor implemented in 6588788 from this PR. I can rework this if necessary.

In reply to the above, 6588788 does modify Globals.getPreludeModule() to return a prelude module with the system object in it. Note that it also changes the meaning of having usePrelude = false. Previously, when usePrelude = false, getPreludeModule() would return a prelude which was an empty SeqExpr. Now when usePrelude = false, a prelude with only the system object will be returned. This PR doesn't modify stdlib/prelude.wyv.

Match on platform still doesn't work with python.

@JonathanAldrich JonathanAldrich merged commit e5e1a8c into wyvernlang:master Jul 18, 2018
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