gh-103743: Add PyUnstable_Object_GC_NewWithExtraData by jbradaric · Pull Request #103744 · python/cpython

jbradaric · 2023-04-24T08:12:12Z

Issue: Allocating extra data after an object instance #103743

bedevere-bot · 2023-04-24T08:12:15Z

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

ghost · 2023-04-24T08:12:39Z

All commit authors signed the Contributor License Agreement.

…3743)

That way it doesn't need the Py_LIMITED_API guard

This is a further workaround: this time for the fact that we can't rely on tp_alloc, which zeroes the memory (including memory for subclasses, like slots in the test) for normal classes. For the test type, this means the instances aren't initialized twice (once in PyUnstable_Object_GC_NewWithExtraData, once again with PyObject_Init after zeroing). Initializing twice throws off the global refcount total in debug builds.

encukou · 2023-04-28T14:43:35Z

Hi,
Time for 3.12 is short, so rather than write review comments I pushed suggestions/fixes to the PR directly. @jbradaric, could you go through the commits (and commit descriptions)? Do they make sense to you?

One more thing to do will be moving GC- or allocation-related tests from _testcapimodule.c to their own file in Modules/_testcapi/, but that can wait for another PR.

jbradaric · 2023-04-28T18:21:44Z

@encukou, commits look good, thanks for the review and fixes. Sorry for the amount of required changes :)

jbradaric · 2023-04-28T19:03:38Z

One more thing to do will be moving GC- or allocation-related tests from _testcapimodule.c to their own file in Modules/_testcapi/, but that can wait for another PR.

Would it be OK if I create a PR to move these tests to a separate file? Or is someone already working on this?

erlend-aasland

LGTM. Minor nits regarding docs.

Fix indentation and reword some parts to sound more natural. Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>

encukou · 2023-05-02T11:39:25Z

Thank you for your first contribution to CPython! (It is first, right?)

Sorry for the amount of required changes

No problem!

Would it be OK if I create a PR to move these tests to a separate file? Or is someone already working on this?

Go ahead! The issue is #93649 and it has a bunch of example PRs.
I don't think anyone's working on this.

jbradaric · 2023-05-02T12:01:37Z

Thank you for your first contribution to CPython! (It is first, right?)

Yes, it's the first one. Thanks for all the help :)

* main: pythongh-103822: [Calendar] change return value to enum for day and month APIs (pythonGH-103827) pythongh-65022: Fix description of tuple return value in copyreg (python#103892) pythonGH-103525: Improve exception message from `pathlib.PurePath()` (pythonGH-103526) pythongh-84436: Add integration C API tests for immortal objects (pythongh-103962) pythongh-103743: Add PyUnstable_Object_GC_NewWithExtraData (pythonGH-103744) pythongh-102997: Update Windows installer to SQLite 3.41.2. (python#102999) pythonGH-103484: Fix redirected permanently URLs (python#104001) Improve assert_type phrasing (python#104081) pythongh-102997: Update macOS installer to SQLite 3.41.2. (pythonGH-102998) pythonGH-103472: close response in HTTPConnection._tunnel (python#103473) pythongh-88496: IDLE - fix another test on macOS (python#104075) pythongh-94673: Hide Objects in PyTypeObject Behind Accessors (pythongh-104074) pythongh-94673: Properly Initialize and Finalize Static Builtin Types for Each Interpreter (pythongh-104072) pythongh-104016: Skip test for deeply neste f-strings on wasi (python#104071)

* main: (760 commits) pythonGH-104102: Optimize `pathlib.Path.glob()` handling of `../` pattern segments (pythonGH-104103) pythonGH-104104: Optimize `pathlib.Path.glob()` by avoiding repeated calls to `os.path.normcase()` (pythonGH-104105) pythongh-103822: [Calendar] change return value to enum for day and month APIs (pythonGH-103827) pythongh-65022: Fix description of tuple return value in copyreg (python#103892) pythonGH-103525: Improve exception message from `pathlib.PurePath()` (pythonGH-103526) pythongh-84436: Add integration C API tests for immortal objects (pythongh-103962) pythongh-103743: Add PyUnstable_Object_GC_NewWithExtraData (pythonGH-103744) pythongh-102997: Update Windows installer to SQLite 3.41.2. (python#102999) pythonGH-103484: Fix redirected permanently URLs (python#104001) Improve assert_type phrasing (python#104081) pythongh-102997: Update macOS installer to SQLite 3.41.2. (pythonGH-102998) pythonGH-103472: close response in HTTPConnection._tunnel (python#103473) pythongh-88496: IDLE - fix another test on macOS (python#104075) pythongh-94673: Hide Objects in PyTypeObject Behind Accessors (pythongh-104074) pythongh-94673: Properly Initialize and Finalize Static Builtin Types for Each Interpreter (pythongh-104072) pythongh-104016: Skip test for deeply neste f-strings on wasi (python#104071) pythongh-104057: Fix direct invocation of test_super (python#104064) pythongh-87092: Expose assembler to unit tests (python#103988) pythongh-97696: asyncio eager tasks factory (python#102853) pythongh-84436: Immortalize in _PyStructSequence_InitBuiltinWithFlags() (pythongh-104054) ...

brandtbucher · 2023-05-05T00:09:20Z

FYI, it looks like this USan buildbot started failing on this commit: https://buildbot.python.org/all/#/builders/719

Not sure if it's a false positive or not, but is probably worth looking into (especially since the 3.12 release is coming up).

brandtbucher · 2023-05-05T13:58:06Z

Fixed in #104191.

bedevere-bot mentioned this pull request Apr 24, 2023
Allocating extra data after an object instance #103743
Closed

bedevere-bot added the awaiting review label Apr 24, 2023

arhadthedev added the topic-C-API label Apr 24, 2023

jbradaric force-pushed the jbradaric/obj-new-with-extra-data branch from d045f45 to d300004Compare April 24, 2023 08:32

Eclips4 reviewed Apr 24, 2023
View reviewed changes

Comment threadDoc/c-api/gcsupport.rst Outdated

jbradaric force-pushed the jbradaric/obj-new-with-extra-data branch 3 times, most recently from aa3716f to cc46476Compare April 24, 2023 10:01

pythongh-103743: Add PyUnstable_Object_GC_NewWithExtraData (python#10…
d644206
…3743)

jbradaric force-pushed the jbradaric/obj-new-with-extra-data branch from cc46476 to d644206Compare April 24, 2023 10:53

jbradaric marked this pull request as ready for review April 24, 2023 11:20

jbradaric requested a review from pablogsal as a code owner April 24, 2023 11:20

jbradaric changed the title ~~[WIP] gh-103743: Add PyUnstable_Object_GC_NewWithExtraData~~gh-103743: Add PyUnstable_Object_GC_NewWithExtraDataApr 24, 2023

ambvand others added 4 commits April 25, 2023 02:34

Merge branch 'main' into jbradaric/obj-new-with-extra-data
14975c0

Don't suggest PEP 697 will be a solution
52135dc

Move declaration to Include/cpython
e2c4b37
That way it doesn't need the Py_LIMITED_API guard

Style nitpicks
ea9e9ac

encukou self-requested a review April 27, 2023 16:08

encukou added 3 commits April 28, 2023 14:57

Clarify the docs a bit
f9817ca

Convert the test class to a heap type
f0098d6

erlend-aasland reviewed Apr 28, 2023
View reviewed changes

Comment threadDoc/c-api/gcsupport.rst Outdated
Comment threadDoc/c-api/gcsupport.rst Outdated
Comment threadMisc/NEWS.d/next/C API/2023-04-24-10-31-59.gh-issue-103743.2xYA1K.rst Outdated

Doc style fixes
525705a
Fix indentation and reword some parts to sound more natural. Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>

erlend-aasland approved these changes Apr 29, 2023
View reviewed changes

bedevere-bot removed the awaiting review label Apr 29, 2023

bedevere-bot added the awaiting merge label Apr 29, 2023

encukou merged commit 87223f3 into python:mainMay 2, 2023

bedevere-bot removed the awaiting merge label May 2, 2023

jbradaric deleted the jbradaric/obj-new-with-extra-data branch May 2, 2023 12:01

jbradaric mentioned this pull request May 11, 2023
gh-93649: Split gc- and allocation tests from _testcapimodule.c #104403
Merged

Navigation Menu

Uh oh!

Uh oh!

gh-103743: Add PyUnstable_Object_GC_NewWithExtraData - #103744

gh-103743: Add PyUnstable_Object_GC_NewWithExtraData#103744
encukou merged 9 commits into
python:mainfrom
jbradaric:jbradaric/obj-new-with-extra-data

jbradaric commented Apr 24, 2023•
edited by bedevere-bot
Loading

Uh oh!

bedevere-bot commented Apr 24, 2023

Uh oh!

ghost commented Apr 24, 2023•
edited by ghost
Loading

Uh oh!

Uh oh!

encukou commented Apr 28, 2023

Uh oh!

jbradaric commented Apr 28, 2023

Uh oh!

jbradaric commented Apr 28, 2023

Uh oh!

erlend-aasland left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

encukou commented May 2, 2023

Uh oh!

jbradaric commented May 2, 2023

Uh oh!

brandtbucher commented May 5, 2023

Uh oh!

brandtbucher commented May 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Uh oh!

Uh oh!

Conversation

jbradaric commented Apr 24, 2023• edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-bot commented Apr 24, 2023

Uh oh!

ghost commented Apr 24, 2023• edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

encukou commented Apr 28, 2023

Uh oh!

jbradaric commented Apr 28, 2023

Uh oh!

jbradaric commented Apr 28, 2023

Uh oh!

erlend-aasland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

encukou commented May 2, 2023

Uh oh!

jbradaric commented May 2, 2023

Uh oh!

brandtbucher commented May 5, 2023

Uh oh!

brandtbucher commented May 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

jbradaric commented Apr 24, 2023•
edited by bedevere-bot
Loading

ghost commented Apr 24, 2023•
edited by ghost
Loading