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

Figure out how to name the wrapper type of xed_decoded_inst_t #6

Open
Phantomical opened this issue Feb 21, 2024 · 1 comment
Open

Comments

@Phantomical
Copy link
Member

Most of the types map pretty well to their rust wrapper types. xed_decoded_inst_t, however, seems to be pulling multiple duties under different names which means we should probably spend some time thinking about how it should be named. For now, I've wrapped it as DecodedInst but that ends up being a bit confusing.

XED uses it in two different ways:

  • xed_decoded_inst_t is used in the decoder
  • xed_operand_values_t is another typedef that is used in the encoder

It is not necessarily a clean split between the two either. XED encourages reusing a xed_decoded_inst_t instance for multiple use cases and even a "mostly" pure decode use of the API may end up setting some operands in the xed_decoded_inst_t before actually decoding an instruction into it.

To make things worse, there's also xed_inst_t. It has the much more obvious name but is pretty much never what you want since it is actually constant information about an instruction.

@Phantomical
Copy link
Member Author

Putting some initial ideas out here.

The core type in the XED API is definitely xed_decoded_inst_t. IMO it's probably worth naming it as an Instruction (because that's what people will look for when starting the crate). We could split the xed_operand_values_t role out into some sort of InstructionBuilder while also exposing most of the setters. We would then have to rename Inst to something that better represents what it actually is (constant data about an instruction) but since it's not something that's supposed to be used that frequently I think that's OK.

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

No branches or pull requests

1 participant