Structure does not support inheritance of fields from parent classes

[mikewadsten]

trafficstars

Using the following code:

from suitcase.structure import Structure
from suitcase.fields import UBInt8

class Base(Structure):
    a = UBInt8()
    b = UBInt8()

class Subclass(Base):
    c = UBInt8()

Subclass.from_data(b'\x00')

Intuitively I would expect the from_data call to error out, because we need 3 bytes (for a, b and c). Instead, it works, because the Subclass type is set up to only have the c field.

Because of how StructureMeta works, specifically that it deletes the field names from the class namespace, we cannot just do something like this either:

class Subclass(Base):
    a = Base.a  # AttributeError
    b = Base.b  # AttributeError
    c = UBInt8()

I have a few ideas for how we could fix this.

1. Define StructureMeta.__getattr__ to look up field placeholder by name

This would allow Base.a to work in the example above. However, it's probably beneficial that we remove the names from the type's namespace, to minimize user error. (Plus, inheriting the fields that way would also inherit the field's seqno, which would throw off the ordering, at least on Python 3.5 or below.)

2. Mark fields that we want to leave in the type namespace

This would resolve some of the issues with option 1 (by only leaving things like Base.a that we mark explicitly), but this does not resolve the issue of seqno's.

3. Mark fields that are automatically inherited

Many use cases for inheriting fields from base classes are from just needing to extend the structure, i.e. add one or more fields at the end of the structure definition. In that case, automatically inheriting all fields from the superclass is beneficial. However, there are also use cases for extending the beginning of a structure, or for only inheriting some fields. (The latter is less likely.)

As such, this is not a generalizable solution.

4. Inherit field

Define a new field type called Inherit with the following behaviors:

1. If a field name (string) is specified, we search the structure's base classes for a field placeholder matching that name. We then generate a copy of that placeholder, updating the seqno accordingly.
2. If no field name is specified, we use the name of the Inherit field itself to search as in 1.
3. (Experimental idea) If a field name (string) and Structure class are specified, we copy the specified field placeholder from that class. (This allows "inheriting" field definitions from disparate classes.)

Example:

class Base(Structure):
    a = UBInt8()

class Sub(Base):
    # base_a = Inherit('a')
    a = Inherit()

# Item 3 above - experimental idea, might not do it
class Different(Structure):
    first = UBInt8()
    a = Inherit('a', Base)  # or Inherit(Base, 'a') but that complicates the function signature

---

I am leaning toward option 4. I might investigate implementing this later today.

[mikewadsten]

So, I spent an hour or two today on this.

The work it took to implement this helped me to remember exactly how the Structure DSL works, and made me realize that if we want to be able to basically copy-paste something like... let's say a DispatchTarget, we can do it this way:

def my_dispatch(dispatch_field):
    return DispatchTarget(None, dispatch_field, {
        ...
    })

class Thing(Structure):
    f = DispatchField(...)
    body = my_dispatch(f)

Because of course my_dispatch(f) needs to be evaluated before we get into StructureMeta.__new__, since class definitions aren't lazily evaluated.

If anything, we should just close this out and open a new issue/feature request, asking for a SubstructureField that inlines the specified structure into the structure it's being included in. To visualize what I mean:

class Inner(Structure):
    a = UBInt8()
    b = UBInt8()

# What's possible today:
class Outer(Structure):
    inner = SubstructureField(inner)
# Outer().inner = Inner(...)

# If we could inline a structure:
class Outer2(Structure):
    _ = Inline(Inner)

# This would be equivalent to:
class Outer2(Structure):
    a = UBInt8()
    b = UBInt8()

We might choose to do that by just virtualizing a FieldProperty for each field defined on the "inlined" structure, i.e.

class Outer(Structure):
    _inner = Inline(Inner)
    # Transparently converted to:
    # _inner = SubstructureField(Inner)
    # a = FieldProperty(_inner, ...)  # where ... is the onget and onset to access _inner.a
    # b = FieldProperty(_inner, ...)  # same for b