Skip to content

[ESI] Don't assume __dict__ attribute for all objects #8911

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

Merged
mortbopet merged 2 commits into from
Sep 3, 2025
Merged

[ESI] Don't assume __dict__ attribute for all objects #8911

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6236b18
[ESI] Don't assume __dict__ attribute for all objects
mortbopet Sep 2, 2025
7834451
Also use __dict__ in serialization if needed
mortbopet Sep 2, 2025
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
4 changes: 4 additions & 0 deletions lib/Dialect/ESI/runtime/python/esiaccel/types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,8 @@ def bit_width(self) -> int:
def is_valid(self, obj) -> Tuple[bool, Optional[str]]:
fields_count = 0
if not isinstance(obj, dict):
if not hasattr(obj, "__dict__"):
return (False, "must be a dict or have __dict__ attribute")
obj = obj.__dict__

for (fname, ftype) in self.fields:
Expand All @@ -260,6 +262,8 @@ def is_valid(self, obj) -> Tuple[bool, Optional[str]]:

def serialize(self, obj) -> bytearray:
ret = bytearray()
if not isinstance(obj, dict):
Copy link
Contributor

@teqdruid teqdruid Sep 2, 2025

Choose a reason for hiding this comment

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

Is is_valid called before this? If not, it should be. Problem is that it should either be called at the top of the serialize call or be called here and not descend into the inner objects. Or is it better to require the user code to check and put it somewhere in the documentation?

Copy link
Contributor Author

@mortbopet mortbopet Sep 3, 2025

Choose a reason for hiding this comment

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

The main place where is_valid is called is when writing to a port. Since it (as you mention) is somewhat critical that a given type is_valid when trying to serialize and object, that is_valid call should probably be moved to the serialization function.

obj = obj.__dict__
for (fname, ftype) in reversed(self.fields):
fval = obj[fname]
ret.extend(ftype.serialize(fval))
Expand Down