Skip to content

Resolve lambdas in Container __getattr__ #1030

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ev1313 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Resolve lambdas in Container __getattr__ #1030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions construct/lib/containers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from construct.lib.py3compat import *
import re
import collections
import inspect


globalPrintFullStrings = False
Expand Down Expand Up @@ -88,9 +89,18 @@ class Container(collections.OrderedDict):
def __getattr__(self, name):
try:
if name in self.__slots__:
return object.__getattribute__(self, name)
ret = object.__getattribute__(self, name)
# the dirty inspect hack is because we do not want to resolve Lazy lambdas for backwards compatibility
# if we want break backwards compatibility, we can remove this hack and just add a context parameter
# to the Lazy execute() inline function
if callable(ret) and len(inspect.signature(ret).parameters) == 1:
return ret(self)
return ret
else:
return self[name]
ret = self[name]
if callable(ret) and len(inspect.signature(ret).parameters) == 1:
return ret(self)
return ret
except KeyError:
raise AttributeError(name)

Expand Down
12 changes: 12 additions & 0 deletions tests/test_core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2383,6 +2383,18 @@ def test_switch_issue_913_using_integers():
common(d, b"\xab", 171, 1, x=1)
common(d, b"\x09\x00", 9, 2, x=2)


def test_lazy_rebuild():
d = Struct(
"foo" / Int32ul,
"bar" / Rebuild(Int32ul, lambda ctx: ctx.baz),
"baz" / Rebuild(Int32ul, lambda ctx: ctx.foo),
)
obj = {"foo": 4, "bar": lambda ctx: ctx.baz, "baz": lambda ctx: ctx.foo}
Copy link

@JayFoxRox JayFoxRox Jul 2, 2023

Choose a reason for hiding this comment

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

Why isn't this just {"foo": 4}? Asked differently: Why are "bar" and "baz" functions?

I think it's a bit hacky to allow the data to decide how it should be processed (~ functions). I'm not sure if there's a precedent for this in construct.

Instead, the lambda ctx: ctx.baz in Rebuild should be deferred until ctx.baz is available.

Is there an actual benefit doing it the proposed way (allowing functions in input to build) over deferring Rebuild?

res = d.build(obj)
assert(res == b'\x04\x00\x00\x00\x04\x00\x00\x00\x04\x00\x00\x00')


@xfail(reason="unfixable defect in the design")
def test_adapters_context_issue_954():
class IdAdapter(Adapter):
Expand Down