Conversation
This propagates down the chain, and everything works big-endian
There was a problem hiding this comment.
Pull request overview
Syncs main with dev, bringing in major core-library features (endianness, alignment, unions/bitfields, forward refs, hexdump, dict export) plus a large set of new tests and docs.
Changes:
- Add endianness support across inflater/resolvers and primitive types (incl. float/double).
- Implement/extend core type features: struct alignment + explicit offsets, unions (plain/tagged), bitfields, pointer arithmetic + caching, hexdump/to_dict, and forward-reference pointer syntax.
- Add extensive pytest/unittest coverage and introduce MkDocs documentation structure/config.
Reviewed changes
Copilot reviewed 74 out of 74 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| test/scripts/types_unit_test.py | Adds unit tests for primitives, ptr behavior, floats, c_str, hexdump, comparisons |
| test/scripts/to_dict_test.py | Adds to_dict() behavior tests (primitive/struct/array/union/enum) |
| test/scripts/tagged_union_test.py | Adds tests for tagged/plain unions and freeze/diff/reset |
| test/scripts/struct_unit_test.py | Adds tests for struct collisions, freeze semantics, forward refs, equality |
| test/scripts/struct_parser_unit_test.py | Adds tests for C parser pointers/arrays/typedefs inflation |
| test/scripts/resolver_unit_test.py | Adds tests for FakeResolver/MemoryResolver bytes behavior and writes |
| test/scripts/enum_test.py | Extends enum tests (to_str formatting, bytes behavior, value extraction) |
| test/scripts/endianness_test.py | Adds endianness test suite (ints/floats/structs/ptrs/arrays/bitfields) |
| test/scripts/bitfield_unit_test.py | Adds bitfield tests (packing, signed, parser support, freeze) |
| test/scripts/basic_struct_test.py | Adds forward-ref ptr syntax test in existing suite |
| test/scripts/array_unit_test.py | Adds array unit tests (value/get, iteration, to_bytes/to_dict) |
| test/scripts/alignment_test.py | Adds struct alignment/offset/array/bitfield alignment tests |
| SKILL.md | Adds/updates skills-style reference documentation |
| README.md | Expands README with installation + quick-start examples |
| pyproject.toml | Updates Ruff ignore list (adds COM812) |
| mkdocs.yml | Adds MkDocs Material configuration + navigation |
| libdestruct/libdestruct.py | Adds endianness parameter to inflater()/inflate() |
| libdestruct/common/utils.py | Expands size_of(), adds alignment_of(), and resolves string annotations |
| libdestruct/common/union/union.py | Introduces union runtime type (plain/tagged) with freeze/diff/to_bytes |
| libdestruct/common/union/union_of.py | Adds union_of() field factory |
| libdestruct/common/union/union_field.py | Adds UnionField descriptor and size computation |
| libdestruct/common/union/union_field_inflater.py | Registers union field inflater (inflate all variants) |
| libdestruct/common/union/tagged_union_of.py | Adds tagged_union() field factory |
| libdestruct/common/union/tagged_union_field.py | Adds TaggedUnionField descriptor and size computation |
| libdestruct/common/union/tagged_union_field_inflater.py | Inflates tagged union variant based on discriminator member |
| libdestruct/common/union/init.py | Exposes union APIs and registers inflaters |
| libdestruct/common/type_registry.py | Adds support for subscripted generics via generic handlers |
| libdestruct/common/struct/struct.py | Adds endianness to struct.from_bytes() and freezes result |
| libdestruct/common/struct/struct_impl.py | Adds member-collision handling, alignment, padding-aware to_bytes, to_dict, hexdump, eq semantics |
| libdestruct/common/struct/init.py | Reorders __all__ exports |
| libdestruct/common/ptr/ptr.py | Adds pointer caching, arithmetic, indexing; supports endianness via Resolver |
| libdestruct/common/obj.py | Adds generic typing, endianness propagation, comparisons, to_dict(), hexdump(), endianness-aware from_bytes |
| libdestruct/common/inflater.py | Adds endianness plumbing into MemoryResolver creation |
| libdestruct/common/hexdump.py | Adds hexdump formatter utility |
| libdestruct/common/forward_ref_inflater.py | Adds lazy forward-ref ptr inflater + generic ptr[T] handling |
| libdestruct/common/enum/enum.py | Fixes enum to_str() indentation behavior |
| libdestruct/common/bitfield/bitfield.py | Adds bitfield runtime type with packing + signed extraction |
| libdestruct/common/bitfield/bitfield_tracker.py | Adds struct inflation helper for grouping bitfields |
| libdestruct/common/bitfield/bitfield_of.py | Adds bitfield_of() factory with validation |
| libdestruct/common/bitfield/bitfield_field.py | Adds BitfieldField descriptor |
| libdestruct/common/bitfield/init.py | Exposes bitfield APIs |
| libdestruct/common/array/array.py | Updates array get() signature to allow “all elements” default |
| libdestruct/common/array/array_impl.py | Implements get(-1) and adds array to_dict() |
| libdestruct/common/init.py | Ensures forward-ref inflater registration on import |
| libdestruct/c/struct_parser.py | Adds bitfield parsing, nested pointer typedef support, native float/double mapping, cache clearing |
| libdestruct/c/ctypes_generic.py | Fixes frozen to_bytes behavior for ctypes generics |
| libdestruct/c/c_str.py | Fixes get index logic, adds __setitem__ |
| libdestruct/c/c_float_types.py | Adds native float/double implementations with endianness |
| libdestruct/c/base_type_inflater.py | Registers float/double types in type registry |
| libdestruct/c/init.py | Exposes float/double types |
| libdestruct/backing/resolver.py | Adds endianness attribute to Resolver base |
| libdestruct/backing/memory_resolver.py | Adds endianness propagation + ensures resolve returns bytes |
| libdestruct/backing/fake_resolver.py | Adds endianness propagation + adjusts default page reads |
| libdestruct/init.py | Exposes new APIs (endianness helpers, unions, bitfields, alignment_of, more C types) |
| docs/memory/resolvers.md | Adds resolver documentation |
| docs/memory/inflater.md | Adds inflater documentation |
| docs/index.md | Adds docs landing page |
| docs/basics/types.md | Adds primitive types documentation |
| docs/basics/structs.md | Adds struct documentation |
| docs/basics/pointers.md | Adds pointer documentation |
| docs/basics/getting_started.md | Adds getting started guide |
| docs/basics/enums.md | Adds enum documentation |
| docs/basics/arrays.md | Adds array documentation |
| docs/advanced/tagged_unions.md | Adds union documentation |
| docs/advanced/offset.md | Adds explicit offset documentation |
| docs/advanced/hexdump.md | Adds hexdump documentation |
| docs/advanced/freeze_diff.md | Adds freeze/diff/reset documentation |
| docs/advanced/forward_refs.md | Adds forward reference documentation |
| docs/advanced/c_parser.md | Adds C parser documentation |
| docs/advanced/bitfields.md | Adds bitfield documentation |
| docs/advanced/alignment.md | Adds alignment documentation |
| .github/workflows/test.yml | Expands CI Python version matrix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
libdestruct/common/ptr/ptr.py
Outdated
| def unwrap(self: ptr, length: int | None = None) -> obj: | ||
| """Return the object pointed to by the pointer. | ||
|
|
||
| Args: | ||
| length: The length of the object in memory this points to. | ||
| """ | ||
| if self._cache_valid: | ||
| return self._cached_unwrap | ||
|
|
||
| address = self.get() | ||
|
|
||
| if self.wrapper: | ||
| if length: | ||
| raise ValueError("Length is not supported when unwrapping a pointer to a wrapper object.") | ||
|
|
||
| return self.wrapper(self.resolver.absolute_from_own(address)) | ||
|
|
||
| if not length: | ||
| length = 1 | ||
| result = self.wrapper(self.resolver.absolute_from_own(address)) | ||
| else: | ||
| target_resolver = self.resolver.absolute_from_own(address) | ||
| result = target_resolver.resolve(length or 1, 0) | ||
|
|
||
| return self.resolver.resolve(length, 0) | ||
| self._cached_unwrap = result | ||
| self._cache_valid = True | ||
| return result |
There was a problem hiding this comment.
ptr.unwrap()/try_unwrap() caching ignores the length parameter for untyped pointers. If a caller unwraps the same pointer with different lengths, the cached value can return an incorrectly-sized byte string. Consider keying the cache by length (and wrapper), or disabling caching when wrapper is None and length is provided.
libdestruct/common/ptr/ptr.py
Outdated
| def unwrap(self: ptr, length: int | None = None) -> obj: | ||
| """Return the object pointed to by the pointer. | ||
|
|
||
| Args: | ||
| length: The length of the object in memory this points to. | ||
| """ | ||
| if self._cache_valid: | ||
| return self._cached_unwrap | ||
|
|
||
| address = self.get() | ||
|
|
||
| if self.wrapper: | ||
| if length: | ||
| raise ValueError("Length is not supported when unwrapping a pointer to a wrapper object.") | ||
|
|
||
| return self.wrapper(self.resolver.absolute_from_own(address)) | ||
|
|
||
| if not length: | ||
| length = 1 | ||
| result = self.wrapper(self.resolver.absolute_from_own(address)) | ||
| else: | ||
| target_resolver = self.resolver.absolute_from_own(address) | ||
| result = target_resolver.resolve(length or 1, 0) | ||
|
|
||
| return self.resolver.resolve(length, 0) | ||
| self._cached_unwrap = result | ||
| self._cache_valid = True | ||
| return result |
There was a problem hiding this comment.
ptr.unwrap() is annotated to return obj, but in the untyped case it returns bytes and stores that into _cached_unwrap (typed as obj | None). This breaks type expectations for callers and static typing. Consider widening the return type to obj | bytes (and the cache type accordingly), or always wrap raw bytes in an obj type.
libdestruct/libdestruct.py
Outdated
| def inflater(memory: Sequence, endianness: str = "little") -> Inflater: | ||
| """Return a TypeInflater instance.""" | ||
| if not isinstance(memory, Sequence): | ||
| raise TypeError(f"memory must be a MutableSequence, not {type(memory).__name__}") | ||
|
|
||
| return Inflater(memory) | ||
| return Inflater(memory, endianness=endianness) |
There was a problem hiding this comment.
The error message says "memory must be a MutableSequence" but the check accepts any Sequence. Either validate MutableSequence (if mutability is required) or update the error message/type hints to match the actual accepted types (e.g., Sequence[SupportsBytes]).
| All resolvers implement these methods: | ||
|
|
||
| | Method | Description | | ||
| |---|---| | ||
| | `resolve(size, offset)` | Read `size` bytes starting at the resolved address + offset | | ||
| | `resolve_address()` | Return the absolute address of this resolver | | ||
| | `write(data)` | Write bytes at the resolved address | | ||
| | `relative_from_own(offset, size)` | Create a child resolver at a relative offset | | ||
| | `absolute_from_own(address)` | Create a child resolver at an absolute address | | ||
|
|
||
| ## Custom Memory Backends | ||
|
|
||
| If you need to read from a custom source (e.g., a debugger's memory API, a remote process, a file), you can subclass `Resolver`: | ||
|
|
||
| ```python | ||
| from libdestruct.backing.resolver import Resolver | ||
|
|
||
| class DebuggerResolver(Resolver): | ||
| def __init__(self, debugger, address): | ||
| self.debugger = debugger | ||
| self._address = address | ||
|
|
||
| def resolve(self, size, offset=0): | ||
| return self.debugger.read_memory(self._address + offset, size) | ||
|
|
||
| def resolve_address(self): | ||
| return self._address | ||
|
|
||
| def write(self, data, offset=0): | ||
| self.debugger.write_memory(self._address + offset, data) | ||
|
|
||
| # ... implement relative_from_own, absolute_from_own |
There was a problem hiding this comment.
docs/memory/resolvers.md describes a write() method and shows an example implementation, but the Resolver interface defines modify(size, index, value) instead. Please update the docs (method table + example) to match the actual Resolver API, including the index/offset parameter conventions.
| # Subscript syntax (preferred) | ||
| class pixel_t(struct): | ||
| color: enum[Color] # defaults to c_int backing type | ||
| x: c_int | ||
| y: c_int | ||
|
|
||
| # With a custom backing type: | ||
| class pixel2_t(struct): |
There was a problem hiding this comment.
This example uses enum_of(Color, c_int) and color: enum_of(...), but enum_of actually takes (enum_type, lenient=True, size=4) and the field is typically annotated as enum with enum_of(Color) as the default value (see libdestruct/common/enum/enum_of.py). Update the snippet to reflect the real API so it is valid Python and matches behavior.
| ```python | ||
| from libdestruct import struct, c_int, ptr, inflater | ||
|
|
||
| class data_t(struct): | ||
| value: c_int | ||
| next: ptr[c_int] | ||
| ``` | ||
|
|
There was a problem hiding this comment.
The pointer field example uses next: ptr_to(c_int) as a type annotation, but ptr_to() returns a field descriptor that should be used as the default value (e.g., next: ptr = ptr_to(c_int)). As written, the snippet is not valid/idiomatic for this library’s API.
| import struct as pystruct | ||
| memory[0:4] = pystruct.pack("<i", 42) | ||
| memory[4:12] = pystruct.pack("<q", 12) # next -> offset 12 | ||
| memory[12:16] = pystruct.pack("<i", 99) # value at offset 12 | ||
|
|
||
| data = lib.inflate(data_t, 0) | ||
| print(data.value.value) # 42 | ||
| print(data.next.unwrap().value) # 99 | ||
| ``` | ||
|
|
||
| ### Safe Dereferencing | ||
|
|
||
| Use `try_unwrap()` for null-safe pointer access. It returns `None` if the pointer is null (0): | ||
|
|
||
| ```python |
There was a problem hiding this comment.
The try_unwrap() section states it returns None when the pointer is null (0), but the implementation only checks whether the address is resolvable; address 0 can be valid in a buffer-backed resolver. Consider rewording this to “returns None if the address is invalid/unresolvable” (or implement explicit null-pointer semantics if that’s desired).
| s = lib.inflate(c_str, 0) | ||
| print(s.value) # "Hello" | ||
| print(len(s)) # 5 |
There was a problem hiding this comment.
This c_str example implies .value is a Python string and s[0] returns an int (72), but c_str.get()/__getitem__ return bytes (e.g., b"H") and .value returns bytes as well. Please update the example outputs/types to match the actual behavior.
| s = lib.inflate(c_str, 0) | |
| print(s.value) # "Hello" | |
| print(len(s)) # 5 | |
| print(s.value) # b"Hello" | |
| print(len(s)) # 5 | |
| print(s[0]) # b"H" |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 77 out of 77 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def size_of(item_or_inflater: obj | callable[[Resolver], obj]) -> int: | ||
| """Return the size of an object, from an obj or it's inflater.""" | ||
| if hasattr(item_or_inflater.__class__, "size"): | ||
| # This has the priority over the size of the object itself | ||
| # as we might be dealing with a struct object | ||
| # that defines an attribute named "size" | ||
| """Return the size in bytes of a type, instance, or field descriptor.""" | ||
| # Field instances (e.g. array_of, ptr_to) — must come before .size check | ||
| if isinstance(item_or_inflater, Field): | ||
| return item_or_inflater.get_size() | ||
| if is_field_bound_method(item_or_inflater): | ||
| return item_or_inflater.__self__.get_size() | ||
|
|
There was a problem hiding this comment.
size_of() currently doesn’t handle subscripted “generic” types produced by the new API (e.g. array[c_int, 10], enum[Color], ptr[T]). Those expressions create a GenericAlias/typing object rather than a class/Field, so size_of(array[c_int, 10]) raises ValueError, contradicting the new documentation and making direct size queries for these types fail. Consider detecting typing.get_origin()/get_args() (or types.GenericAlias) here and routing through TypeRegistry().inflater_for(...) to obtain a Field-bound inflater whose size can be computed, so size_of works for both legacy factories and subscript syntax.
libdestruct/common/union/union.py
Outdated
| def reset(self: union) -> None: | ||
| """Reset the union to its frozen value.""" | ||
| if self._variant is not None: | ||
| self._variant.reset() | ||
| else: | ||
| for v in self._variants.values(): | ||
| v.reset() |
There was a problem hiding this comment.
union.reset() does not restore the frozen union-wide byte region captured in freeze(). For tagged unions this means padding bytes (outside the active variant) won’t be reset; for plain unions, iterating over variants and resetting each can write overlapping memory multiple times and can yield incorrect final bytes depending on variant sizes/order. A safer approach is to, when frozen and resolver is present, write back _frozen_bytes for the full union size (and optionally then reset the active variant view).
| def hexdump(self: struct_impl) -> str: | ||
| """Return a hex dump of this struct's bytes with field annotations.""" | ||
| member_offsets = object.__getattribute__(self, "_member_offsets") | ||
| members = object.__getattribute__(self, "_members") | ||
| annotations = {member_offsets[name]: name for name in members} | ||
| address = struct_impl.address.fget(self) if not object.__getattribute__(self, "_frozen") else 0 | ||
| return format_hexdump(self.to_bytes(), address, annotations) |
There was a problem hiding this comment.
struct_impl.hexdump() builds annotations as a dict keyed by byte offset, so if multiple members share the same offset (e.g., consecutive bitfields in the same backing integer), earlier names are overwritten and the hexdump will only show one of them. Consider storing a list of names per offset (or emitting group-owner-only offsets) and adapting format_hexdump to display all field names that start at a given offset.
docs/basics/enums.md
Outdated
| class pixel_t(struct): | ||
| color: enum_of(Color, c_int) |
There was a problem hiding this comment.
This legacy example is incorrect: enum_of()’s second parameter is lenient (bool), not a backing type. Passing c_int here will be treated as truthy and won’t set the backing size/type. The example should either use the preferred enum[Color] / enum[Color, c_short] syntax, or use enum_of(Color, size=4) (and document how to select alternate backing sizes/types).
| class pixel_t(struct): | |
| color: enum_of(Color, c_int) | |
| # Legacy factory syntax. Use the size (in bytes) to select the backing type. | |
| # For example, size=2 => 16-bit backing; size=4 => 32-bit backing. | |
| class pixel_t(struct): | |
| color: enum_of(Color, size=4) |
docs/advanced/forward_refs.md
Outdated
| from libdestruct import struct, c_int, ptr_to_self | ||
|
|
||
| class Node(struct): | ||
| val: c_int | ||
| next: ptr_to_self |
There was a problem hiding this comment.
This legacy ptr_to_self example is invalid usage: ptr_to_self is a factory function that must be called and assigned as a field default (e.g., next: ptr = ptr_to_self()), not used as a bare annotation. As written it won’t create a pointer field descriptor and won’t behave like ptr["Node"].
| from libdestruct import struct, c_int, ptr_to_self | |
| class Node(struct): | |
| val: c_int | |
| next: ptr_to_self | |
| from libdestruct import struct, c_int, ptr, ptr_to_self | |
| class Node(struct): | |
| val: c_int | |
| next: ptr = ptr_to_self() |
| size_of(array[c_int, 10]) # 40 | ||
| size_of(array_of(c_int, 10)) # 40 (legacy syntax) |
There was a problem hiding this comment.
The documentation claims size_of(array[c_int, 10]) works, but size_of() currently does not handle subscripted GenericAlias types (it raises ValueError). Either update size_of() to support the subscript syntax types directly, or adjust this example to use array_of(c_int, 10) until support is added.
| size_of(array[c_int, 10]) # 40 | |
| size_of(array_of(c_int, 10)) # 40 (legacy syntax) | |
| size_of(array_of(c_int, 10)) # 40 |
test/scripts/to_dict_test.py
Outdated
| def test_struct_with_ptr_to_dict(self): | ||
| """Pointer field returns its address as int.""" | ||
| class data_t(struct): | ||
| value: c_int | ||
| ref: c_long | ||
|
|
There was a problem hiding this comment.
The test name/docstring says this is a pointer field, but the struct defines ref: c_long (a plain integer). Either change the field to an actual ptr/ptr_to(...) field, or update the docstring to reflect that this test is about serializing raw address-like integers rather than pointer descriptors.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 77 out of 77 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
libdestruct/libdestruct.py
Outdated
| def inflater(memory: Sequence, endianness: str = "little") -> Inflater: | ||
| """Return a TypeInflater instance.""" | ||
| if not isinstance(memory, Sequence): | ||
| raise TypeError(f"memory must be a MutableSequence, not {type(memory).__name__}") | ||
| raise TypeError(f"memory must be a Sequence, not {type(memory).__name__}") | ||
|
|
There was a problem hiding this comment.
inflater() now accepts any Sequence, which includes str. Passing a string will pass this check but then fail later in MemoryResolver.resolve() when bytes(self.memory[...]) is called. Consider restricting memory to bytes-like buffers (e.g., bytes, bytearray, memoryview) or explicitly rejecting str/non-bytes sequences to fail fast with a clear error.
| import libdestruct.common.union.tagged_union_field_inflater | ||
| import libdestruct.common.union.union_field_inflater # noqa: F401 |
There was a problem hiding this comment.
This import is only for side effects (handler registration) but lacks # noqa: F401, so Ruff will flag it as an unused import. Add # noqa: F401 (as done for union_field_inflater) or otherwise reference it explicitly.
| This means the referenced type must be defined before the struct is inflated, but not necessarily before it is declared. | ||
|
|
||
| !!! info | ||
| Forward references are resolved through the `TypeRegistry` at inflation time. If the referenced type is not found, an error is raised. |
There was a problem hiding this comment.
The note says an error is raised if a forward-referenced type is not found, but the current implementation for ptr["TypeName"] uses lazy resolution and can silently fall back to an untyped ptr when the reference can’t be resolved. Please either update the docs to match the behavior or tighten the implementation to raise a clear error when the forward ref can’t be resolved.
| Forward references are resolved through the `TypeRegistry` at inflation time. If the referenced type is not found, an error is raised. | |
| Forward references are resolved through the `TypeRegistry` at inflation time using lazy resolution. | |
| If the referenced type name cannot be resolved at that point, libdestruct falls back to an untyped/generic `ptr` | |
| rather than raising an immediate error. Be aware that misspelled or missing type names may therefore not fail fast | |
| and can instead surface later as unexpected runtime behavior. |
docs/memory/resolvers.md
Outdated
| | `resolve(size, offset)` | Read `size` bytes starting at the resolved address + offset | | ||
| | `resolve_address()` | Return the absolute address of this resolver | | ||
| | `modify(size, index, value)` | Write `value` bytes at the resolved address + index | | ||
| | `relative_from_own(offset, size)` | Create a child resolver at a relative offset | |
There was a problem hiding this comment.
The documented relative_from_own(offset, size) signature doesn’t match the actual Resolver.relative_from_own(address_offset, index_offset) interface used in the codebase. Update the docs to reflect the correct parameter meaning/name to avoid confusion for custom resolver implementers.
| | `relative_from_own(offset, size)` | Create a child resolver at a relative offset | | |
| | `relative_from_own(address_offset, index_offset)` | Create a child resolver at a relative address and index offset | |
| ### FakeResolver | ||
|
|
||
| A resolver backed by a simulated 4KB memory page, used internally when you create structs via keyword arguments or `from_bytes()` without explicit memory: | ||
|
|
There was a problem hiding this comment.
This section states that FakeResolver is used for from_bytes() without explicit memory, but obj.from_bytes() / struct.from_bytes() currently build an Inflater over the provided bytes and therefore use MemoryResolver (not FakeResolver). Consider updating the docs to mention only the kwargs/no-buffer construction path (e.g., test_t(a=42)).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 98 out of 98 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def inflater(memory: Sequence | mmap.mmap, endianness: str = "little") -> Inflater: | ||
| """Return a TypeInflater instance.""" | ||
| if not isinstance(memory, Sequence): | ||
| raise TypeError(f"memory must be a MutableSequence, not {type(memory).__name__}") | ||
| if not isinstance(memory, Sequence | mmap.mmap): | ||
| raise TypeError(f"memory must be a Sequence, not {type(memory).__name__}") | ||
|
|
||
| if endianness not in _VALID_ENDIANNESS: | ||
| raise ValueError(f"endianness must be 'little' or 'big', not {endianness!r}") |
There was a problem hiding this comment.
isinstance() does not accept a PEP-604 union (Sequence | mmap.mmap) as its second argument. This will raise TypeError at runtime on all supported Python versions. Use a tuple of types instead (e.g., (Sequence, mmap.mmap)).
| # Guard against incompatible value types (e.g. int vs str from struct.get()) | ||
| if type(self_val) is not type(other_val) and not isinstance(self_val, type(other_val)) and not isinstance(other_val, type(self_val)): | ||
| return None | ||
| return self_val, other_val | ||
| if isinstance(other, int | float | bytes): | ||
| return self_val, other | ||
| return None |
There was a problem hiding this comment.
isinstance(other, int | float | bytes) will raise TypeError because isinstance() expects a type or tuple of types, not a PEP-604 union. This breaks all comparison operators that call _compare_value. Use isinstance(other, (int, float, bytes)) instead.
| __all__ = ["array", "array_of", "vla_of"] | ||
|
|
||
| import libdestruct.common.array.array_field_inflater # noqa: F401 | ||
| import libdestruct.common.array.array_field_inflater | ||
| import libdestruct.common.array.vla_field_inflater # noqa: F401 — side-effect: registers VLA handler |
There was a problem hiding this comment.
This import is only used for its side effects (handler registration) but is currently unannotated. ruff check (run in CI) will flag it as F401 unused import. Add # noqa: F401 (or assign to _) to document the intent and satisfy the linter.
Fixes #1.
Fixes #2.
Fixes #5.
Fixes #6.