GeezerVison

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

GeezerVison is showing github.com — gh-106917: fix super classmethod calls to non-classmethods by carljm · Pull Request #106977 · python/cpython

Conversation

carljm commented Jul 21, 2023 •

edited by bedevere-bot

Loading

Uh oh!

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

Copy link

Copy Markdown

Member

The fix for #105035 was subtly incomplete. It checked that cls->tp_getattro == PyObject_GetGenericAttr (as a precondition for attempting the load-method optimization), but what we really need to know is that Py_TYPE(self)->tp_getattro == PyObject_GenericGetAttr. In the typical case of super(MyClass, instance)... these are effectively the same thing (instance will be an instance of MyClass), but in a classmethod super() call (super(MyClass, MyClass)...) they are quite different; the type of MyClass is type, which has a different tp_getattro.

For actual classmethods this was fine; the load-method optimization still works correctly for classmethods looked up on types. But it doesn't work for regular methods (plain function objects) looked up on types; those are just regular function calls with no bound method behavior.

It's pretty unusual to use classmethod-style super where the call target is a regular function, not a classmethod. But it can occur, e.g. if using super with __new__ and dynamically-assigned __new__ methods, as in #106917.

Issue: 3.12.0b4 Backwards incompatible change with reassignment of cls.__new__ and super() #106917

Sorry, something went wrong.

Uh oh!

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

All reactions


          pythongh-106917: fix super classmethod calls to non-classmethods

c603b7e

bedevere-bot mentioned this pull request

Jul 21, 2023

3.12.0b4 Backwards incompatible change with reassignment of cls.__new__ and super() #106917

Closed

bedevere-bot added the awaiting core review label

Jul 21, 2023

carljm requested review from Fidget-Spinner and sunmy2019

July 21, 2023 21:41


          Merge branch 'main' into superfix

b9fab74

* main:
  pythongh-106004: Add PyDict_GetItemRef() function (python#106005)

carljm added needs backport to 3.12

only security fixes

release-blocker labels

Jul 21, 2023

carljm self-assigned this

Jul 21, 2023

Fidget-Spinner reviewed

Jul 22, 2023

View reviewed changes

Fidget-Spinner left a comment

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.

Thanks. This is quite the subtlety. LGTM in general. I just have a comment about the tests.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

Lib/test/test_super.py Resolved

Uh oh!

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

carljm added 2 commits

July 24, 2023 10:21


          Merge branch 'main' into superfix

9a6bdfa

* main: (73 commits)
  Thoroughly refactor the cases generator (python#107151)
  Docs: Add missing markup to Argument Clinic docs (python#106876)
  pythongh-107162: Document errcode.h usage in its comment (python#107177)
  pythongh-106320: Remove private _PyDict C API (python#107145)
  Fix PyVectorcall_Function doc versionadded (python#107140)
  Docs: Remove duplicate word in Argument Clinic howto heading (python#107169)
  pythongh-107017: Change Chapter Strings to Texts in the Introduction chapter. (python#107104)
  pythongh-106320: Remove private _PyObject C API (python#107159)
  Docs: fix typo in os.pwrite docstring (python#107087)
  pythongh-105291: Add link to migration guide for distutils (python#107130)
  pythongh-106948: Docs: Disable links for C standard library functions, OS utility functions and system calls (python#107062)
  pythongh-106320: Remove _PyBytes_Join() C API (python#107144)
  pythongh-106320: Remove private _PyObject C API (python#107147)
  pythongh-106320: Remove _PyTuple_MaybeUntrack() C API (python#107143)
  pythongh-106320: Remove _PyIsSelectable_fd() C API (python#107142)
  Remove superflous whitespaces in `layout.html`. (pythonGH-107067)
  pythongh-107122: Update what's news for dbm.*dbm.clear() method (pythongh-107135)
  pythongh-107122: Add clear method to dbm.ndbm module (pythongh-107126)
  pythongh-62519: Make pgettext search plurals when translation is not found (python#107118)
  pythongh-107122: Add clear method to dbm.gdbm.module (pythongh-107127)
  ...


          make specialization warmup more explicit in tests

7003e96

Fidget-Spinner approved these changes

Jul 24, 2023

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting core review labels

Jul 24, 2023

carljm merged commit e5d5522 into python:main

Jul 24, 2023

miss-islington commented Jul 24, 2023

Copy link

Copy Markdown

Contributor

Thanks @carljm for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

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

bedevere-bot removed the awaiting merge label

Jul 24, 2023

miss-islington commented Jul 24, 2023

Copy link

Copy Markdown

Contributor

Sorry, @carljm, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker e5d5522612e03af3941db1d270bf6caebf330b8a 3.12

All reactions

Sorry, something went wrong.

Uh oh!

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

carljm deleted the superfix branch

July 24, 2023 20:15

carljm added a commit to carljm/cpython that referenced this pull request

Jul 24, 2023


          [3.12] pythongh-106917: fix super classmethod calls to non-classmetho…

4e1f4a5

…ds (pythonGH-106977).

(cherry picked from commit e5d5522)

Co-authored-by: Carl Meyer <[email protected]>

bedevere-bot commented Jul 24, 2023

Copy link

Copy Markdown

GH-107204 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-bot removed the needs backport to 3.12 only security fixes label

Jul 24, 2023

carljm added a commit that referenced this pull request

Jul 24, 2023


          [3.12] gh-106917: fix super classmethod calls to non-classmethods (GH…

5fd028b

…-106977). (#107204)

(cherry picked from commit e5d5522)

jtcave pushed a commit to jtcave/cpython that referenced this pull request

Jul 27, 2023


          pythongh-106917: fix super classmethod calls to non-classmethods (pyt…

b8564f6

…hon#106977)

JelleZijlstra removed the release-blocker label

May 24, 2024

facebook-github-bot pushed a commit to facebookincubator/MetaPython that referenced this pull request

Mar 30, 2025


          Fix 3.12 super implementation

164f351

Summary:
Fixes a couple of issues which are causing `test_super` to fail on 3.12:
1) We don't actually respect the global super object at all! We are using the exported Cinder function to implement this on 3.10, but just calling the equivalent of `Ci_Super_Lookup` on 3.12. Because we need to have the `global_super != &PySuper_Type` logic on 3.12 this just brings it over and stops exporting it from Cinder 3.10.
3) We get `no_args_in_super_call` backwards as oparg is effectively the number of arguments.
2) Ports the fix in python/cpython#106977 to the implementation of super in both the JIT and Cinder 3.10.

Reviewed By: alexmalyshev

Differential Revision: D71638145

fbshipit-source-id: 0f5a189d8cea573034e578c921925ce2988c3bb1

facebook-github-bot pushed a commit to facebookincubator/cinderx that referenced this pull request

Mar 30, 2025


          Fix 3.12 super implementation

f1cab9d

Summary:
Fixes a couple of issues which are causing `test_super` to fail on 3.12:
1) We don't actually respect the global super object at all! We are using the exported Cinder function to implement this on 3.10, but just calling the equivalent of `Ci_Super_Lookup` on 3.12. Because we need to have the `global_super != &PySuper_Type` logic on 3.12 this just brings it over and stops exporting it from Cinder 3.10.
3) We get `no_args_in_super_call` backwards as oparg is effectively the number of arguments.
2) Ports the fix in python/cpython#106977 to the implementation of super in both the JIT and Cinder 3.10.

Reviewed By: alexmalyshev

Differential Revision: D71638145

fbshipit-source-id: 0f5a189d8cea573034e578c921925ce2988c3bb1

Reviewers

Fidget-Spinner

Fidget-Spinner approved these changes

sunmy2019

Awaiting requested review from sunmy2019

Uh oh!

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

Assignees

carljm

Labels

None yet

Projects

Release and Deferred blockers 🚫

Status: Done

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.

5 participants

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.

GeezerVison

Conversation

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

Uh oh!

Uh oh!

Fidget-Spinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

miss-islington commented Jul 24, 2023

Uh oh!

miss-islington commented Jul 24, 2023

Uh oh!

bedevere-bot commented Jul 24, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

carljm commented Jul 21, 2023 •

edited by bedevere-bot

Loading

Uh oh!

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