Skip to content

Add warning to old/classic class inheritance problem#48

Open
Eddy114514 wants to merge 1 commit intosoftdevteam:migrationfrom
Eddy114514:mro_warning
Open

Add warning to old/classic class inheritance problem#48
Eddy114514 wants to merge 1 commit intosoftdevteam:migrationfrom
Eddy114514:mro_warning

Conversation

@Eddy114514
Copy link
Copy Markdown
Collaborator

@Eddy114514 Eddy114514 commented Apr 22, 2026

Python 2 classic classes use depth-first, left-to-right method lookup. Python 3 only has new-style classes, which use C3 MRO. In classic multiple inheritance, this can silently change which base class provides an attribute after migration.

Example:

class A:
    def do_this(self): return "A"

class B(A): pass

class C(A):
    def do_this(self): return "C"

class D(B, C): pass

In Python 2 classic classes, D().do_this() resolves to A.
Under Python 3 / C3 MRO, the same hierarchy resolves to C.

Behavior Difference:
Python 2 classic lookup:
D -> B -> A -> C
Python 3 / C3 lookup:
D -> B -> C -> A
So the same attribute name can resolve to a different provider.

To implement the warning I only consider the classic classes and classes with multiple inheritance. The code compare class MRO against hypothetical C3 MRO, and warn only when a real existing attribute would resolve to a different provider. It also warns when the hierarchy has no valid C3 linearization and would fail in python3.

@Eddy114514 Eddy114514 self-assigned this Apr 22, 2026
@ltratt ltratt assigned ltratt and unassigned Eddy114514 Apr 23, 2026
@ltratt
Copy link
Copy Markdown
Member

ltratt commented Apr 23, 2026

Can you fix the PR description (the formatting has gone wrong) and squash the commits to remove the ones that aren't relevant for review (note: I'm not necessarily saying squash down to one commit!).

@ltratt
Copy link
Copy Markdown
Member

ltratt commented Apr 29, 2026

@Eddy114514 Ping.

@Eddy114514
Copy link
Copy Markdown
Collaborator Author

Eddy114514 commented May 1, 2026

@ltratt Sorry I had 2 exam this week and I was preparing for it. fixed and squashed

Comment thread Lib/test/test_py3kwarn.py Outdated
Comment thread Objects/classobject.c Outdated
Comment thread Objects/classobject.c
err = set_bases(op, v);
if (err != NULL && *err == '\0') {
if (warn_classic_mro_risk(op) < 0)
return -1;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this supposed to be -1 and not 1? [There are no docs for the function, so I could be very off in my question!]

@ltratt
Copy link
Copy Markdown
Member

ltratt commented May 1, 2026

This is a very interesting PR, and something I had not even thought about -- I like it!

@Eddy114514
Copy link
Copy Markdown
Collaborator Author

@ltratt Thank you for your fix suggestions. I have fix them, and for the -1 question. -1 is correct here because class_setattr() is a tp_setattro slot where 0 means success and -1 means failure with an exception set.

@ltratt
Copy link
Copy Markdown
Member

ltratt commented May 1, 2026

Please squash and rebase.

@Eddy114514
Copy link
Copy Markdown
Collaborator Author

@ltratt squashed.

@ltratt
Copy link
Copy Markdown
Member

ltratt commented May 2, 2026

Please also rebase against master to resolve the conflict.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants