search

iximeow / yaxpeax-x86

x86 decoders for the yaxpeax project
BSD Zero Clause License
129 stars 23 forks source link

`Prefixes::cs` vs `Prefixes::set_cs` is probably a very weird typo #28

Closed meithecatte closed 1 year ago

meithecatte commented 1 year ago

In all three copies of the Prefixes struct, cs is a public copy of set_cs, with the signature fn(&mut self), while all the other segment registers have functions fn(&self) -> bool. I highly doubt that this is intentional, and if it is, it deserves a doc-comment.

iximeow commented 1 year ago

that is extremely a bug, yes. oof. and to fix that would be API-breaking and a 2.x bump. so i think this'll have to be a

#[deprecated(since = "forever, probably", note = "use of `pub fn cs()` is incorrect; this function sets segment state rather than queries the current segment")]

that's then removed in an eventual 2.x bump..

at least segment_override_for_op lets users discover the correct segment, assuming cs() wasn't called :confused:

meithecatte commented 1 year ago

The good thing is that cs returns (), so attempting to use it to check the segment will cause a type error.

iximeow commented 1 year ago

oop, as github remarks, https://github.com/iximeow/yaxpeax-x86/commit/72f8f6677719a8ad7c8e61e4d629f47deef746cc makes this deprecated with a selects_cs that actually does the right thing in the mean time. this'll end up in an eventual 1.16 release. thanks for reporting the issue!

(edit: not only does that commit make Prefixes::cs deprecated, it no-ops the deprecated-in-1.x function.)