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

[wasm] Have the WasmError::User member be a String #760

Merged
merged 1 commit into from
May 14, 2019

Conversation

bnjbvr
Copy link
Member

@bnjbvr bnjbvr commented May 3, 2019

As said on IRC, using failure::Error requires to have the failure/std feature set, which adds an old version of backtrace as a crate dependency. In addition to supplementary compilation cost (the build is longer!), it also means that some platforms won't quite work: windows aarch64 in particular doesn't plain work.

A few options I've considered:

  • go the hard, open-source way, and update backtrace in failure etc. This would be my preferred way to do things, but that implies a lot of reliance on external factors (failure accepting the pull request, releasing a new version etc.) and our work (on Spidermonkey) is unfortunately blocked by this. So I've removed the one use of the failure::Error in the code, and the failure/std dependency.
  • instead of having a failure::Error, this could be a Box<failure::Fail> or Box<error::Error + Sync + Send>.
  • or a plain String, which also works in non-std builds. I've chosen this path since our use cases don't indicate a clear preference over one way or the other (so I've concluded that the simplest would be best), but I'm open to other opinions.

Anyway, I'd be happy to hear people's thoughts about this.

@pchickey
Copy link
Contributor

pchickey commented May 3, 2019

Seems like the most expedient thing to do is switch to either String or the boxed trait object in order to get you unblocked, and then in the background we can get changes upstreamed in failure, and switch back to using failure::Error once that is done.

The boxed trait object and String will both work the same for me, so I'd just go with String because it is simpler.

@bnjbvr
Copy link
Member Author

bnjbvr commented May 14, 2019

Thanks!

@bnjbvr bnjbvr merged commit cc216b4 into bytecodealliance:master May 14, 2019
@bnjbvr bnjbvr deleted the wasmerror-user-string branch May 27, 2019 12:41
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

2 participants