From: <cg...@us...> - 2009-01-25 12:19:33
|
Revision: 5975 http://jython.svn.sourceforge.net/jython/?rev=5975&view=rev Author: cgroves Date: 2009-01-25 12:19:27 +0000 (Sun, 25 Jan 2009) Log Message: ----------- Use Python's mro to determine an mro for Java classes and interfaces. If the Java class has an mro conflict, resolve it by arbitrarily choosing an interface or class. If an interface or class involved in an mro conflict has had its dict modified, or has its dict modified, raise a TypeError. Modified Paths: -------------- trunk/jython/Lib/test/test_import_jy.py trunk/jython/Lib/test/test_java_integration.py trunk/jython/src/org/python/core/PyJavaType.java trunk/jython/src/org/python/core/PyType.java Modified: trunk/jython/Lib/test/test_import_jy.py =================================================================== --- trunk/jython/Lib/test/test_import_jy.py 2009-01-25 12:15:25 UTC (rev 5974) +++ trunk/jython/Lib/test/test_import_jy.py 2009-01-25 12:19:27 UTC (rev 5975) @@ -8,6 +8,7 @@ import sys import tempfile import unittest +import subprocess from test import test_support from test_chdir import read, safe_mktemp, COMPILED_SUFFIX @@ -148,7 +149,11 @@ # causes a stack overflow if the bug occurs self.assertRaises(Exception, getattr, anygui, 'abc') + def test_import_star(self): + self.assertEquals(subprocess.call([sys.executable, + test_support.findfile("import_star_from_java.py")]), 0) + def test_main(): test_support.run_unittest(MislabeledImportTestCase, OverrideBuiltinsImportTestCase, Modified: trunk/jython/Lib/test/test_java_integration.py =================================================================== --- trunk/jython/Lib/test/test_java_integration.py 2009-01-25 12:15:25 UTC (rev 5974) +++ trunk/jython/Lib/test/test_java_integration.py 2009-01-25 12:19:27 UTC (rev 5975) @@ -20,6 +20,7 @@ from org.python.core.util import FileUtil from org.python.tests import BeanImplementation, Child, Listenable, CustomizableMapHolder +from org.python.tests.mro import (ConfusedOnGetitemAdd, FirstPredefinedGetitem, GetitemAdder) class InstantiationTest(unittest.TestCase): def test_cant_instantiate_abstract(self): @@ -411,11 +412,19 @@ self.assertEquals(7, m.held["initial"], "Existing fields should still be accessible") self.assertEquals(7, m.initial) self.assertEquals(None, m.nonexistent, "Nonexistent fields should be passed on to the Map") + + def test_adding_on_interface(self): + GetitemAdder.addPredefined() + class UsesInterfaceMethod(FirstPredefinedGetitem): + pass + self.assertEquals("key", UsesInterfaceMethod()["key"]) + + def test_add_on_mro_conflict(self): + """Adding same-named methods to Java classes with MRO conflicts produces TypeError""" + GetitemAdder.addPredefined() + self.assertRaises(TypeError, __import__, "org.python.tests.mro.ConfusedOnImport") + self.assertRaises(TypeError, GetitemAdder.addPostdefined) - def test_import_star(self): - self.assertEquals(subprocess.call([sys.executable, - test_support.findfile("import_star_from_java.py")]), 0) - def test_main(): test_support.run_unittest(InstantiationTest, BeanTest, Modified: trunk/jython/src/org/python/core/PyJavaType.java =================================================================== --- trunk/jython/src/org/python/core/PyJavaType.java 2009-01-25 12:15:25 UTC (rev 5974) +++ trunk/jython/src/org/python/core/PyJavaType.java 2009-01-25 12:19:27 UTC (rev 5975) @@ -35,6 +35,19 @@ private static Map<Class<?>, PyBuiltinMethod[]> collectionProxies; + /** + * Other Java classes this type has MRO conflicts with. This doesn't matter for Java method + * resolution, but if Python methods are added to the type, the added methods can't overlap with + * methods added to any of the types in this set. If this type doesn't have any known conflicts, + * this is null. + */ + private Set<PyJavaType> conflicted; + + /** + * The names of methods that have been added to this class. + */ + private Set<String> modified; + public static PyObject wrapJavaObject(Object o) { PyObject obj = new PyObjectDerived(PyType.fromClass(o.getClass())); obj.javaProxy = o; @@ -62,11 +75,101 @@ return !(attr instanceof PyReflectedField || attr instanceof PyReflectedFunction); } - PyObject[] compute_mro() { - return mro; + @Override + public void addMethod(PyBuiltinMethod meth) { + if (modified == null) { + modified = Generic.set(); + } + if (modified.add(meth.info.getName())) { + if (conflicted != null) { + for (PyJavaType conflict : conflicted) { + if (conflict.modified != null + && conflict.modified.contains(meth.info.getName())) { + throw Py.TypeError(getName() + + " does not have a consistent method resolution" + " order with " + + conflict.getName() + ", and it already has " + + meth.info.getName() + " added for Python"); + } + } + } + } + super.addMethod(meth); } @Override + public void removeMethod(PyBuiltinMethod meth) { + super.removeMethod(meth); + if (modified != null) { + modified.remove(meth.info.getName()); + } + } + + @Override + void handleMroError(MROMergeState[] toMerge, List<PyObject> mro) { + if (underlying_class != null) { + // If this descends from PyObject, don't do the Java mro cleanup + super.handleMroError(toMerge, mro); + } + Set<PyJavaType> inConflict = Generic.set(); + PyJavaType winner = null; + for (MROMergeState mergee : toMerge) { + for (int i = mergee.next; i < mergee.mro.length; i++) { + if (mergee.mro[i] == PyObject.TYPE + || mergee.mro[i] == PyType.fromClass(Object.class)) { + continue; + } + if (winner == null) { + // Pick an arbitrary class to be added to the mro next and break the conflict. + // If method name conflicts were allowed between methods added to Java types, + // it would go first, but that's prevented, so being a winner doesn't actually + // get it anything + winner = (PyJavaType)mergee.mro[i]; + } + inConflict.add((PyJavaType)mergee.mro[i]); + } + } + + Set<String> allModified = Generic.set(); + PyJavaType[] conflicted = inConflict.toArray(new PyJavaType[inConflict.size()]); + for (PyJavaType type : conflicted) { + if (type.modified == null) { + continue; + } + for (String method : type.modified) { + if (!allModified.add(method)) { // Another type in conflict has this method, fail + PyList types = new PyList(); + for (PyJavaType othertype : conflicted) { + if (othertype.modified != null && othertype.modified.contains(method)) { + types.add(othertype); + } + } + throw Py.TypeError(String.format("Supertypes that share a method " + + " have an MRO conflict[method=%s, types=%s]", method, types)); + } + } + } + + // We can keep trucking, there aren't any existing method name conflicts. Mark the + // conflicts in all the classes so further method additions can check for trouble + for (PyJavaType type : conflicted) { + for (PyJavaType otherType : inConflict) { + if (otherType != type) { + if (type.conflicted == null) { + type.conflicted = Generic.set(); + } + type.conflicted.add(otherType); + } + } + } + + // Add our winner to the mro, clear the clog, and try to finish the rest + mro.add(winner); + for (MROMergeState mergee : toMerge) { + mergee.removeFromUnmerged(winner); + } + computeMro(toMerge, mro); + } + @Override protected void init(Class<?> forClass) { name = forClass.getName(); // Strip the java fully qualified class name from Py classes in core @@ -83,31 +186,30 @@ } else { javaProxy = forClass; objtype = PyType.fromClass(Class.class); - // Wrapped Java types fill in their mro first using their base class and then all of - // their interfaces. - if (baseClass == null) { + // Wrapped Java types fill in their mro first using all of their interfaces then their + // super class. + List<PyObject> visibleBases = Generic.list(); + for (Class<?> iface : forClass.getInterfaces()) { + if (iface == PyProxy.class || iface == ClassDictInit.class) { + // Don't show the interfaces added by proxy type construction; otherwise Python + // subclasses of proxy types and another Java interface can't make a consistent + // mro + continue; + } + visibleBases.add(PyType.fromClass(iface)); + } + if (javaProxy == Object.class) { base = PyType.fromClass(PyObject.class); - } else if (javaProxy == Class.class) { + } else if(baseClass == null) { + base = PyType.fromClass(Object.class); + }else if (javaProxy == Class.class) { base = PyType.fromClass(PyType.class); } else { base = PyType.fromClass(baseClass); } - bases = new PyObject[1 + forClass.getInterfaces().length]; - bases[0] = base; - for (int i = 1; i < bases.length; i++) { - bases[i] = PyType.fromClass(forClass.getInterfaces()[i - 1]); - } - Set<PyObject> seen = Generic.set(); - List<PyObject> mros = Generic.list(); - mros.add(this); - for (PyObject obj : bases) { - for (PyObject mroObj : ((PyType)obj).mro) { - if (seen.add(mroObj)) { - mros.add(mroObj); - } - } - } - mro = mros.toArray(new PyObject[mros.size()]); + visibleBases.add(base); + this.bases = visibleBases.toArray(new PyObject[visibleBases.size()]); + mro = computeMro(); } // PyReflected* can't call or access anything from non-public classes that aren't in @@ -392,7 +494,8 @@ || getDescrMethod(forClass, "_doset", OO) != null; has_delete = getDescrMethod(forClass, "__delete__", PyObject.class) != null || getDescrMethod(forClass, "_dodel", PyObject.class) != null; - } else { + } + if (forClass == Object.class) { // Pass __eq__ and __repr__ through to subclasses of Object addMethod(new PyBuiltinMethodNarrow("__eq__", 1) { @Override @@ -468,42 +571,39 @@ */ private void handleSuperMethodArgCollisions(Class<?> forClass) { for (Class<?> iface : forClass.getInterfaces()) { - for (Method meth : iface.getMethods()) { - if (!Modifier.isPublic(meth.getDeclaringClass().getModifiers())) { - // Ignore methods from non-public interfaces as they're similarly bugged - continue; + mergeMethods(iface); + } + if (forClass.getSuperclass() != null) { + mergeMethods(forClass.getSuperclass()); + } + } + + private void mergeMethods(Class<?> parent) { + for (Method meth : parent.getMethods()) { + if (!Modifier.isPublic(meth.getDeclaringClass().getModifiers())) { + // Ignore methods from non-public interfaces as they're similarly bugged + continue; + } + String nmethname = normalize(meth.getName()); + PyObject[] where = new PyObject[1]; + PyObject obj = lookup_where(nmethname, where); + if (obj == null) { + // Nothing in our supertype hierarchy defines something with this name, so it + // must not be visible there. + continue; + } else if (where[0] == this) { + // This method is the only thing defining items in this class' dict, so it must + // be a PyReflectedFunction created here. See if it needs the current method + // added to it. + if (!((PyReflectedFunction)obj).handles(meth)) { + ((PyReflectedFunction)obj).addMethod(meth); } - String nmethname = normalize(meth.getName()); - PyObject[] where = new PyObject[1]; - PyObject obj = lookup_where(nmethname, where); - if (obj == null) { - // Nothing in our supertype hierarchy defines something with this name, so it - // must not be visible there. - continue; - } else if (where[0] == this) { - // This method is the only thing defining items in this class' dict, so it must - // be a PyReflectedFunction created here. See if it needs the current method - // added to it. - if (!((PyReflectedFunction)obj).handles(meth)) { - ((PyReflectedFunction)obj).addMethod(meth); - } - } else { - // There's something in a superclass with the same name. If this class extends a - // class and doesn't just implement something, the extended class is first in - // mro, so items defined on the extended class will show up here. Thanks to that - // and copying the base function, we can get away with just looping over - // interface methods. - PyReflectedFunction func; - if (obj instanceof PyReflectedFunction) { - func = ((PyReflectedFunction)obj).copy(); - if (!func.handles(meth)) { - func.addMethod(meth); - } - } else { - func = new PyReflectedFunction(meth); - } - dict.__setitem__(nmethname, func); - } + } else { + // There's something in a superclass with the same name. Add an item to this type's + // dict to hide it. If it's this method, nothing's changed. If it's a field, we + // want to make the method visible. If it's a different method, it'll be added to + // the reflected function created here in a later call. + dict.__setitem__(nmethname, new PyReflectedFunction(meth)); } } } Modified: trunk/jython/src/org/python/core/PyType.java =================================================================== --- trunk/jython/src/org/python/core/PyType.java 2009-01-25 12:15:25 UTC (rev 5974) +++ trunk/jython/src/org/python/core/PyType.java 2009-01-25 12:19:27 UTC (rev 5975) @@ -507,7 +507,7 @@ private void mro_internal() { if (getType() == TYPE) { - mro = compute_mro(); + mro = computeMro(); } else { PyObject mroDescr = getType().lookup("mro"); if (mroDescr == null) { @@ -652,55 +652,18 @@ return acc.toArray(new PyObject[acc.size()]); } - private static boolean tail_contains(PyObject[] lst, int whence, PyObject o) { - int n = lst.length; - for (int i = whence + 1; i < n; i++) { - if (lst[i] == o) { - return true; - } - } - return false; - } - - private static PyException mro_error(PyObject[][] to_merge, int[] remain) { - StringBuilder msg = new StringBuilder("Cannot create a consistent method resolution\n" - + "order (MRO) for bases "); - PyDictionary set = new PyDictionary(); - for (int i = 0; i < to_merge.length; i++) { - PyObject[] lst = to_merge[i]; - if (remain[i] < lst.length) { - set.__setitem__(lst[remain[i]], Py.None); - } - } - PyObject iter = set.__iter__(); - PyObject cur; - boolean subq = false; - while ((cur = iter.__iternext__()) != null) { - PyObject name = cur.__findattr__("__name__"); - if (!subq) { - subq = true; - } else { - msg.append(", "); - } - msg.append(name == null ? "?" : name.toString()); - } - return Py.TypeError(msg.toString()); - } - @ExposedMethod(defaults = "null", doc = BuiltinDocs.type_mro_doc) final PyList type_mro(PyObject o) { if (o == null) { - return new PyList(compute_mro()); + return new PyList(computeMro()); } - return new PyList(((PyType)o).compute_mro()); + return new PyList(((PyType)o).computeMro()); } - PyObject[] compute_mro() { - PyObject[] bases = this.bases; - int n = bases.length; - for (int i = 0; i < n; i++) { + PyObject[] computeMro() { + for (int i = 0; i < bases.length; i++) { PyObject cur = bases[i]; - for (int j = i + 1; j < n; j++) { + for (int j = i + 1; j < bases.length; j++) { if (bases[j] == cur) { PyObject name = cur.__findattr__("__name__"); throw Py.TypeError("duplicate base class " + @@ -709,58 +672,77 @@ } } - int nmerge = n + 1; - PyObject[][] to_merge = new PyObject[nmerge][]; - int[] remain = new int[nmerge]; - - for (int i = 0; i < n; i++) { - PyObject cur = bases[i]; - remain[i] = 0; - if (cur instanceof PyType) { - to_merge[i] = ((PyType)cur).mro; - } else if (cur instanceof PyClass) { - to_merge[i] = classic_mro((PyClass)cur); + MROMergeState[] toMerge = new MROMergeState[bases.length + 1]; + for (int i = 0; i < bases.length; i++) { + toMerge[i] = new MROMergeState(); + if (bases[i] instanceof PyType) { + toMerge[i].mro = ((PyType)bases[i]).mro; + } else if (bases[i] instanceof PyClass) { + toMerge[i].mro = classic_mro((PyClass)bases[i]); } } + toMerge[bases.length] = new MROMergeState(); + toMerge[bases.length].mro = bases; - to_merge[n] = bases; - remain[n] = 0; + List<PyObject> mro = Generic.list(); + mro.add(this); + return computeMro(toMerge, mro); + } - List<PyObject> acc = Generic.list(); - acc.add(this); - - int empty_cnt = 0; - - scan : for (int i = 0; i < nmerge; i++) { - PyObject candidate; - PyObject[] cur = to_merge[i]; - if (remain[i] >= cur.length) { - empty_cnt++; + PyObject[] computeMro(MROMergeState[] toMerge, List<PyObject> mro) { + scan : for (int i = 0; i < toMerge.length; i++) { + if (toMerge[i].isMerged()) { continue scan; } - candidate = cur[remain[i]]; - for (int j = 0; j < nmerge; j++) - if (tail_contains(to_merge[j], remain[j], candidate)) { + PyObject candidate = toMerge[i].getCandidate(); + for (MROMergeState mergee : toMerge) { + if(mergee.unmergedContains(candidate)) { continue scan; } - acc.add(candidate); - for (int j = 0; j < nmerge; j++) { - if (remain[j] < to_merge[j].length && to_merge[j][remain[j]] == candidate) { - remain[j]++; - } } - // restart scan - i = -1; - empty_cnt = 0; + mro.add(candidate); + for (MROMergeState element : toMerge) { + element.noteMerged(candidate); + } + i = -1;// restart scan } - if (empty_cnt == nmerge) { - return acc.toArray(bases); + for (MROMergeState mergee : toMerge) { + if (!mergee.isMerged()) { + handleMroError(toMerge, mro); + } } - throw mro_error(to_merge, remain); + return mro.toArray(new PyObject[mro.size()]); } + /** + * Must either throw an exception, or bring the merges in <code>toMerge</code> to completion by + * finishing filling in <code>mro</code>. + */ + void handleMroError(MROMergeState[] toMerge, List<PyObject> mro) { + StringBuilder msg = new StringBuilder("Cannot create a consistent method resolution\n" + + "order (MRO) for bases "); + Set<PyObject> set = Generic.set(); + for (MROMergeState mergee : toMerge) { + if(!mergee.isMerged()) { + set.add(mergee.mro[0]); + } + } + boolean first = true; + for (PyObject unmerged : set) { + PyObject name = unmerged.__findattr__("__name__"); + if (first) { + first = false; + } else { + msg.append(", "); + } + msg.append(name == null ? "?" : name.toString() + new PyList(((PyType)unmerged).bases)); + } + throw Py.TypeError(msg.toString()); + } + + /** * Finds the parent of type with an underlying_class or with slots sans a __dict__ * slot. */ @@ -1076,6 +1058,13 @@ dict.__setitem__(pmd.getName(), pmd); } + /** + * Removes the given method from this type's dict or raises a KeyError. + */ + public void removeMethod(PyBuiltinMethod meth) { + dict.__delitem__(meth.info.getName()); + } + protected void checkSetattr() { if (builtin) { throw Py.TypeError(String.format("can't set attributes of built-in/extension type " @@ -1380,4 +1369,64 @@ return pytyp; } } + + /** + * Tracks the status of merging a single base into a subclass' mro in computeMro. + */ + static class MROMergeState { + + /** The mro of the base type we're representing. */ + public PyObject[] mro; + + /** + * The index of the next item to be merged from mro, or mro.length if this base has been + * completely merged. + */ + public int next; + + public boolean isMerged() { + return mro.length == next; + } + + public PyObject getCandidate() { + return mro[next]; + } + + /** + * Marks candidate as merged for this base if it's the next item to be merged. + */ + public void noteMerged(PyObject candidate) { + if (!isMerged() && getCandidate() == candidate) { + next++; + } + } + + /** + * Returns true if candidate is in the items past this state's next item to be merged. + */ + public boolean unmergedContains(PyObject candidate) { + for (int i = next + 1; i < mro.length; i++) { + if (mro[i] == candidate) { + return true; + } + } + return false; + } + + /** + * Removes the given item from this state's mro if it isn't already finished. + */ + public void removeFromUnmerged(PyJavaType winner) { + if (isMerged()) { + return; + } + List<PyObject> newMro = Generic.list(); + for (PyObject mroEntry : mro) { + if (mroEntry != winner) { + newMro.add(mroEntry); + } + } + mro = newMro.toArray(new PyObject[newMro.size()]); + } + } } This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site. |