CodegenError should have optional context, rather than pretty-ifying it into a string

[fitzgen]

pretty_error should be replaced with methods on CodegenError that mutate the error and provide the context that pretty_error otherwise adds, and the CodegenError's Display impl should print what pretty_error creates when it has the necessary context.

These context-adding methods would be similar to how the wast crate adds file contents and filename context to errors with these methods:

• https://docs.rs/wast/24.0.0/wast/struct.Error.html#method.set_text
• https://docs.rs/wast/24.0.0/wast/struct.Error.html#method.set_path

Then we can delete pretty_error, and avoid using string as an error type in more places (notably in cranelift-filetests and its pretty_anyhow_error workaround).

[github-actions]

Subscribe to Label Action

cc @bnjbvr

This issue or pull request has been labeled: "cranelift"

Thus the following users have been cc'd because of the following labels:

• bnjbvr: cranelift

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

[bjorn3]

pretty_error takes a reference to isa and func. Doing this would require CodegenError to borrow from them.

[fitzgen]

The idea is to take copies of the data gathered from those things and store them into the error, rather than borrow those things.

[bjorn3]

pretty_error needs the entire function, as the error shows the full function source. From the target isa it needs the register_info and encoding_info.

[taegyunkim]

I'd like to take this issue. Is anyone looking into this?

[fitzgen]

It's all you @taegyunkim!

[maekawatoshiki]

Hi, is this issue still open? I'm looking for issues labeled E-easy.

[jameysharp]

This issue is still relevant, but looking over the past discussion and the implementation in cranelift/codegen/src/print_errors.rs, I'm not convinced it should be labeled "easy". You're welcome to give it a shot anyway and we'll help however we can!