Issue2445

classification
Title: Eclipse's DelegatingFeatureMap has MRO conflict (and IBM's MQQueue)
Type: crash Severity: normal
Components: Core Versions: Jython 2.7
Milestone: Jython 2.7.2
process
Status: pending Resolution: fixed
Dependencies: Superseder:
Assigned To: jeff.allen Nosy List: bmvanwyk, jeff.allen, tomluk, wfouche2, zyasoft
Priority: high Keywords:

Created on 2015-12-29.13:20:54 by tomluk, last changed 2019-04-28.16:59:33 by jeff.allen.

Files
File name Uploaded Description Edit Remove
jython MRO problem.zip tomluk, 2015-12-29.13:20:54 extract this, paste standalone version of jython into extracted folder, edit runMe.bat to use new jython.jar, run runMe.bat
Messages
msg10561 (view) Author: Tomas Lukosius (tomluk) Date: 2015-12-29.13:30:19
using: 
Windows 8.1 64 bit OS
java 1.8u66 64 bit

Tryed to execute script with jython v2.5.4rc1. Import was succesful.
msg10565 (view) Author: Jim Baker (zyasoft) Date: 2015-12-29.14:51:15
First, please submit your bug with some more explanatory text so we don't have to dig through a zip file to triage.

Related bug is #1878

I see you also tried this on Jython 2.7, so there's some additional diagnostic output to consider:

TypeError: Supertypes that share a modified attribute have an MRO conflict[
attribute=remove,
supertypes=[
<type 'org.eclipse.emf.ecore.util.DelegatingNotifyingInternalEListImpl'>,
<type 'java.util.AbstractList'>,
<type 'org.eclipse.emf.common.util.DelegatingEList'>,
<type 'org.eclipse.emf.common.util.AbstractEList'>,
<type 'org.eclipse.emf.common.notify.NotifyingList'>,
<type 'org.eclipse.emf.common.notify.impl.DelegatingNotifyingListImpl'>,
<type 'org.eclipse.emf.common.util.EList'>,
<type 'java.util.List'>,
<type 'org.eclipse.emf.ecore.util.InternalEList'>
],
type=DelegatingFeatureMap]

It looks like the same problem we saw in #1878 - DelegatingFeatureMap is a Map that implements Iterable
msg10566 (view) Author: Tomas Lukosius (tomluk) Date: 2015-12-29.15:23:15
Thank you Jim for your quick response. Sorry for that skinny report, forgot to paste output i got from jython. But you did it for me.
msg10567 (view) Author: Jim Baker (zyasoft) Date: 2015-12-29.16:08:05
Tom, no worries about your initial "skinny" report :)

The underlying problem may be this line of code in PyJavaType:

if (method.equals("__iter__") && proxySet.equals(Generic.set(Iterable.class, Map.class))) {
    continue;
}

Mostly likely this equality test is too restricted and it should be instead testing it the proxy set is a subset:

if (method.equals("__iter__") && proxySet.containsAll(Generic.set(Iterable.class, Map.class))) {
    continue;
}
msg10568 (view) Author: Jim Baker (zyasoft) Date: 2015-12-29.17:45:19
So the problem is not what I thought it could be in msg10567. Define a test class like so:

// example simplified from
// org.eclipse.emf.ecore.util.DelegatingFeatureMap in the Eclipse
// Modeling Framework

// used as a test of http://bugs.jython.org/issue2445

package javatests;

import java.io.Serializable;
import java.util.AbstractList;
import java.util.AbstractSequentialList;
import java.util.Iterator;
import java.util.ListIterator;

// The following tag interfaces duplicate the interface/abstract class supertypes of
// org.eclipse.emf.ecore.util.DelegatingFeatureMap
// NOTE this class/interface hierarchy is sort of crazy, isn't it?

interface EList<E> extends java.util.List<E> {}
interface InternalEList<E> extends EList<E> {
    interface Unsettable<E> extends InternalEList<E> {}
}

abstract class AbstractSequentialInternalEList<E> extends AbstractSequentialList<E> implements InternalEList<E> {}

abstract class EContentsEList<E> extends AbstractSequentialInternalEList<E> implements EList<E>, InternalEList<E> {
    interface FeatureListIterator<E> extends FeatureIterator<E>, ListIterator<E> {}
    interface FeatureIterator<E> extends Iterator<E> {}
}

interface FeatureMap extends EList<FeatureMap.Entry> {
    interface Entry {
        interface Internal extends Entry {
        }
    }
    interface ValueListIterator<E> extends EContentsEList.FeatureListIterator<E> {}
    interface Internal extends FeatureMap, InternalEList<Entry>, EStructuralFeature.Setting {
        interface Wrapper {}
    }
}

abstract class AbstractEList<E> extends AbstractList<E> implements EList<E> {}
abstract class DelegatingEList<E> extends AbstractEList<E> implements Cloneable, Serializable {}
interface NotifyingList<E> extends EList<E> {}
abstract class DelegatingNotifyingListImpl<E> extends DelegatingEList<E> implements NotifyingList<E> {}
abstract class DelegatingNotifyingInternalEListImpl<E> extends DelegatingNotifyingListImpl<E> implements InternalEList<E> {}
interface Notifier {}
interface EObject extends Notifier {}
interface EModelElement extends EObject {}
interface ENamedElement extends EModelElement {}
interface ETypedElement extends ENamedElement {}
interface EStructuralFeature extends ETypedElement {
    interface Setting {}
}
abstract class DelegatingEcoreEList<E> extends DelegatingNotifyingInternalEListImpl<E>
        implements InternalEList.Unsettable<E>, EStructuralFeature.Setting {}

public abstract class DelegatingFeatureMapMRO extends DelegatingEcoreEList<FeatureMap.Entry>
  implements FeatureMap.Internal, FeatureMap.Internal.Wrapper {}

Then

>>> from javatests import DelegatingFeatureMapMRO
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: Supertypes that share a modified attribute have an MRO conflict[attribute=remove, supertypes=[<type 'java.util.List'>, <type 'java.util.AbstractList'>], type=DelegatingFeatureMapMRO]

Given that java.util.AbstractList implements java.util.List, there should be no conflict.

Note that as in #1878, some inheritance complexity is required to demonstrate the bug. A simple test class like so presents no problems:

package javatests;

import java.util.AbstractList;
import java.util.List;

abstract class DiamondAbstractList extends AbstractList implements List {}

>>> from javatests import DiamondAbstractList
>>>

Presumably in between the two test classes is one of the appropriate complexity to trigger this problem.
msg10569 (view) Author: Jim Baker (zyasoft) Date: 2015-12-29.17:54:50
Possibly related to #2391, which maintains ordering in attributes. In particular I'm suspicious of the code in PyJavaType#handleMroError.
msg10576 (view) Author: Tomas Lukosius (tomluk) Date: 2015-12-30.07:36:19
After some debugging i found out that might be some problem in method org.python.core.PyType#computeMro(org.python.core.PyType.MROMergeState[], java.util.List<org.python.core.PyObject>) when merging mro's, but i'am not fully sure
msg10642 (view) Author: Jim Baker (zyasoft) Date: 2016-01-13.18:52:48
Not related to the recently fixed #2391

Tomas, right, it's just part of the overall merge mechanism, and what appears to be overeager conflict detection when none actually exists.
msg10650 (view) Author: Tomas Lukosius (tomluk) Date: 2016-01-15.14:51:33
Is there an expected date, when this bug could be fixed?
msg10653 (view) Author: Jim Baker (zyasoft) Date: 2016-01-15.15:25:39
We should be able to fix in 2.7.2. The timeline for that release is not yet fixed, although I would expect approximately 6 months from now, given that it's best to keep our schedules roughly aligned with PyCon.
msg10944 (view) Author: Barry-Michael van Wyk (bmvanwyk) Date: 2016-09-07.14:12:11
This issue can easily be triggered by importing the IBM MQ Java class:

import com.ibm.mq.jms.MQQueue

TypeError: Supertypes that share a modified attribute have an MRO conflict[attribute=get, supertypes=[<type 'java.util.Map'>, <type 'com.ibm.msg.client.jms.admin.JmsJndiDestinationImpl'>, <type 'com.ibm.msg.client.jms.admin.JmsDestinationImpl'>, <type 'com.ibm.msg.client.jms.internal.JmsPropertyContextImpl'>, <type 'com.ibm.msg.client.jms.JmsPropertyContext'>, <type 'com.ibm.msg.client.jms.JmsDestination'>, <type 'com.ibm.mq.jms.MQDestination'>, <type 'com.ibm.msg.client.jms.JmsQueue'>], type=MQQueue]

Works fine on Jython 2.5, but not on Jython 2.7
msg10945 (view) Author: Jim Baker (zyasoft) Date: 2016-09-08.23:38:56
We were hoping 2.7.2 would be released by now (msg10653), but we are obviously still working on getting 2.7.1 released. I still see this as a 2.7.2 priority.
msg11581 (view) Author: Jim Baker (zyasoft) Date: 2017-09-09.22:39:10
Still a problem in Jython 2.7.x latest, using the test class from msg10568
msg11591 (view) Author: Jim Baker (zyasoft) Date: 2017-09-11.22:26:21
So likely the solution here is to add a type unification step to PyJavaType#handleMroError, such that if for some pairing of types T and U, if type T is reported to conflict with type U, but T is a subclass or subinterface of U, then ignore as a MRO conflict error (because it's not). For some reason, the MRO conflict detection reports false positives, and we already have one workaround for this with respect to __iter__. I suspect these all seem to revolve around methods that are part of the Python integration support seen in JavaProxyList, etc. In particular, remove was added to this support in 2.7; it was not part of 2.5, so this adds further evidence to why this conflict now exists.

This type test can be readily done with https://docs.oracle.com/javase/7/docs/api/java/lang/Class.html#isAssignableFrom(java.lang.Class)
msg11592 (view) Author: Jim Baker (zyasoft) Date: 2017-09-12.23:40:47
My suggestion in msg11591 works as expected. Need to look at some additional testing, but it should be part of 2.7.2:

diff -r 8d52ad2559f5 src/org/python/core/PyJavaType.java
--- a/src/org/python/core/PyJavaType.java	Sat Sep 09 14:02:01 2017 -0600
+++ b/src/org/python/core/PyJavaType.java	Tue Sep 12 17:38:22 2017 -0600
@@ -187,7 +187,15 @@
                     PyList types = new PyList();
                     Set<Class<?>> proxySet = Generic.set();
                     for (PyJavaType othertype : conflictedAttributes) {
-                        if (othertype.modified != null && othertype.modified.contains(method)) {
+                        /* Ignore any pairings of types that are in a superclass/superinterface
+                         * relationship with each other. This problem is a false positive that happens
+                         * because of the automatic addition of methods so that Java classes behave
+                         * more like their corresponding Python types, such as adding sort or remove.
+                         * See http://bugs.jython.org/issue2445
+                         */
+                        if (othertype.modified != null && othertype.modified.contains(method) &&
+                            !(othertype.getProxyType().isAssignableFrom(type.getProxyType()) ||
+                                type.getProxyType().isAssignableFrom(othertype.getProxyType()))) {
                             types.add(othertype);
                             proxySet.add(othertype.getProxyType());
                         }
@@ -199,13 +207,16 @@
                      * __iter__. Annoying but necessary logic. See http://bugs.jython.org/issue1878
                      */
                     if (method.equals("__iter__")
-                            && proxySet.equals(Generic.set(Iterable.class, Map.class))) {
+                            && Generic.set(Iterable.class, Map.class).containsAll(proxySet)) {
                         continue;
                     }
-                    throw Py.TypeError(String.format(
-                            "Supertypes that share a modified attribute "
-                                    + "have an MRO conflict[attribute=%s, supertypes=%s, type=%s]",
-                            method, types, this.getName()));
+
+                    if (types.size() > 0) {
+                        throw Py.TypeError(String.format(
+                                "Supertypes that share a modified attribute "
+                                        + "have an MRO conflict[attribute=%s, supertypes=%s, type=%s]",
+                                method, types, this.getName()));
+                    }
                 }
             }
         }
msg11593 (view) Author: Jim Baker (zyasoft) Date: 2017-09-12.23:42:52
In particular, this type unification seems to work as expected with the preceding diff. No additional computational complexity is seen here in the pairwise conflict detection (O(n^2)), although perhaps we could reduce to O(n) via union-find for type unification. But in practice we have not seen this blow up, and I will hand wave to state that the sets of types in conflict that are seen in practice, even with Eclispe or Clojure, are small, maybe on the order of 2 or 3 elements.
msg11746 (view) Author: Jeff Allen (jeff.allen) Date: 2018-03-06.07:35:48
Seems like a lot of work has gone into getting this to the 90% mark. Any chance you could do the other 90% in time for 2.7.2? (Not that we have a specific time in mind yet.)
msg12062 (view) Author: Werner Fouché (wfouche2) Date: 2018-07-23.18:14:03
Importing IBM MQ Java client libraries continue to fail with the latest Jython source code updated to change https://hg.python.org/jython/rev/86b859301121

Example on Windows:

set CLASSPATH=com.ibm.mqjms-7.0.1.jar;jms-1.1.jar;com.ibm.mq.jmqi-7.0.1.jar

jython -V
Jython 2.7.2a1+

jython -c "import com.ibm.mq.jms.MQQueue"

TypeError: Supertypes that share a modified attribute have an MRO conflict[attribute=get, supertypes=[<type 'com.ibm.msg.client.jms.internal.JmsPropertyContextImpl'>, <type 'com.ibm.msg.client.jms.JmsPropertyContext'>, <type 'com.ibm.msg.client.jms.admin.JmsJndiDestinationImpl'>, <type 'com.ibm.msg.client.jms.admin.JmsDestinationImpl'>, <type 'com.ibm.msg.client.jms.JmsQueue'>, <type 'java.util.Map'>, <type 'com.ibm.mq.jms.MQDestination'>, <type 'com.ibm.msg.client.jms.JmsDestination'>], type=MQQueue]

What remains to do be done to implement a 100% fix?
msg12063 (view) Author: Werner Fouché (wfouche2) Date: 2018-07-23.19:11:34
The IBM MQ client libraries are available from: https://developer.ibm.com/messaging/learn-mq/mq-tutorials/develop-mq-jms/

New example on Windows:

set CLASSPATH=com.ibm.mq.allclient-9.0.4.0.jar;javax.jms-api-2.0.1.jar

jython -V
Jython 2.7.2a1+

jython -c "import com.ibm.mq.jms.MQQueue"

TypeError: Supertypes that share a modified attribute have an MRO conflict[attribute=values, supertypes=[<type 'com.ibm.msg.client.jms.admin.JmsJndiDestinationImpl'>, <type 'java.util.Map'>, <type 'com.ibm.msg.client.jms.internal.JmsPropertyContextImpl'>, <type 'com.ibm.msg.client.jms.JmsQueue'>, <type 'com.ibm.msg.client.jms.JmsPropertyContext'>, <type 'com.ibm.msg.client.jms.admin.JmsDestinationImpl'>, <type 'com.ibm.mq.jms.MQDestination'>, <type 'com.ibm.msg.client.jms.JmsDestination'>], type=MQQueue]
msg12364 (view) Author: Jeff Allen (jeff.allen) Date: 2019-03-17.09:03:04
I intend to try Jim's fix for this, but first to create a failing test.

We seem only to lack a concise test that does not depend on external products. Jim has given us an independent test, but it lacks conciseness. I'll start with that and try tofigure out what minimal subset properly exercises the defect.
msg12366 (view) Author: Werner Fouché (wfouche2) Date: 2019-03-18.06:12:54
https://bugs.jython.org/issue2445
msg12368 (view) Author: Jeff Allen (jeff.allen) Date: 2019-03-18.06:26:25
I derived a simplified (but still failing) equivalent and made it the basis of an addition to test_java_integration. Jim's patch as posted fixes that without my having to understand too much!

Now in at: https://hg.python.org/jython/rev/3c4f2c14b808 (Thanks Jim.)
msg12396 (view) Author: Werner Fouché (wfouche2) Date: 2019-03-25.14:06:41
Jim / Jeff, thanks for the continued effort to resolve this issue.

The IBM MQ example however still fail with the latest fixes applied.

set CLASSPATH=com.ibm.mq.allclient-9.0.4.0.jar;javax.jms-api-2.0.1.jar

jython -V
Jython 2.7.2a1+

jython -c "import com.ibm.mq.jms.MQQueue"

TypeError: Supertypes that share a modified attribute have an MRO conflict[attribute=get, supertypes=[<type 'com.ibm.msg.client.jms.internal.JmsPropertyContextImpl'>], type=MQQueue]

The good news is that it is now only failing on JmsPropertyContextImpl. 

Close, very close to being resolved it seems!
msg12436 (view) Author: Jeff Allen (jeff.allen) Date: 2019-04-14.07:07:53
It looks like I have to understand this after all. Here are some initial thoughts.

Having read a bit (https://python-history.blogspot.com/2010/06/method-resolution-order.html), (https://en.wikipedia.org/wiki/C3_linearization), Barrett 1996, (https://opendylan.org/documentation/intro-dylan/methods-generic-functions.html), my first question-to-self is why we are using C3 to resolve the MRO of classes defined in Java.

A class defined in Python having Java bases should use C3 at that step, but here we are tripping over a class validly defined in Java. In addressing the merge for com.ibm.mq.jms.MQQueue, we attempt to merge these lists:

[javax.jms.Queue, javax.jms.Destination, java.lang.Object, object]

[com.ibm.msg.client.jms.JmsQueue, com.ibm.msg.client.jms.JmsDestination, com.ibm.msg.client.jms.JmsPropertyContext, com.ibm.msg.client.jms.JmsReadablePropertyContext, java.io.Serializable, java.util.Map, javax.jms.Queue, javax.jms.Destination, java.lang.Object, object]

[com.ibm.mq.jms.MQDestination, com.ibm.jms.JMSDestination, com.ibm.msg.client.jms.admin.JmsJndiDestinationImpl, javax.naming.Referenceable, com.ibm.msg.client.jms.admin.JmsDestinationImpl, com.ibm.msg.client.jms.JmsDestination, com.ibm.msg.client.jms.internal.JmsPropertyContextImpl, com.ibm.msg.client.jms.JmsPropertyContext, com.ibm.msg.client.provider.ProviderPropertyContextCallback, com.ibm.msg.client.jms.internal.JmsReadablePropertyContextImpl, com.ibm.msg.client.jms.JmsReadablePropertyContext, java.io.Serializable, java.util.Map, javax.jms.Destination, java.lang.Object, object]

[javax.jms.Queue, com.ibm.msg.client.jms.JmsQueue, com.ibm.mq.jms.MQDestination]

The last of these is the bases of com.ibm.mq.jms.MQQueue and the first 3 are the MROs of these bases. Each of the list heads is in one of the list tails, and so C3 falls at the first hurdle.

Java allows multiple inheritance from interfaces, but the resolution strategy is not C3. Ordering of bases is (I think) irrelevant, since at most one ancestor can bring an implementation. The exception to this (that I know about) relates to constants. Constants of the same name can have conflicting definitions by inheritance from interfaces. Python would decide which one wins using order: Java would make it illegal to use the ambiguous name (at compile time).

If Java is insensitive to the ordering of bases, complex heterarchies defined in Java will surely turn up a lot of examples that Java has allowed but C3 rejects. Shouldn't we be using Java rules for Java classes?

Also, I really don't see why, if the problem is that the *classes* can't be merged by C3, a solution should be sought in the examination of particular clashing *attributes*. When we find such a clash, it appears we force our MRO to accept a candidate C3 would not, then continue from there. This forcing obviously moves us towards termination, but I don't see the logic of the particular choice.
msg12438 (view) Author: Jim Baker (zyasoft) Date: 2019-04-15.14:52:21
@Jeff, +1 on your analysis on using Java rules for Java classes. Good to look at this problem freshly. With respect to constants, it would seem defender methods would also come into play here, but regardless we could select using C3 with a Python class that implemented Java interfaces as bases.
msg12440 (view) Author: Jeff Allen (jeff.allen) Date: 2019-04-16.22:16:59
@Jim Thanks for taking a look. It seems a bit radical to say that we've been doing it wrong for 10 years (https://hg.python.org/jython/rev/f3660ced0608). But if you don't see the flaw, having worked on it, it's probably worth my trying.

We have a fair amount of test around this, so it will tell me if I go wrong.

Characteristically, I'll start by explaining to myself (in comments) how it works now.
msg12444 (view) Author: Jeff Allen (jeff.allen) Date: 2019-04-19.17:36:29
I've restructured handleMroError for readability, and I see what it does to fix the C3 merge conflict, which is simple enough. https://hg.python.org/jython/rev/262428b2c51a

I have no idea why we go looking for same-named methods at this point. It does nothing to solve the merge problem, which is structural amongst classes irrespective of their methods.

I understand why conflicting definitions arriving through different bases might be an issue. I don't see why we only check for it once C3 MRO generation has stalled. If it is necessary at all, why don't we do it unconditionally when (re-)calculating the MRO?

In an interesting simple case, handleMroError is called for org.python.util.PythonInterpreter, and the state is:
[java.lang.AutoCloseable, java.lang.Object, object]
[java.io.Closeable, java.lang.AutoCloseable, java.lang.Object, object]
[java.lang.Object, object]
[java.lang.AutoCloseable, java.io.Closeable, java.lang.Object]

Neither type in conflict here [<type 'java.lang.AutoCloseable'>, <type 'java.io.Closeable'>] has a .modified list, but on return each has a .conflicted list accusing the other.

Now Closeable extends AutoCloseable, so we might as well have declared that PythonInterpreter implements just one of those. In that case we would never have called handleMroError. But then .conflicted would not be set on Closeable and AutoCloseable. If the state of these two type objects varies in response to an inconsequential (compile-time) change in a third class, we must be doing something wrong.

The only other uses of .conflicted and .modified (in Java, anyway) are in type___setattr__ and type___delattr__, where .conflicted is consulted (but not updated) and .modified consulted and updated with the name added/deleted.

handleMroErrorappears to be the *only* place we set .conflicted, and whether or not we do so appears to be a matter of chance unrelated to the possession of conflictingly-named methods. 
It leaves me wondering what could possibly go wrong if we simply removed this whole test and the .conflicted attribute.
msg12450 (view) Author: Jeff Allen (jeff.allen) Date: 2019-04-26.07:45:50
This continues to provide interest. I can only guess at the reasoning that led to performing this test on the .modified attributes once C3 has failed. However, it seems questionable, in that:

1. A modified attribute may conflict with a method defined in Java (e.g. proxy method __repr__ added to Map does this, and many others).

2. Since https://hg.python.org/jython/rev/f3660ced0608#l3.131, we have been changing order Java provides, placing the superclass last in the bases. :/

I think the motive for 2. is to give to give proxy methods added to interfaces Map, List, etc. precedence over implementations provided from Java, but it makes what we're doing with C3 and the MRO even less likely to match Java. I'll experiment with reversing this in circumstances where all the conflicts are reported.
msg12456 (view) Author: Jeff Allen (jeff.allen) Date: 2019-04-26.22:10:20
I wrote a diagnostic that finds all methods with conflicting definitions in a set of bases, and applied it to the bases in PyJavaType.init, just before we compute the MRO. Taking the example of a List type and method __eq__, we find a conflict but that the definition added to java.util.List wins:

>>> import java
>>> AL = java.util.ArrayList
java.util.AbstractList.__eq__ defined by:
  <type 'java.util.List'> as <method '__eq__' of 'java.util.List' objects>
  <type 'java.util.AbstractCollection'> as <method '__eq__' of 'java.lang.Object' objects>

The first base in the list is the one where __eq__ was found in .modified, and the others are those bases that disagree with it. The actual order of bases has te super-class last:
>>> AL.__bases__
(<type 'java.util.RandomAccess'>, <type 'java.lang.Cloneable'>, <type 'java.io.Serializable'>, <type 'java.util.AbstractList'>)
>>> AL.__mro__
(<type 'java.util.ArrayList'>, <type 'java.util.RandomAccess'>, <type 'java.lang.Cloneable'>, <type 'java.io.Serializable'>, <type 'java.util.AbstractList'>, <type 'java.util.List'>, <type 'java.util.AbstractCollection'>, <type 'java.util.Collection'>, <type 'java.lang.Iterable'>, <type 'java.lang.Object'>, <type 'object'>)
>>> AL.__eq__
<method '__eq__' of 'java.util.List' objects>


If we put the order of bases back to its earlier Java-like order (super-class then interfaces), and run this again, we see the expected change in __bases__ and consequences in the MRO:
>>> AL.__bases__
(<type 'java.util.AbstractList'>, <type 'java.util.RandomAccess'>, <type 'java.lang.Cloneable'>, <type 'java.io.Serializable'>)
>>> AL.__mro__
(<type 'java.util.ArrayList'>, <type 'java.util.AbstractList'>, <type 'java.util.AbstractCollection'>, <type 'java.util.List'>, <type 'java.util.Collection'>, <type 'java.lang.Iterable'>, <type 'java.util.RandomAccess'>, <type 'java.lang.Cloneable'>, <type 'java.io.Serializable'>, <type 'java.lang.Object'>, <type 'object'>)

However, the effective definition of __eq__ is still the one added to the proxy of List:
>>> AL.__eq__
<method '__eq__' of 'java.util.List' objects>

The exciting change is that we no longer fail the MQQueue import. :)
PS> dist\bin\jython -S -c "from org.python.tests.mro import IBMMQChallengeMRO as m; t=m.mq.jms.MQQueue"

However, we get an apparent regression in the handling of diamond inheritance of Map and Iterable (#1878) in test_java_integration. This regression may not be real: the test looks for a specific MRO, and of course the MRO has changed, but the essence of #1878 is that when a Map defines an __iter__ the operative definition should still come from (our proxy for) Iterable. And it does:

PS> dist\bin\jython -S -i -c "from javatests import DiamondIterableMapMRO"
>>> DiamondIterableMapMRO.__iter__
<method '__iter__' of 'java.lang.Iterable' objects>

The import of DiamondIterableMapMRO makes plenty of visits to handleMroError (on behalf of the class and its ancestors) since C3 stalls on the Java-like heterarchy. With the diagnostic turned on we can see the potential conflict over __iter__:
javatests.DiamondIterableMapMRO.__iter__ defined by:
  <type 'java.util.Map'> as <method '__iter__' of 'java.util.Map' objects>
  <type 'javatests.IPersistentMap'> as <method '__iter__' of 'java.lang.Iterable' objects>

However, the final MRO is such that <method '__iter__' of 'java.lang.Iterable' objects> wins:
>>> DiamondIterableMapMRO.__mro__
(<type 'javatests.DiamondIterableMapMRO'>, <type 'javatests.AFn'>, <type 'javatests.IFn'>, <type 'java.util.concurrent.Callable'>, <type 'java.lang.Runnable'>, <type 'javatests.ILookup'>, <type 'javatests.IPersistentMap'>, <type 'java.lang.Iterable'>, <type 'javatests.Associative'>, <type 'javatests.IPersistentCollection'>, <type 'javatests.Seqable'>, <type 'javatests.Counted'>, <type 'java.util.Map'>, <type 'java.lang.Object'>, <type 'object'>)

This makes me think that test_diamond_inheritance_of_iterable_and_map should not be testing for an exact MRO, only that Iterable precedes Map.
msg12458 (view) Author: Jim Baker (zyasoft) Date: 2019-04-27.01:34:45
@Jeff, I wrote test_diamond_inheritance, so I can address this question reasonably well. The MRO assertion is most likely wrong, given this change in C3 vs Java resolution.

However, I believe the MRO computation should still be deterministic, based on the ordering of bases. Perhaps separate out in 2 assertions or even 2 tests?

* Precedence of Iterable before Map
* Determinism of MRO computation

I would still recommend using stringification, then unpack for the actual precedence, eg mro_str[1:-1].split(", "), etc
msg12460 (view) Author: Jeff Allen (jeff.allen) Date: 2019-04-27.11:32:11
Thanks @Jim. I realised this was yours, so thanks for the look over.

I don't see anything non-deterministic, e.g. iterating a HashSet. From a certain perspective the MRO is unstable since we have changed it for the next release. I would say it is closer to Java ordering, but there are aspects Java that make me think there is no "correct MRO" reproducing it exactly.

This whole issue raises questions for me about the correct relationship between Java classes and their Python proxies. Users (and we ourselves) tend to assume the inheritance relationships must carry over exactly, and whilst we can approximate that expectation, there is a line beyond which Python is different and it isn't a bug or a TypeError. We have not been clear where the line runs.

Speaking of test_diamond_inheritance_of_iterable_and_map, can you explain the comment:

"Also instead of directly importing, which would cause annoying bloat in javatests by making lots of little files, just match using str - this will still be stable/robust."

By the time we make an MRO, we've loaded and exposed all the classes and methods involved, so I'm not sure where the additional "little files" come in (or little objects). I'm going to stick with the string approach anyway and simply find the class names.
msg12468 (view) Author: Jeff Allen (jeff.allen) Date: 2019-04-28.16:59:33
Fixed, I claim: https://hg.python.org/jython/rev/36db7416b3b7

@Werner I'm confident MQQueue will import, but would you see if it behaves too?

The historic work-arounds seem still to be necessary. The special-case for Map/Iterable still is and I had to add one for Enumeration/Iterator. I have not tried to remove the one based on assignableFrom() Jim added, nor make other inessential improvements that ocurred to me. Maybe in another change set.

If it's not obvious yet, I think that when it comes to treating Java collections as Python ones, or giving proxied Java objects whatever API, explicit would be better than implicit. E.g. m.map really is (only) a Python map, but wraps the Map that m is proxy for, while m has exactly the methods its type requires. (Choose name/syntax.)

Also, I don't think I'm able to express "Java rules" in MRO-speak. It may be possible (suspect not), but by far the most reliable course is surely to ask Java itself what is inherited and from where. Not for 2.7.2 I think.
History
Date User Action Args
2019-04-28 16:59:33jeff.allensetstatus: open -> pending
resolution: accepted -> fixed
messages: + msg12468
title: Eclipse's DelegatingFeatureMap has MRO conflict -> Eclipse's DelegatingFeatureMap has MRO conflict (and IBM's MQQueue)
2019-04-27 11:32:11jeff.allensetmessages: + msg12460
2019-04-27 01:34:45zyasoftsetmessages: + msg12458
2019-04-26 22:10:20jeff.allensetmessages: + msg12456
2019-04-26 07:45:51jeff.allensetmessages: + msg12450
2019-04-19 17:36:30jeff.allensetmessages: + msg12444
2019-04-16 22:16:59jeff.allensetmessages: + msg12440
2019-04-15 14:52:22zyasoftsetmessages: + msg12438
2019-04-14 07:07:54jeff.allensetmessages: + msg12436
2019-03-31 10:18:07jeff.allensetstatus: pending -> open
resolution: fixed -> accepted
2019-03-25 14:06:41wfouche2setmessages: + msg12396
2019-03-18 06:26:25jeff.allensetstatus: open -> pending
resolution: accepted -> fixed
messages: + msg12368
2019-03-18 06:12:55wfouche2setmessages: + msg12366
2019-03-17 09:03:04jeff.allensetassignee: jeff.allen
resolution: remind -> accepted
messages: + msg12364
2018-07-23 19:11:34wfouche2setmessages: + msg12063
2018-07-23 18:14:04wfouche2setnosy: + wfouche2
messages: + msg12062
2018-03-06 07:35:49jeff.allensetnosy: + jeff.allen
messages: + msg11746
2017-09-12 23:42:52zyasoftsetmessages: + msg11593
2017-09-12 23:40:48zyasoftsetmessages: + msg11592
2017-09-11 22:26:22zyasoftsetmessages: + msg11591
2017-09-09 22:39:10zyasoftsetresolution: remind
messages: + msg11581
2016-09-08 23:38:57zyasoftsetmessages: + msg10945
2016-09-07 14:12:12bmvanwyksetnosy: + bmvanwyk
messages: + msg10944
2016-01-15 15:25:39zyasoftsetpriority: high
messages: + msg10653
milestone: Jython 2.7.2
2016-01-15 14:51:33tomluksetmessages: + msg10650
2016-01-13 18:52:49zyasoftsetmessages: + msg10642
2015-12-30 07:36:19tomluksetmessages: + msg10576
2015-12-29 17:54:50zyasoftsetmessages: + msg10569
2015-12-29 17:45:19zyasoftsetmessages: + msg10568
2015-12-29 16:08:05zyasoftsetmessages: + msg10567
title: Eclipse's DelegatingFeatureMap is reported as having a MRO conflict -> Eclipse's DelegatingFeatureMap has MRO conflict
2015-12-29 15:23:16tomluksetmessages: + msg10566
2015-12-29 14:51:15zyasoftsetnosy: + zyasoft
messages: + msg10565
title: MRO conflict when importing class from library -> Eclipse's DelegatingFeatureMap is reported as having a MRO conflict
2015-12-29 13:30:19tomluksetmessages: + msg10561
2015-12-29 13:20:54tomlukcreate