brson / miri

An experimental compiler from Rust to WebAssembly (inactive - do not use)
Apache License 2.0
209 stars 15 forks source link

First pass at asserts #36

Closed lqd closed 8 years ago

lqd commented 8 years ago

The lack of user message, the generated console message and trace are less useful without the ability to print characters when the assert fails, but the interpreter does trap when it needs to!

Here you go @eholk, it is still interesting for the tests — even though we might now need a run-fail to check that assert fails :)

brson commented 8 years ago

lgtm

lqd commented 8 years ago

an interesting question we should keep in mind for the future: should panic! also unwind and call drop everywhere ?

lqd commented 8 years ago

After thinking about panic! for some time: while the assert! as implemented here works, the more general case of panic! will probably have to be tweaked a bit. Here a diverging function will return void in wasm, and any rust call to panic! to not require returning a value will cause a problem: wasm will validate that all paths return a value, except if one of the paths is the unreachable node, which is used in the $crate::panic() code, but AFAICT, and I don't know the spec on this, calling a function that will trigger unreachable is not itself considered (at least by binaryen) the same as triggering unreachable, therefore the module could be seen as not valid. This can be mitigated by emitting unreachable directly instead of the panic fn — but has interesting ramifications with the previous comment: if panic has to unwind and drop, then we might have to emit both the fn call and the unreachable node. That being said, there is a MIR an Unreachable TerminatorKind which I think will be useful to handle those cases I describe, however said terminator is not available in the nightly we use. We really have to update :)

lqd commented 8 years ago

Turns out there's also an Assert TerminatorKind in more recent nightlies!

brson commented 8 years ago

LLVM at least is able to mark functions as non-returning. I don't know what binaryen's facilities for unwinding are but it must support it somehow.

Punting on unwinding for quite a while is reasonable. You can get a lot done without it.

lqd commented 8 years ago

@brson indeed, I was just noting those things both dropping and unwinding, as I was looking at the Unreachable validation and its impact on the general panic! impl — probably for another PR! (but my tests indicate this general panic! will be a requirement to compile libcore)