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

Move entryIDs into separate data files #1585

Merged
merged 3 commits into from Oct 17, 2023
Merged

Conversation

hainest
Copy link
Contributor

@hainest hainest commented Oct 15, 2023

This will make it much easier to update them as we migrate to Capstone.

This is a very poor solution, but is the only means of splitting the architectures apart while maintaining the existing entryID enum usage. I tried several different means of making this better, but the only one that worked well requires C++20's using enum.

@bbiiggppiigg How much effort is it to modify your generator scripts to accommodate the new locations and file formats?

This will make it much easier to update them as we migrate to Capstone.

This is a very poor solution, but is the only means of splitting the
architectures apart while maintaining the existing entryID enum usage.
@hainest hainest added PowerPC ARMv8 code cleanup Bring the code up to modern standards or remove deprecated features GPUs This issue is related to handling of GPU code x86_64 Related to x86 family of ISAs labels Oct 15, 2023
@hainest hainest self-assigned this Oct 15, 2023
@hainest hainest mentioned this pull request Oct 15, 2023
@bbiiggppiigg
Copy link
Collaborator

@hainest
I generate the file by script and copy them into Dyninst manually.
So I assume there isn't any additional effort if we are just putting them in a different location.

Copy link
Contributor

@kupsch kupsch left a comment

Choose a reason for hiding this comment

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

OK, but two things

  1. common/h/mnemonics/x86 is missing a NL as its last character
  2. should the names of the file end in .h (perhaps _entryIds.h or _entryId_enums.h) to indicate that they contain C/C++? This would help with editors and other tools that determine the file type by suffix.

@hainest
Copy link
Contributor Author

hainest commented Oct 16, 2023

OK, but two things

1. `common/h/mnemonics/x86` is missing a NL as its last character

Fixed.

2. should the names of the file end in `.h` (perhaps `_entryIds.h` or `_entryId_enums.h`) to indicate that they contain C/C++? This would help with editors and other tools that determine the file type by suffix.

I didn't give them file extensions since they are supposed to be just data files. They also don't contain any valid syntax. EclipseCDT can read them as enum values because it does a full parse when it indexes files, but I guess I was biased by that. Is your editor able to see them like that?

@kupsch
Copy link
Contributor

kupsch commented Oct 16, 2023

Vim's syntax highlighting for enum values is the same as text, so hard to tell. Line counting tools like cloc ignore these files without extensions using the default settings.

@bbiiggppiigg
Copy link
Collaborator

I'm not sure but a file extension like .include.h or .include.data
or maybe change the directory name from mnemonic to something like entryID.include
might give people a better understanding of what these files are or where there are used?

This gives the reader a better idea of what they are and lets tools like
cloc know that they correspond to a language.
@hainest
Copy link
Contributor Author

hainest commented Oct 16, 2023

@kupsch @bbiiggppiigg I updated the file suffixes. Does that work for you both?

@kupsch
Copy link
Contributor

kupsch commented Oct 16, 2023

Looks good.

@bbiiggppiigg
Copy link
Collaborator

Looks good to me.

@hainest hainest merged commit 077d65c into master Oct 17, 2023
5 checks passed
@hainest hainest deleted the thaines/refactor_entryID branch October 17, 2023 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ARMv8 code cleanup Bring the code up to modern standards or remove deprecated features GPUs This issue is related to handling of GPU code PowerPC x86_64 Related to x86 family of ISAs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants