construct / construct

Construct: Declarative data structures for python that allow symmetric parsing and building
http://construct.readthedocs.org
Other
906 stars 153 forks source link

`Computed` fields that defined later not seen in fields defined above #1077

Open Mingun opened 4 months ago

Mingun commented 4 months ago

Kaitai Struct project has a Construct target and it can define so called "instance properties" for types. That properties can use another instance properties to calculate their values.

That properties generated as Construct's Computed fields. For example, the KSY

meta:
  id: construct
instances:
  a:
    value: 42
  b:
    value: a

will be generated as:

d = Struct(
   "a" / Computed(lambda this: 42),
   "b" / Computed(lambda this: this.a),
)

This code works perfectly fine, but if we change the order slightly in KSY:

meta:
  id: construct
instances:
  a:
    value: b
  b:
    value: 42

then the generated code would be:

d = Struct(
   "a" / Computed(lambda this: this.b),
   "b" / Computed(lambda this: 42),
)

Now, however, the exception is raised when you try to d.parse(b""):

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "D:/Tools/msys64/mingw64/lib/python3.10/site-packages/construct/core.py", line 404, in parse
    return self.parse_stream(io.BytesIO(data), **contextkw)
  File "D:/Tools/msys64/mingw64/lib/python3.10/site-packages/construct/core.py", line 416, in parse_stream
    return self._parsereport(stream, context, "(parsing)")
  File "D:/Tools/msys64/mingw64/lib/python3.10/site-packages/construct/core.py", line 428, in _parsereport
    obj = self._parse(stream, context, path)
  File "D:/Tools/msys64/mingw64/lib/python3.10/site-packages/construct/core.py", line 2236, in _parse
    subobj = sc._parsereport(stream, context, path)
  File "D:/Tools/msys64/mingw64/lib/python3.10/site-packages/construct/core.py", line 428, in _parsereport
    obj = self._parse(stream, context, path)
  File "D:/Tools/msys64/mingw64/lib/python3.10/site-packages/construct/core.py", line 2770, in _parse
    return self.subcon._parsereport(stream, context, path)
  File "D:/Tools/msys64/mingw64/lib/python3.10/site-packages/construct/core.py", line 428, in _parsereport
    obj = self._parse(stream, context, path)
  File "D:/Tools/msys64/mingw64/lib/python3.10/site-packages/construct/core.py", line 2915, in _parse
    return self.func(context) if callable(self.func) else self.func
  File "<stdin>", line 2, in <lambda>
  File "D:/Tools/msys64/mingw64/lib/python3.10/site-packages/construct/lib/containers.py", line 100, in __getattr__
    raise AttributeError(name)
AttributeError: b

This is because Kaitai Struct internally keeps instances sorted by name to generate code in the same order each time. Although IndexMap would be better in that case (to keep the order the same as in KSY), it still suffer from that problem, because user should ensure the correct order, which is conceptually wrong for the declarative format.

The exception seems counter-intuitive in any way. We use a lambda to define rules how property should be computed, but accessing of other properties inside lambda does not call their evaluation, but returns an error instead.

You can see results of such behaviour on the Kaitai CI page which tracks how much features each target language is supported: https://ci.kaitai.io/. For example, the test BcdUserTypeBe failed because of this. Although this can be fixed from Kaitai side by generating instances in topological order, I think, that Construct should not raise errors in this case.

franzhaas commented 4 months ago

Not solving the problem, but.:

d = Struct(
    "a" / Computed(42),
    "b" / Computed(this.a),
)

could be a interesting alternative to the lambdas... it allows for the compile feature to work better (or at all).

franzhaas commented 4 months ago

would you mind to check this PR?

https://github.com/franzhaas/construct/tree/speedOptimisations

does this solve your issue (this will not work with the lambdas, only with expressions...)?