Implementation of bit fields. #2
Conversation
structures.py
Outdated
| self.fields = OrderedDict() | ||
| for field in map(str.strip, spec.split(',')): | ||
| name, length = field.split(':') | ||
| self.fields[name] = int(length) |
There was a problem hiding this comment.
I guess, you expect here only positive integers and field spec like foo:-1 is invalid.
| super().__init__() | ||
| self.spec = spec | ||
| self._embedded = embedded | ||
| self.fields = OrderedDict() |
There was a problem hiding this comment.
I'm not familiar with Construct design, but why spec and fields are public and embedded is not?
There was a problem hiding this comment.
Because _embedded field is really not a part of public interface.
Since one construct can wrap another (e.g. Adapted(SomeStruct(embedded=True))), I need a way to preserve this flag in wrapping constructs. _embedded is how I preserve it nowadays - but it may change in future.
| def _sizeof(self, context): | ||
| return self._length | ||
|
|
||
| def _repr(self): |
There was a problem hiding this comment.
Why you use own private methods for magic analogues? Just curious.
There was a problem hiding this comment.
Mostly because of forward compatibility.
In future I may want to extend the default __repr__ behavior (e.g. also show construct names if they're defined as a struct field).
There's also another (very minor) semantic reason: currently a Construct subclass must implement exactly 4 library-defined methods. Not 3 library-defined methods and one general-purpose dunder method.
I have ...
I'd like to propose a change: introduce bit fields support.
I included an example of my change according to the contributing guide.