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

Remove error macros in the verifier #1248

@abrown

Description

@abrown
  • What is the feature or code improvement you would like to do in Cranelift?

Remove report!, fatal!, and nonfatal! macros from the verifier (cranelift-codegen/src/verifier/mod.rs).

  • What is the value of adding this in Cranelift?

Macros are hard to troubleshoot and sometimes, as in #1226, we need the flexibility to change what is reported on the error.

  • Do you have an implementation plan, and/or ideas for data structures or
    algorithms to use?

I propose we remove the report!, fatal!, and nonfatal! macros and replace them with methods on VerifierErrors. The verifier code already threads a VerifierErrors object through all of the verifier methods so we could make better use of that object.

One good thing about the macros was that they allowed variadic arguments, e.g. fatal!(errors, inst, "there was an error in {}", ebb). Moving to methods on VerifierErrors prevents this shorthand but I believe the alternative is not terrible: errors.fatal(inst, format!("there was an error in {}", ebb)). We can discuss this further in the comments but I would suggest that, though slightly more verbose, makes it easier to work with verifier errors.

  • Have you considered alternative implementations? If so, how are they better
    or worse than your proposal?

We could leave the code as-is.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions