Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Optimizing InstructionInfo access. #1595

Merged
merged 8 commits into from
Jul 12, 2016
Merged

Optimizing InstructionInfo access. #1595

merged 8 commits into from
Jul 12, 2016

Conversation

tomusdrw
Copy link
Collaborator

No description provided.

@tomusdrw tomusdrw added the A0-pleasereview 🤓 Pull request needs code review. label Jul 12, 2016

while reader.position < code.len() {
let instruction = code[reader.position];
reader.position += 1;

let info = instructions::get_info(instruction);
let info = infos[instruction as usize];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Bounds check required?

Copy link
Collaborator Author

@tomusdrw tomusdrw Jul 12, 2016

Choose a reason for hiding this comment

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

instruction is u8 - array is initialized for 0x100 elements, but maybe it will be better to be more explicit?

Copy link
Contributor

Choose a reason for hiding this comment

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

no, the instructions array has 0x100 len and instruction is a u8.

@arkpar arkpar added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Jul 12, 2016
@rphmeier rphmeier merged commit ab44168 into master Jul 12, 2016
@tomusdrw tomusdrw deleted the evm-instructions branch July 13, 2016 18:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants