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

introduce const in instruction and register modules #240

Merged
merged 1 commit into from
Dec 17, 2021

Conversation

i509VCB
Copy link
Contributor

@i509VCB i509VCB commented Dec 17, 2021

Start of the series in #237 but in no way the end of it.

@wtfsck
Copy link
Member

wtfsck commented Dec 17, 2021

When would the decoder ever run in a const context? Since it uses lazy static that should never be possible.

@i509VCB
Copy link
Contributor Author

i509VCB commented Dec 17, 2021

Well the only things const in decoder are related to querying the state of some fields on decoder, the actual construction or decoding can't be const because of lazy static.

@wtfsck
Copy link
Member

wtfsck commented Dec 17, 2021

Yes but AFAIK it's not possible to use those functions in a const context since the whole expression (including the creation of the decoder) must be evaluated in a const context. IOW, this shouldn't even be possible:

let decoder = create it;
const X: Y = decoder.some_const_fn();

Since that's not possible ,there should be no reason to make any decoder fns const.

src/rust/iced-x86/src/instruction.rs Outdated Show resolved Hide resolved
src/rust/iced-x86/src/instruction.rs Outdated Show resolved Hide resolved
@wtfsck
Copy link
Member

wtfsck commented Dec 17, 2021

Start of the series in #237 but in no way the end of it.

Do you plan on pushing more commits or will you create more PRs? If you plan on pushing more commits, you can update the first post saying Closes #237 to it gets auto closed when this gets merged!

@i509VCB
Copy link
Contributor Author

i509VCB commented Dec 17, 2021

Well it's going to be more pull requests in the future. Some of them are going to need higher rust version dependencies while others may need some internal changes.

@wtfsck
Copy link
Member

wtfsck commented Dec 17, 2021

Well it's going to be more pull requests in the future. Some of them are going to need higher rust version dependencies while others may need some internal changes.

I'm not ready to bump the MSRV yet though. I did that not long ago. 1.48.0 was released only a year ago.

@i509VCB
Copy link
Contributor Author

i509VCB commented Dec 17, 2021

I've made sure this builds locally with 1.48

@i509VCB i509VCB changed the title introduce const in decoder, instruction and register modules introduce const in instruction and register modules Dec 17, 2021
src/rust/iced-x86/src/instruction.rs Outdated Show resolved Hide resolved
@wtfsck wtfsck merged commit 1cd7f93 into icedland:master Dec 17, 2021
@wtfsck
Copy link
Member

wtfsck commented Dec 17, 2021

Thanks!

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.

2 participants