go-interpreter / wagon

wagon, a WebAssembly-based Go interpreter, for Go.
BSD 3-Clause "New" or "Revised" License
903 stars 148 forks source link

Expose access to Ops to avoid stringly typing downstream #172

Open silasdavis opened 5 years ago

silasdavis commented 5 years ago

Avoid this kind of sadness:

https://github.com/perlin-network/life/blob/master/compiler/ssa.go#L445-L454

with:

operators.Get(operators.CurrentMemory).Name

Signed-off-by: Silas Davis silas@monax.io

codecov-io commented 5 years ago

Codecov Report

Merging #172 into master will increase coverage by 0.01%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #172      +/-   ##
==========================================
+ Coverage   69.71%   69.72%   +0.01%     
==========================================
  Files          48       48              
  Lines        5484     5486       +2     
==========================================
+ Hits         3823     3825       +2     
  Misses       1317     1317              
  Partials      344      344
Impacted Files Coverage Δ
wasm/operators/op.go 76.47% <100%> (+1.47%) :arrow_up:

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 01d3d33...5f0a572. Read the comment docs.

iwasaki-kenta commented 5 years ago

Might using Stringer for op's be a better option over directly fetching the name of the op?

https://github.com/golang/tools/blob/master/cmd/stringer/stringer.go

twitchyliquid64 commented 5 years ago

Feel free to integrate stringer for Op if you wish.

Apologies for the confusion, what I meant was to add a doc comment to Get(opcode byte) Op. Its part of the exported API surface, so we need a doc comment so it shows up with documentation in godoc (See the Commentary --> Doc comments section in effective Go for more rationale).

sbinet commented 5 years ago

ping?