bpo-35983: improve and test old trashcan macros

[jdemeyer]

This is a follow-up to #11841. I made a separate PR since that one has been reviewed already.

I realized that the backwards-compatibility macros Py_TRASHCAN_SAFE_BEGIN(op) can be made safe by only using the trashcan if the class inherits directly from object. That case is safe and it's also a very common case (few builtin types have a non-trivial subtype).

https://bugs.python.org/issue35983

[pitrou]

You need to merge/rebase and fix conflicts now.

[jdemeyer]

skip news because this really goes together with #11841, it's not a separate issue.

[jdemeyer]

You need to merge/rebase and fix conflicts now.

Done

[pitrou]

I'm not sure it's safe to change the meaning of the old macros. @benjaminp @serhiy-storchaka What do you think?

[jdemeyer]

I'm not sure it's safe to change the meaning of the old macros.

They are already changed as part of #11841. This is just changing them in a safer way.

[pitrou]

AFAICT, Py_TRASHCAN_SAFE_BEGIN wasn't changed in #11841 (though it was implemented differently).

[jdemeyer]

I see what you mean. Let me be very precise what this PR will fix and what it will break, hopefully clarifying things:

This will fix a crash in this case:

1. class X uses Py_TRASHCAN_SAFE_BEGIN in its deallocator
2. a class Y inherits from X
3. a 50 levels deep nested instance of Y is deallocated

This will add a new crash in this case:

1. class X uses Py_TRASHCAN_SAFE_BEGIN in its deallocator
2. class X does not inherit directly from object, but from another base class
3. a very deeply nested instance of X is deallocated, where this nesting does not involve any other trashcan-using class like tuple or list

I consider the first set of conditions more likely than the second, so this will fix more crashes than it introduces. But I cannot deny the possibility that it will break stuff.

[ambv]

Closing in favor of GH-27693. See BPO-44874 and the linked mailing list thread for details.