Skip to content

Sync main with dev branch#7

Open
MrIndeciso wants to merge 46 commits intomainfrom
dev
Open

Sync main with dev branch#7
MrIndeciso wants to merge 46 commits intomainfrom
dev

Conversation

@MrIndeciso
Copy link
Copy Markdown
Member

@MrIndeciso MrIndeciso commented Apr 1, 2026

Fixes #1.
Fixes #2.
Fixes #5.
Fixes #6.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.

Comment on lines +86 to +108
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
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Comment on lines +86 to +108
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
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Comment on lines +19 to +24
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)
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

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]).

Copilot uses AI. Check for mistakes.
Comment on lines +48 to +79
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
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Comment on lines +16 to +23
# 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):
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Comment on lines +9 to +16
```python
from libdestruct import struct, c_int, ptr, inflater

class data_t(struct):
value: c_int
next: ptr[c_int]
```

Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Comment on lines +38 to +52
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
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

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).

Copilot uses AI. Check for mistakes.
Comment on lines +125 to +127
s = lib.inflate(c_str, 0)
print(s.value) # "Hello"
print(len(s)) # 5
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
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"

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.

Comment on lines 28 to +35
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()

Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Comment on lines +103 to +109
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()
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

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).

Copilot uses AI. Check for mistakes.
Comment on lines +258 to +264
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)
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Comment on lines +34 to +35
class pixel_t(struct):
color: enum_of(Color, c_int)
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

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).

Suggested change
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)

Copilot uses AI. Check for mistakes.
Comment on lines +24 to +28
from libdestruct import struct, c_int, ptr_to_self

class Node(struct):
val: c_int
next: ptr_to_self
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

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"].

Suggested change
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()

Copilot uses AI. Check for mistakes.
Comment on lines +88 to +89
size_of(array[c_int, 10]) # 40
size_of(array_of(c_int, 10)) # 40 (legacy syntax)
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
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

Copilot uses AI. Check for mistakes.
Comment on lines +59 to +64
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

Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.

Comment on lines +22 to +26
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__}")

Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Comment on lines +13 to +14
import libdestruct.common.union.tagged_union_field_inflater
import libdestruct.common.union.union_field_inflater # noqa: F401
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
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.
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
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.

Copilot uses AI. Check for mistakes.
| `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 |
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
| `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 |

Copilot uses AI. Check for mistakes.
Comment on lines +28 to +31
### 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:

Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

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)).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.

Comment on lines +28 to +34
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}")
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

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)).

Copilot uses AI. Check for mistakes.
Comment on lines +140 to +146
# 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
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Comment on lines +11 to +14
__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
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants