Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-103743: Add PyUnstable_Object_GC_NewWithExtraData#103744

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

jbradaric
Copy link
Contributor

@jbradaricjbradaric commentedApr 24, 2023•
edited by bedevere-bot
Loading

@bedevere-bot
Copy link

Most changes to Pythonrequire a NEWS entry.

Please add it using theblurb_it web app or theblurb command-line tool.

@ghost
Copy link

ghost commentedApr 24, 2023•
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@jbradaricjbradaricforce-pushed thejbradaric/obj-new-with-extra-data branch fromd045f45 tod300004CompareApril 24, 2023 08:32
@jbradaricjbradaricforce-pushed thejbradaric/obj-new-with-extra-data branch 3 times, most recently fromaa3716f tocc46476CompareApril 24, 2023 10:01
@jbradaricjbradaricforce-pushed thejbradaric/obj-new-with-extra-data branch fromcc46476 tod644206CompareApril 24, 2023 10:53
@jbradaricjbradaric marked this pull request as ready for reviewApril 24, 2023 11:20
@jbradaricjbradaric changed the title[WIP] gh-103743: Add PyUnstable_Object_GC_NewWithExtraDatagh-103743: Add PyUnstable_Object_GC_NewWithExtraDataApr 24, 2023
@encukouencukou self-requested a reviewApril 27, 2023 16:08
This is a further workaround: this time for the fact thatwe can't rely on tp_alloc, which zeroes the memory (includingmemory 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 withPyObject_Init after zeroing). Initializing twice throws off the globalrefcount total in debug builds.
@encukou
Copy link
Member

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 inModules/_testcapi/, but that can wait for another PR.

erlend-aasland reacted with thumbs up emoji

@jbradaric
Copy link
ContributorAuthor

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

@jbradaric
Copy link
ContributorAuthor

One more thing to do will be moving GC- or allocation-related tests from_testcapimodule.c to their own file inModules/_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 reacted with thumbs up emoji

Copy link
Contributor

@erlend-aaslanderlend-aasland left a comment

Choose a reason for hiding this comment

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

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
Copy link
Member

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
Copy link
ContributorAuthor

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

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

erlend-aasland reacted with hooray emoji

@jbradaricjbradaric deleted the jbradaric/obj-new-with-extra-data branchMay 2, 2023 12:01
carljm added a commit to carljm/cpython that referenced this pull requestMay 2, 2023
* 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)
carljm added a commit to carljm/cpython that referenced this pull requestMay 2, 2023
* 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
Copy link
Member

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
Copy link
Member

Fixed in#104191.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@Eclips4Eclips4Eclips4 left review comments

@erlend-aaslanderlend-aaslanderlend-aasland approved these changes

@pablogsalpablogsalAwaiting requested review from pablogsalpablogsal is a code owner

@encukouencukouAwaiting requested review from encukou

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

8 participants
@jbradaric@bedevere-bot@encukou@brandtbucher@erlend-aasland@Eclips4@arhadthedev@ambv
[8]ページ先頭
©2009-2025 Movatter.jp