-
Notifications
You must be signed in to change notification settings - Fork 255
compiler: Fix SizeOf reconstruction #2946
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -980,7 +980,6 @@ class SizeOf(DefFunction): | |
| __rargs__ = ('intype', 'stars') | ||
|
|
||
| def __new__(cls, intype, stars=None, **kwargs): | ||
| stars = stars or '' | ||
| if not isinstance(intype, (str, ReservedWord)): | ||
| ctype = dtype_to_ctype(intype) | ||
| for k, v in ctypes_vector_mapper.items(): | ||
|
|
@@ -990,15 +989,40 @@ def __new__(cls, intype, stars=None, **kwargs): | |
| else: | ||
| intype = ctypes_to_cstr(ctype) | ||
|
|
||
| newobj = super().__new__(cls, 'sizeof', arguments=f'{intype}{stars}', **kwargs) | ||
| newobj.stars = stars | ||
| newobj.intype = intype | ||
| stars = stars or '' | ||
| if not all(c == '*' for c in str(stars)): | ||
| raise ValueError("`stars` must be a string of zero or more `*` characters") | ||
|
|
||
| if not isinstance(intype, (str, ReservedWord)): | ||
| intype = ctypes_vector_mapper[intype].__name__ | ||
|
|
||
| return newobj | ||
| return super().__new__(cls, 'sizeof', arguments=(intype, stars), **kwargs) | ||
|
|
||
| @property | ||
| def args(self): | ||
| return super().args[1] | ||
| return self._arguments | ||
|
|
||
| @property | ||
| def intype(self): | ||
| return self.arguments[0] | ||
|
|
||
| @cached_property | ||
| def ctype(self): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks quite weird to have to compare those str
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SizeOf was fundamentally broken even before the tweaks in the last merged PR. The new tests exposes them. I admit it gave me a headache or two this morning to come up with something "sufficiently clean", I guess that's the best I can do for now |
||
| for v in ctypes_vector_mapper.values(): | ||
| if str(self.intype) == v.__name__: | ||
| return v | ||
| return self.intype | ||
|
|
||
| @property | ||
| def stars(self): | ||
| return self.arguments[1] | ||
|
|
||
| def __str__(self): | ||
| try: | ||
| intype = ctypes_to_cstr(self.ctype) | ||
| except TypeError: | ||
| intype = str(self.ctype) | ||
| return f"sizeof({intype}{self.stars})" | ||
|
|
||
|
|
||
| def rfunc(func, item, *args): | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure i understand how can end up here. Line 983-990 already enforce that, it cannot be not a str at this point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reconstruction voodoo IIRC