Skip to content
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

Add traceback to VmException #657

Merged
merged 25 commits into from
Dec 28, 2022
Merged

Add traceback to VmException #657

merged 25 commits into from
Dec 28, 2022

Conversation

fmoletta
Copy link
Contributor

@fmoletta fmoletta commented Dec 22, 2022

  • Add the traceback field to VmException
  • Add logic necessary to construct the traceback
  • Add tests for new/changed methods
  • Add changelog
    The following will be added in a later PR:
  • Improving the function get_error_attr_value by adding the method substitute_error_message_references.

This PR will not add/process the following information:

  • hints field of debug_info.inst_location
  • input_file contents
  • notes
    This may be added in the future in terms of priority

@fmoletta fmoletta marked this pull request as ready for review December 22, 2022 22:13
Copy link
Contributor

@Oppen Oppen left a comment

Choose a reason for hiding this comment

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

Few minor comments but should be good to ship.

src/types/instruction.rs Show resolved Hide resolved
@@ -78,3 +81,38 @@ impl Instruction {
}
}
}

// Returns True if the given instruction looks like a call instruction.
pub(crate) fn is_call_instruction(encoded_instruction: &BigInt, imm: Option<BigInt>) -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is encoded_instruction a BigInt?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use an Option<&BigInt> for the immediate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

encoded_instruction was left as a BigInt in order to reduce the complexity of get_traceback_entries. is_call_instruction is only used inside get_traceback_entries., which is on the longer side, so it was a bit more convenient to have the type conversion inside the simpler method is_call_instruction

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added reference to immediate argument in is_call_instruction and decode_instruction

src/vm/errors/vm_exception.rs Outdated Show resolved Hide resolved
src/vm/errors/vm_exception.rs Show resolved Hide resolved
@fmoletta fmoletta changed the title Add traceback to VmExceptiom Add traceback to VmException Dec 27, 2022
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@fmoletta fmoletta requested a review from Oppen December 28, 2022 16:45
@fmoletta fmoletta merged commit 7d0f69b into main Dec 28, 2022
@fmoletta fmoletta deleted the traceback branch December 28, 2022 18:22
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.

3 participants