Conversation
|
Thanks! I think you also need to add a rule to check.ml to allow expressions like assigning a |
regarding removing globals, no problem, I'm using this as a training to understand the project and the spec so no problem for me throwing this later :). Just out of curiosity, will globals be removed for good or there are plans to reintroduce them later? I think globals are easily "shimed" with module memory so maybe they are not that primitive... |
|
@marianoguerra Globals would just be removed from the MVP. See the new #344 (which, when merged, will be followed by a PR in this repo) for more explanation. |
There was a problem hiding this comment.
Here and below, it might be nice to factor out via let t1 = local c x in to avoid duplicating the expression.
There was a problem hiding this comment.
I will, just need to read an introduction to ocaml somewhere, any good recommendation? (I already code in clojure, erlang and scheme so it can be a high level introduction)
There was a problem hiding this comment.
FWIW, I actually find it more readable (since shorter) as is. :)
There was a problem hiding this comment.
Hehe, fair enough. I was on the edge anyhow for such a simple duplicated expr.
|
Should we avoid merging this, based on WebAssembly/design#344 removing globals from MVP? |
|
@jfbastien I can remove globals and merge the other changes if it's ok for you. |
|
Sounds good. Though this needs a new description and name :) |
|
If you update this PR to remove globals, I think we can merge it. |
|
Created #106 which subsumes this PR. |
Merge remote-tracking branch spec/,master
spec:
WebAssembly@f5a260a2
This change was automatically generated by `update-testsuite.sh`
plus `git rm -r proposals/simd/` to remove the simd tests
which have now been merged into the spec repo.
this are some simple tests to check that globals are set and return the set value both from inside the module and from outside and that setting two different globals doesn't affect each other.
this is not in sync with the spec which says that store should return the stored value, will look into it now.