Use Py_TPFLAGS_IMMUTABLETYPE in __class__ assignments check

[erlend-aasland]

BPO	43973
Nosy	@gvanrossum, @pitrou, @vstinner, @erlend-aasland, @shreyanavigyan
PRs	bpo-43973: Use Py_TPFLAGS_IMMUTABLETYPE to check for class assignments #25714

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2021-04-30.10:09:43.485>
created_at = <Date 2021-04-29.08:27:05.693>
labels = []
title = 'Use Py_TPFLAGS_IMMUTABLETYPE in __class__ assignments check'
updated_at = <Date 2021-04-30.10:58:54.503>
user = 'https://github.com/erlend-aasland'

bugs.python.org fields:

activity = <Date 2021-04-30.10:58:54.503>
actor = 'erlendaasland'
assignee = 'none'
closed = True
closed_date = <Date 2021-04-30.10:09:43.485>
closer = 'erlendaasland'
components = []
creation = <Date 2021-04-29.08:27:05.693>
creator = 'erlendaasland'
dependencies = []
files = []
hgrepos = []
issue_num = 43973
keywords = ['patch']
message_count = 7.0
messages = ['392290', '392291', '392313', '392404', '392405', '392412', '392415']
nosy_count = 5.0
nosy_names = ['gvanrossum', 'pitrou', 'vstinner', 'erlendaasland', 'shreyanavigyan']
pr_nums = ['25714']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue43973'
versions = []

[erlend-aasland]

Use Py_TPFLAGS_IMMUTABLETYPE iso. Py_TPFLAGS_HEAPTYPE in object_set_class().

See also:

• bpo-43908 (particularly msg392286)
• bpo-24912

• cpython/Objects/typeobject.c

  Lines 4620 to 4668 in e047239

  	/* In versions of CPython prior to 3.5, the code in
  	compatible_for_assignment was not set up to correctly check for memory
  	layout / slot / etc. compatibility for non-HEAPTYPE classes, so we just
  	disallowed __class__ assignment in any case that wasn't HEAPTYPE ->
  	HEAPTYPE.
  	During the 3.5 development cycle, we fixed the code in
  	compatible_for_assignment to correctly check compatibility between
  	arbitrary types, and started allowing __class__ assignment in all cases
  	where the old and new types did in fact have compatible slots and
  	memory layout (regardless of whether they were implemented as HEAPTYPEs
  	or not).
  	Just before 3.5 was released, though, we discovered that this led to
  	problems with immutable types like int, where the interpreter assumes
  	they are immutable and interns some values. Formerly this wasn't a
  	problem, because they really were immutable -- in particular, all the
  	types where the interpreter applied this interning trick happened to
  	also be statically allocated, so the old HEAPTYPE rules were
  	"accidentally" stopping them from allowing __class__ assignment. But
  	with the changes to __class__ assignment, we started allowing code like
  	class MyInt(int):
  	...
  	# Modifies the type of *all* instances of 1 in the whole program,
  	# including future instances (!), because the 1 object is interned.
  	(1).__class__ = MyInt
  	(see https://bugs.python.org/issue24912).
  	In theory the proper fix would be to identify which classes rely on
  	this invariant and somehow disallow __class__ assignment only for them,
  	perhaps via some mechanism like a new Py_TPFLAGS_IMMUTABLE flag (a
  	"denylisting" approach). But in practice, since this problem wasn't
  	noticed late in the 3.5 RC cycle, we're taking the conservative
  	approach and reinstating the same HEAPTYPE->HEAPTYPE check that we used
  	to have, plus an "allowlist". For now, the allowlist consists only of
  	ModuleType subtypes, since those are the cases that motivated the patch
  	in the first place -- see https://bugs.python.org/issue22986 -- and
  	since module objects are mutable we can be sure that they are
  	definitely not being interned. So now we allow HEAPTYPE->HEAPTYPE *or*
  	ModuleType subtype -> ModuleType subtype.
  	So far as we know, all the code beyond the following 'if' statement
  	will correctly handle non-HEAPTYPE classes, and the HEAPTYPE check is
  	needed only to protect that subset of non-HEAPTYPE classes for which
  	the interpreter has baked in the assumption that all instances are
  	truly immutable.
  	*/

[erlend-aasland]

IMO, we can drop most (all?) of the huge comment before the check in object_set_class(). History is preserved in the git log. Contrary to Py_TPFLAGS_HEAPTYPE, using Py_TPFLAGS_IMMUTABLETYPE speaks for itself. We might reduce it to just refer to to bpo-43908 and bpo-24912.

Something a la this:
/* Historically, only static types were immutable.

• Py_TPFLAGS_IMMUTABLETYPE was introduced in Python 3.10.
• See bpo-43908 and bpo-24912. */

What do you think?

[vstinner]

IMO, we can drop most (all?) of the huge comment before the check in object_set_class().

I don't think so. I don't understand why it's allowed to override the __class__ attribute of ModuleType instances whereas it's a static type. IMO the long comment remains useful and something should be done later to make the condition more generic, rather than checking PyType_IsSubtype(newto, &PyModule_Type) specifically.

[vstinner]

New changeset b73b5fb by Erlend Egeberg Aasland in branch 'master':
bpo-43973: object_set_class() checks Py_TPFLAGS_IMMUTABLETYPE (GH-25714)
b73b5fb

[erlend-aasland]

Ok! Did you see bpo-24912 regarding the ModuleType check?

[vstinner]

Ok! Did you see bpo-24912 regarding the ModuleType check?

No, I didn't. Sorry, there are too many things to look at, and too little bit. Do you think that something should be changed? If yes, please open a new separated issue.

[erlend-aasland]

Do you think that something should be changed? If yes, please open a new separated issue.

No, my comment was just for information :)