Skip to content
This repository has been archived by the owner on May 11, 2020. It is now read-only.

wasm: fix types decoding #152

Merged
merged 2 commits into from
Aug 2, 2019
Merged

wasm: fix types decoding #152

merged 2 commits into from
Aug 2, 2019

Conversation

laizy
Copy link
Member

@laizy laizy commented Aug 2, 2019

No description provided.

ValueTypeI32 ValueType = 0x7f
ValueTypeI64 ValueType = 0x7e
ValueTypeF32 ValueType = 0x7d
ValueTypeF64 ValueType = 0x7c
Copy link
Member Author

Choose a reason for hiding this comment

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

These constants are obtained from the specification: https://webassembly.github.io/spec/core/binary/types.html#binary-valtype

Copy link
Contributor

@sbinet sbinet left a comment

Choose a reason for hiding this comment

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

LGTM modulo a few nitpicks.

wasm/read.go Outdated
@@ -35,6 +35,17 @@ func readBytes(r io.Reader, n int) ([]byte, error) {
return nil, io.ErrUnexpectedEOF
}

func WriteByte(w io.Writer, b byte) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps put it in a new wasm/write.go file?

also, perhaps:

Suggested change
func WriteByte(w io.Writer, b byte) error {
func writeByte(w io.Writer, b byte) error {

Copy link
Member Author

Choose a reason for hiding this comment

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

The ReadByte method is used in disasm package. For the sake of symmetry, I exposed it too. and maybe it will be used in later fix.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

wasm/types.go Outdated Show resolved Hide resolved
wasm/read.go Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Aug 2, 2019

Codecov Report

Merging #152 into master will decrease coverage by 0.06%.
The diff coverage is 64.86%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #152      +/-   ##
==========================================
- Coverage   69.61%   69.54%   -0.07%     
==========================================
  Files          43       43              
  Lines        4959     4974      +15     
==========================================
+ Hits         3452     3459       +7     
- Misses       1222     1226       +4     
- Partials      285      289       +4
Impacted Files Coverage Δ
disasm/asm.go 100% <100%> (ø) ⬆️
wasm/read.go 76.59% <100%> (+4.09%) ⬆️
disasm/disasm.go 81.78% <100%> (ø) ⬆️
wasm/types.go 45.83% <50%> (-2.3%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 46451e6...57e3f7e. Read the comment docs.

@sbinet sbinet merged commit 4c85b4e into go-interpreter:master Aug 2, 2019
@laizy laizy deleted the fix-types branch August 2, 2019 09:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants