GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-125783: Add more tests to prevent regressions with the combination of ctypes and metaclasses. by junkmd · Pull Request #126126 · python/cpython

Conversation

@junkmd

@junkmd junkmd commented Oct 29, 2024 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

In #125881, tests were added for generating pointer types in __new__ of metaclasses.
This time, I've added tests for generating pointer types in __init__ of metaclasses.

This is internal-only, so I don’t think it needs a NEWS entry.
I would like to backport this to 3.13 as well.
Please see also #125783 (comment).

  • Issue: Add tests to prevent regressions with the combination of ctypes and metaclasses. #125783

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
junkmd added 2 commits October 29, 2024 14:05
@junkmd
Add test_creating_pointer_in_dunder_init_....
011c978
@junkmd
Add comments.
7a67476
@bedevere-app bedevere-app Bot added tests Tests in the Lib/test dir awaiting review labels Oct 29, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Oct 29, 2024
Add tests to prevent regressions with the combination of ctypes and metaclasses. #125783
Closed
@skirpichev skirpichev added the skip news label Oct 29, 2024
encukou
encukou reviewed Oct 30, 2024
View reviewed changes
Comment thread Lib/test/test_ctypes/test_c_simple_type_meta.py Outdated
Comment on lines +90 to +94
# Test for modern metaclass initialization that does not
# require recursion avoidance.

class ct_meta(type):
def __init__(self, name, bases, namespace):

@encukou encukou Oct 30, 2024

Copy link
Copy Markdown
Member

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.

  • Please don't leave out the super().__init__ call when subclassing.
  • Please do limit recursion, as with __new__.
Suggested change
# Test for modern metaclass initialization that does not
# require recursion avoidance.
class ct_meta(type):
def __init__(self, name, bases, namespace):
class ct_meta(type):
def __init__(self, name, bases, namespace):
super().__init__(name, bases, namespace)
# Avoid recursion.
# (See test_creating_pointer_in_dunder_new_1)
if bases == (c_void_p,):
return
if issubclass(self, PtrBase):
return

(And similar in test_creating_pointer_in_dunder_init_2.)

The reason you don't need recursion avoidance here is that the internal PyCSimpleType is not set up for diamond inheritance patterns, and doesn't call super().__init__ (that is, ct_meta.__init__ here). That's arguably a bug. (It definitely would be one if we wanted to support such subclassing.)
If you switch the metaclass bases, writing class p_meta(ct_meta, PyCSimpleType), recursion avoidance becomes necessary again.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
junkmd reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@junkmd junkmd Oct 30, 2024

Copy link
Copy Markdown
Contributor Author

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.

Good catch! thank you.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@junkmd
Add super().__init__ calls and early returns to limit recursion.
5996870
@junkmd

junkmd commented Oct 30, 2024

Copy link
Copy Markdown
Contributor Author

I realized that with super().__init__ and recursion limits, this test might pass in Python 3.12 as well.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@junkmd
Fix an early return in __init__.
e1831ac
@junkmd
junkmd requested a review from encukou October 30, 2024 15:09
@junkmd

junkmd commented Oct 31, 2024

Copy link
Copy Markdown
Contributor Author

I built 3.12 locally and ran this test, and it passed without any errors.

In #125783, I said:

Since this approach cannot be used in 3.12, the backport would be limited to 3.13.

However, this was because I had overlooked super().__init__ and the recursion limits.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@encukou encukou added needs backport to 3.12 only security fixes needs backport to 3.13 only security fixes labels Oct 31, 2024
@encukou
Merge branch 'main' into test_metacls_PyCSimpleType_dunder_init
0b67450
@encukou
encukou merged commit 6c67446 into python:main Nov 1, 2024
@miss-islington-app

miss-islington-app Bot commented Nov 1, 2024

Copy link
Copy Markdown

Thanks @junkmd for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the awaiting review label Nov 1, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 1, 2024
@junkmd @miss-islington
pythongh-125783: Add more tests to prevent regressions with the combi…
473d402 
…nation of ctypes and metaclasses. (pythonGH-126126)

(cherry picked from commit 6c67446)

Co-authored-by: Jun Komoda <[email protected]>
@bedevere-app

bedevere-app Bot commented Nov 1, 2024

Copy link
Copy Markdown

GH-126275 is a backport of this pull request to the 3.13 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 1, 2024
@junkmd @miss-islington
pythongh-125783: Add more tests to prevent regressions with the combi…
65e40db 
…nation of ctypes and metaclasses. (pythonGH-126126)

(cherry picked from commit 6c67446)

Co-authored-by: Jun Komoda <[email protected]>
@bedevere-app bedevere-app Bot removed the needs backport to 3.13 only security fixes label Nov 1, 2024
@bedevere-app

bedevere-app Bot commented Nov 1, 2024

Copy link
Copy Markdown

GH-126276 is a backport of this pull request to the 3.12 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.12 only security fixes label Nov 1, 2024
@junkmd
junkmd deleted the test_metacls_PyCSimpleType_dunder_init branch November 1, 2024 13:30
encukou pushed a commit that referenced this pull request Nov 4, 2024
@miss-islington @junkmd
[3.13] gh-125783: Add more tests to prevent regressions with the comb…
cddecf2 
…ination of ctypes and metaclasses. (GH-126126) (GH-126275)

gh-125783: Add more tests to prevent regressions with the combination of ctypes and metaclasses. (GH-126126)
(cherry picked from commit 6c67446)

Co-authored-by: Jun Komoda <[email protected]>
encukou pushed a commit that referenced this pull request Nov 4, 2024
@miss-islington @junkmd
[3.12] gh-125783: Add more tests to prevent regressions with the comb…
a235619 
…ination of ctypes and metaclasses. (GH-126126) (GH-126276)

gh-125783: Add more tests to prevent regressions with the combination of ctypes and metaclasses. (GH-126126)
(cherry picked from commit 6c67446)

Co-authored-by: Jun Komoda <[email protected]>
picnixz pushed a commit to picnixz/cpython that referenced this pull request Dec 8, 2024
@junkmd @picnixz
pythongh-125783: Add more tests to prevent regressions with the combi…
e9269ce 
…nation of ctypes and metaclasses. (pythonGH-126126)
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
@junkmd @ebonnal
pythongh-125783: Add more tests to prevent regressions with the combi…
cf808e1 
…nation of ctypes and metaclasses. (pythonGH-126126)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@encukou encukou Awaiting requested review from encukou

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@junkmd @encukou @skirpichev
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.