gh-101072: support default and kw default in PyEval_EvalCodeEx for 3.11+ #101127
Conversation
9c50259 to
0bb1c96
Compare
There was a problem hiding this comment.
Please format the C code as per https://peps.python.org/pep-0007/
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @kumaraditya303: please review the changes made to this pull request. |
There was a problem hiding this comment.
👍🏻 Fix works.
👍🏻 New tests fail without the fix applied.
👎🏻 There's a refleak in the new test.
Before (main at 7a25310):
0:00:00 load avg: 3.77 [1/1] test_capi
beginning 9 repetitions
123456789
.........
test_capi passed in 49.2 sec
== Tests result: SUCCESS ==
After (this PR rebased on top of 7a25310):
0:00:00 load avg: 3.72 [1/1] test_capi
beginning 9 repetitions
123456789
.........
test_capi leaked [1, 1, 1, 1] references, sum=4
test_capi failed (reference leak) in 49.6 sec
== Tests result: FAILURE ==
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
| for (int i = 0; i < kwcount; i++) { | ||
| PyTuple_SET_ITEM(kwnames, i, Py_NewRef(kws[2*i])); | ||
| } |
There was a problem hiding this comment.
This code is duplicated in the block right above, leading to double new refs.
|
Thanks for the edits @ambv ! |
I applied the formatting Kumar wanted
|
Thanks @MatthieuDartiailh for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11. |
|
Sorry, @MatthieuDartiailh and @ambv, I could not cleanly backport this to |
…CodeEx for 3.11+ (pythonGH-101127) Co-authored-by: Łukasz Langa <lukasz@langa.pl> (cherry picked from commit ae62bdd) Co-authored-by: Matthieu Dartiailh <m.dartiailh@gmail.com>
|
GH-101636 is a backport of this pull request to the 3.11 branch. |
| globals, | ||
| locals, | ||
| c_args, | ||
| c_args_len, |
There was a problem hiding this comment.
@ambv Looks like there are compiler warnings. See L-3152
There was a problem hiding this comment.
Good catch, looks like PyEval_EvalCodeEx lists those arguments simply as ints. I'll convert this code to do the same.
There was a problem hiding this comment.
Nikita reported this as GH-101656.
Fixes the regression observed in PyEval_EvalCodeEx on 3.11+ and add a test to avoid future regressions.
This should be backported to 3.11 I believe.