gh-78724: raise RuntimeError's when calling methods on non-ready Struct()'s #143643
Conversation
… by dunder methods
Now modification of the Struct() while packing trigger a RuntimeError
…y Struct()'s Calling the ``Struct.__new__()`` dunder without required argument now is deprecated. Calling the ``Struct.__init__()`` dunder method on initialized object now also is deprecated.
This comment was marked as outdated.
This comment was marked as outdated.
serhiy-storchaka
left a comment
serhiy-storchaka
left a comment
There was a problem hiding this comment.
This issue is relatively independed from the concurrent packing issue, so I suggest fir it separately. We should not add deprecation in maintained versions, so I suggest to only leave checks that Struct was initialized, and add the deprecation in a separate PR.
prepare_s() should be rewritten so it only sets s_codes and other fields when it is successful.
Modules/_struct.c
Outdated
| { | ||
| PyObject *self; | ||
|
|
||
| if (PyTuple_GET_SIZE(args) != 1 |
There was a problem hiding this comment.
I do not see how can this be useful.
There was a problem hiding this comment.
This is to trigger deprecation warning in a following scenario:
>>> import struct
>>> class MyStruct(struct.Struct):
... def __init__(self):
... super().__init__('>h')
...
>>> my_struct = MyStruct()
<python-input-2>:1: DeprecationWarning: Struct().__new__() has one required argumentPS: deprecation part moved to #143659.
serhiy-storchaka
added
needs backport to 3.13
I think it's already the case. We can move a bit s_size/s_len assignments, but as far as we interested only in s_codes - I'm not sure if it's worth. There should be no failures after |
|
Well, it does not matter. It would reduce the chance of race condition, but nobody should run |
Lib/test/test_struct.py
Outdated
| self.assertRaises(RuntimeError, meth, spam) | ||
| self.assertRaises(RuntimeError, S.iter_unpack, 1) | ||
| self.assertRaises(RuntimeError, S.pack, 1) | ||
| self.assertRaises(RuntimeError, S.pack_into, 1) |
There was a problem hiding this comment.
S.pack_into(buffer, offset, v1, v2, ...)
There was a problem hiding this comment.
BTW, I think that argument processing in the struct's functions/methods should be refactored and transformed to the AC. E.g. S.pack_info could check required arguments before validation of the soself struct.
Though, this belongs to a separate issue.
I guess this belongs to the #143382 pr thread. |
|
Thanks @skirpichev for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14. |
…y Struct()'s (pythonGH-143643) (cherry picked from commit 515ae4078dffa0b74e5e5431462c2f4fe4563ffa) Co-authored-by: Sergey B Kirpichev <[email protected]>
|
Sorry, @skirpichev and @serhiy-storchaka, I could not cleanly backport this to |
|
GH-143695 is a backport of this pull request to the 3.14 branch. |
|
GH-143714 is a backport of this pull request to the 3.13 branch. |
2 participants
Uh oh!
There was an error while loading. Please reload this page.