Use C3 compatible ExtensionClass

[icemac]

No description provided.

[icemac]

The tests now run with activated EXTENSIONCLASS_C3_MRO. There seem to be currently two main problems:

• ZCatalog does not have a consistent MRO.
• Starting Zope in the tests has a problem with None.

[d-maurer]

Michael Howitz wrote at 2021-6-23 23:33 -0700:

The tests now run with activated `EXTENSIONCLASS_C3_MRO`. There seem to be currently two main problems: * ZCatalog does not have a consistent MRO.

I already had an adapted `ZCatalog` (but forgot to push the changes). Its now in branch `experimental_C3_MRO`.

[icemac]

After fixing the linter complaints the tests got green for the c3 variant. So I added to classic one to see what the changes break there: Some reprs are broken: they no longer use the __repr__ method provided by OFS.SimpleItem.PathReprProvider. Do we need a different class hierarchy here or could this be fixable for both MROs?

[d-maurer]

Do we need a different class hierarchy here or could this be fixable for both MROs?

We could try to move Item before ObjectManager in OFS.Folder.__bases__. We would then need to explicitly set Folder.__len__ = ObjectManager.__len__ to prevent the use of the wrong Item.__len__.
On second look, this is not enough. Regarding __len__, we could simply delete the definition in Item (it does what Python does anyway when there is no __len__ definition). But there are also objectIds, objectValues and objectItems which need to be taken from ObjectManager and not from Item.
The most promising (regarding compatiblity) approach is likely to refactor Item into BaseItem (with just the basic item methods) and Item (adding __len__, object*). In Folder, we could then have BaseItem, ObjectManager, Collection ... as __bases__.

[d-maurer]

I have implemented the idea from the previous comment: tests now succeed (with the exception of a catalog test under Windows -- almost surely unrelated to the MRO).

The wrong __repr__, observed with Folder, applies likely to other classes deriving from ObjectManager, too. Likely, for most of them corresponding tests are missing and the problem will only be detected by chance. Fortunately, a wrong __repr__ is typically not fatal (but just hampers problem analysis).

[d-maurer]

The wrong __repr__, observed with Folder, applies likely to other classes deriving from ObjectManager, too. Likely, for most of them corresponding tests are missing and the problem will only be detected by chance. Fortunately, a wrong __repr__ is typically not fatal (but just hampers problem analysis).

Likely, this means that we should not go this route: someone who does not call for C3_MRO should (likely) not need to review and adapt his class hierarchies.
An alternative would be to use different inheritance orders depending on C3_MRO where the C3 and traditional orders give different results.

[icemac]

@d-maurer Do you see any future for this PR and C3 compatible classes in Zope or should we call it an experiment which showed that we cannot be C3 compatible because of our heritage?

[d-maurer]

@d-maurer Do you see any future for this PR and C3 compatible classes in Zope or should we call it an experiment which showed that we cannot be C3 compatible because of our heritage?

I believe that C3 is possible. But it requires a lot of adaptation, not only in Zope itself but also in many packages depending on Zope. It might be difficult to maintain a switch between classic and C3 resolution controlled via an envvar (because the class inheritance structure may differ considerably).

C3 is a prerequisite to give super a well defined semantics. Apart from that, I find the classic method resolution far more intuitive and predictable than C3. It might not be worth to switch to C3: we lose a reliable super but keep an intuitive method resolution order.

[icemac]

@d-maurer Thank you for your statement. I am going to end the C3 experiment now as I see no way to get all needed parties on board. I do not see the manpower to maintain two resolution orders forever and having just partial C3 support does not help either.