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

Map error locations to original location via source map #42

Closed
wants to merge 8 commits into from

Conversation

nomeata
Copy link
Collaborator

@nomeata nomeata commented Oct 15, 2018

Thanks to @FloorLamp we now have source maps, and we can use this to post-process the trap messages by wasm to make them both more useful, and more stable as the compiler output changes, by mapping them to the original location in the actorscript file.

This needs a bit more cleanup, in particular about the plumbing where to find this script.

I am tempted to redo the whole test suite in bash or something.

so that instead of unhelpful error locations in the `.wat` or `.wasm`
file upon trap, we put in the corresponding location in the ActorScript.
as an npm package, and also including a nix derivation for it.
and make sure that the nix environment provides it.
@nomeata nomeata changed the title [wip] Map error locations to original location via source map Map error locations to original location via source map Oct 16, 2018
@nomeata
Copy link
Collaborator Author

nomeata commented Oct 16, 2018

Ok, this is good to go, but I fear that it will interfere with @rossberg and @crusso’s current development environment, so I’d like to get a review here.

Basically there is a small useful script (happens to be written in JS) that I would like to use in the test suite, called source-map-pp. These are the ways you can use this now:

  • Use nix-shell -A native to enter an environment with all used tools (including dsh, wasm etc.) available. Does not interfere with your usual environment.
  • Use nix-env -i -f test/source-map-pp -A package once to add souce-map-pp to your PATH, and then continue to use make as before.
  • Avoid nix, and use npm install in test/source-map-pp, and continue to use make as before.

So basically either deal with npm or nix

The stability of the trap locations in the testsuite is only a minor thing (and I can still use source-map-pp to debug my code gen manually). OTOH, if you have to deal with nix anyways eventually, then this would be nice to have…

@nomeata nomeata requested a review from rossberg October 16, 2018 13:36
nomeata added a commit that referenced this pull request Oct 17, 2018
@nomeata
Copy link
Collaborator Author

nomeata commented Nov 26, 2018

I guess now that everyone is using nix, this little feature would be possible. Although it is only half as useful as long as v8 does not report the position of an Unreachable … I think I will close this for now, and maybe come back to it if the itch it scratches gets stronger.

@nomeata nomeata closed this Nov 26, 2018
@rossberg
Copy link
Contributor

V8 can actually report it, but the C API currently doesn't propagate that information. I just opened WebAssembly/wasm-c-api#54 for this.

@nomeata nomeata deleted the map-errorloc-sourcemap branch January 18, 2019 11:44
dfinity-bot added a commit that referenced this pull request Oct 2, 2021
mergify bot pushed a commit that referenced this pull request Oct 2, 2021
dfinity-bot added a commit that referenced this pull request Sep 3, 2023
## Changelog for ic-wasm:
Branch: main
Commits: [dfinity/ic-wasm@496f63f6...0cc218f2](dfinity/ic-wasm@496f63f...0cc218f)

* [`0cc218f2`](dfinity/ic-wasm@0cc218f) Aggressive inlining options for `wasm-opt` ([dfinity/ic-wasm⁠#42](https://togithub.com/dfinity/ic-wasm/issues/42))
mergify bot pushed a commit that referenced this pull request Sep 5, 2023
## Changelog for ic-wasm:
Branch: main
Commits: [dfinity/ic-wasm@496f63f6...0cc218f2](dfinity/ic-wasm@496f63f...0cc218f)

* [`0cc218f2`](dfinity/ic-wasm@0cc218f) Aggressive inlining options for `wasm-opt` ([dfinity/ic-wasm⁠#42](https://togithub.com/dfinity/ic-wasm/issues/42))
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