-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unsupported opcode fallback #22
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm aside from typo.
it would be nice to make this configurable by a command-line argument, in case one wanted to see the stack trace from failed instructions. but this could come later.
Co-authored-by: Kait Lam <[email protected]>
it occurs to me that this is better placed within aslp, especially since it introduces a new kind of intrinsic. but this can be merged for now. |
Maybe it would be better to use a distinct JSON structure to represent the error, instead of embedding it as a fake ASL statement. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"use a distinct JSON structure to represent the error, instead of embedding it as a fake ASL statement".
Probably implement as a map containing the same information (debug gts output):
|
some module renaming because every library we import shadows Result
lgtm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
forecast looks good
Emit a
Stmt_TCall("unsupported_opcode", Expr_IntLiteral(opcode))
on lift failures rather than crashing. This occurs currently due to svc, ldaxr, and mrs instructionsAlso fixed dune config for new release of
ocaml_protoc_plugin