Skip to content

Conversation

@abrown
Copy link
Member

@abrown abrown commented Nov 15, 2019

As discussed in bytecodealliance/cranelift#1226, the context of Cranelift errors is lost after exiting the scope containing the Cranelift function. CodegenError then only contains something like inst2: arg 0 (v4) has type i16x8, expected i8x16, which is rarely enough information for investigating a codegen failure. This change uses Cranelift's pretty_error function to improve the error messages wrapped in CompileError; CompileError has lost the reference to CodegenError due to pretty_error taking ownership but this seems preferable since no backtrace is attached and losing the pretty-printed context would be worse (if CodegenError gains a Backtrace or implements Clone we can revisit this).

As discussed in bytecodealliance/cranelift#1226, the context of Cranelift errors is lost after exiting the scope containing the Cranelift function. `CodegenError` then only contains something like `inst2: arg 0 (v4) has type i16x8, expected i8x16`, which is rarely enough information for investigating a codegen failure. This change uses Cranelift's `pretty_error` function to improve the error messages wrapped in `CompileError`; `CompileError` has lost the reference to `CodegenError` due to `pretty_error` taking ownership but this seems preferable since no backtrace is attached and losing the pretty-printed context would be worse (if `CodegenError` gains a `Backtrace` or implements `Clone` we can revisit this).
Copy link
Member

@sunfishcode sunfishcode left a comment

Choose a reason for hiding this comment

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

Sounds good!

@sunfishcode sunfishcode merged commit ea04aa5 into bytecodealliance:master Nov 16, 2019
@abrown abrown deleted the improve-cranelift-logging branch May 26, 2020 03:18
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