Merge pull request #943 from kmod/sqlalchemy

type.__getattribute__ and object.__getattribute__ are different

from_cpython/Lib/weakref.py

@@ -468,7 +468,7 @@ class WeakKeyDictionary(UserDict.UserDict):
 
     def pop(self, key, *args):
         r = ref(key)
-        self.keys.pop(r, None)
+        self.refs.pop(r, None)
         return self.data.pop(r, *args)
 
     def setdefault(self, key, default=None):

src/capi/typeobject.cpp

@@ -962,7 +962,8 @@ Box* slotTpGetattrHookInternal(Box* self, BoxedString* name, GetattrRewriteArgs*
 
             GetattrRewriteArgs grewrite_args(rewrite_args->rewriter, rewrite_args->obj, rewrite_args->destination);
             try {
-                res = getattrInternalGeneric(self, name, &grewrite_args, false, false, NULL, NULL);
+                assert(!PyType_Check(self)); // There would be a getattribute
+                res = getattrInternalGeneric<false>(self, name, &grewrite_args, false, false, NULL, NULL);
             } catch (ExcInfo e) {
                 if (!e.matches(AttributeError)) {
                     if (S == CAPI) {
@@ -1005,7 +1006,8 @@ Box* slotTpGetattrHookInternal(Box* self, BoxedString* name, GetattrRewriteArgs*
             }
         } else {
             try {
-                res = getattrInternalGeneric(self, name, NULL, false, false, NULL, NULL);
+                assert(!PyType_Check(self)); // There would be a getattribute
+                res = getattrInternalGeneric<false>(self, name, NULL, false, false, NULL, NULL);
             } catch (ExcInfo e) {
                 if (!e.matches(AttributeError)) {
                     if (S == CAPI) {

src/runtime/capi.cpp

@@ -234,7 +234,7 @@ extern "C" PyObject* PyObject_GenericGetAttr(PyObject* o, PyObject* name) noexce
     try {
         BoxedString* s = static_cast<BoxedString*>(name);
         internStringMortalInplace(s);
-        Box* r = getattrInternalGeneric(o, s, NULL, false, false, NULL, NULL);
+        Box* r = getattrInternalGeneric<false>(o, s, NULL, false, false, NULL, NULL);
         if (!r)
             PyErr_Format(PyExc_AttributeError, "'%.50s' object has no attribute '%.400s'", o->cls->tp_name,
                          PyString_AS_STRING(name));

src/runtime/descr.cpp

@@ -697,6 +697,8 @@ void setupDescr() {
     member_descriptor_cls->giveAttr("__get__", new BoxedFunction(boxRTFunction((void*)memberGet, UNKNOWN, 3)));
     member_descriptor_cls->freeze();
 
+    property_cls->instances_are_nonzero = true;
+
     property_cls->giveAttr("__init__",
                            new BoxedFunction(boxRTFunction((void*)propertyInit, UNKNOWN, 5, false, false,
                                                            ParamNames({ "", "fget", "fset", "fdel", "doc" }, "", "")),

src/runtime/dict.cpp

@@ -571,6 +571,9 @@ Box* dictEq(BoxedDict* self, Box* _rhs) {
         raiseExcHelper(TypeError, "descriptor '__eq__' requires a 'dict' object but received a '%s'",
                        getTypeName(self));
 
+    if (_rhs->cls == attrwrapper_cls)
+        _rhs = attrwrapperToDict(_rhs);
+
     if (!PyDict_Check(_rhs))
         return NotImplemented;
 

src/runtime/list.cpp

@@ -1078,12 +1078,6 @@ Box* listRemove(BoxedList* self, Box* elt) {
 BoxedClass* list_iterator_cls = NULL;
 BoxedClass* list_reverse_iterator_cls = NULL;
 
-Box* listNew(BoxedClass* cls, Box* container) {
-    assert(PyType_Check(cls));
-    assert(isSubclass(cls, list_cls));
-    return new (cls) BoxedList();
-}
-
 Box* listInit(BoxedList* self, Box* container) {
     assert(PyList_Check(self));
 
@@ -1337,9 +1331,9 @@ void setupList() {
     list_cls->giveAttr("extend", new BoxedFunction(boxRTFunction((void*)listIAdd, UNKNOWN, 2)));
 
     list_cls->giveAttr("insert", new BoxedFunction(boxRTFunction((void*)listInsert, NONE, 3)));
-    list_cls->giveAttr("__mul__", new BoxedFunction(boxRTFunction((void*)listMul, LIST, 2)));
-    list_cls->giveAttr("__rmul__", new BoxedFunction(boxRTFunction((void*)listMul, LIST, 2)));
-    list_cls->giveAttr("__imul__", new BoxedFunction(boxRTFunction((void*)listImul, LIST, 2)));
+    list_cls->giveAttr("__mul__", new BoxedFunction(boxRTFunction((void*)listMul, UNKNOWN, 2)));
+    list_cls->giveAttr("__rmul__", new BoxedFunction(boxRTFunction((void*)listMul, UNKNOWN, 2)));
+    list_cls->giveAttr("__imul__", new BoxedFunction(boxRTFunction((void*)listImul, UNKNOWN, 2)));
 
     list_cls->giveAttr("__iadd__", new BoxedFunction(boxRTFunction((void*)listIAdd, UNKNOWN, 2)));
     list_cls->giveAttr("__add__", new BoxedFunction(boxRTFunction((void*)listAdd, UNKNOWN, 2)));
@@ -1349,7 +1343,6 @@ void setupList() {
                                                  { None, None, False }));
     list_cls->giveAttr("__contains__", new BoxedFunction(boxRTFunction((void*)listContains, BOXED_BOOL, 2)));
 
-    list_cls->giveAttr("__new__", new BoxedFunction(boxRTFunction((void*)listNew, UNKNOWN, 2, false, false), { None }));
     list_cls->giveAttr("__init__",
                        new BoxedFunction(boxRTFunction((void*)listInit, UNKNOWN, 2, false, false), { NULL }));
 

src/runtime/objmodel.cpp

@@ -307,6 +307,23 @@ extern "C" void raiseAttributeErrorCapi(Box* obj, llvm::StringRef attr) noexcept
     }
 }
 
+extern "C" PyObject* type_getattro(PyObject* o, PyObject* name) noexcept {
+    assert(PyString_Check(name));
+    BoxedString* s = static_cast<BoxedString*>(name);
+    assert(PyString_CHECK_INTERNED(name));
+
+    try {
+        Box* r = getattrInternalGeneric<true>(o, s, NULL, false, false, NULL, NULL);
+        if (!r && !PyErr_Occurred())
+            PyErr_Format(PyExc_AttributeError, "type object '%.50s' has no attribute '%.400s'",
+                         static_cast<BoxedClass*>(o)->tp_name, PyString_AS_STRING(name));
+        return r;
+    } catch (ExcInfo e) {
+        setCAPIException(e);
+        return NULL;
+    }
+}
+
 extern "C" void raiseIndexErrorStr(const char* typeName) {
     raiseExcHelper(IndexError, "%s index out of range", typeName);
 }
@@ -1461,6 +1478,19 @@ Box* getattrInternalEx(Box* obj, BoxedString* attr, GetattrRewriteArgs* rewrite_
                 return slotTpGetattrHookInternal<S>(obj, attr, rewrite_args);
             } else if (obj->cls->tp_getattro == instance_getattro) {
                 return instanceGetattroInternal<S>(obj, attr, rewrite_args);
+            } else if (obj->cls->tp_getattro == type_getattro) {
+                try {
+                    Box* r = getattrInternalGeneric<true>(obj, attr, rewrite_args, cls_only, for_call, bind_obj_out,
+                                                          r_bind_obj_out);
+                    return r;
+                } catch (ExcInfo e) {
+                    if (S == CAPI) {
+                        setCAPIException(e);
+                        return NULL;
+                    } else {
+                        throw e;
+                    }
+                }
             }
 
             Box* r = obj->cls->tp_getattro(obj, attr);
@@ -1522,7 +1552,9 @@ Box* getattrInternalEx(Box* obj, BoxedString* attr, GetattrRewriteArgs* rewrite_
 
     if (S == CAPI) {
         try {
-            return getattrInternalGeneric(obj, attr, rewrite_args, cls_only, for_call, bind_obj_out, r_bind_obj_out);
+            assert(!PyType_Check(obj)); // There would be a tp_getattro
+            return getattrInternalGeneric<false>(obj, attr, rewrite_args, cls_only, for_call, bind_obj_out,
+                                                 r_bind_obj_out);
         } catch (ExcInfo e) {
             setCAPIException(e);
             return NULL;
@@ -1532,7 +1564,8 @@ Box* getattrInternalEx(Box* obj, BoxedString* attr, GetattrRewriteArgs* rewrite_
             class Helper {
             public:
                 static Box* call(Box* obj, BoxedString* attr, bool cls_only) {
-                    return getattrInternalGeneric(obj, attr, NULL, cls_only, false, NULL, NULL);
+                    assert(!PyType_Check(obj) || cls_only); // There would be a tp_getattro
+                    return getattrInternalGeneric<false>(obj, attr, NULL, cls_only, false, NULL, NULL);
                 }
             };
 
@@ -1545,7 +1578,8 @@ Box* getattrInternalEx(Box* obj, BoxedString* attr, GetattrRewriteArgs* rewrite_
             return Helper::call(obj, attr, cls_only);
         }
 
-        return getattrInternalGeneric(obj, attr, rewrite_args, cls_only, for_call, bind_obj_out, r_bind_obj_out);
+        assert(!PyType_Check(obj) || cls_only); // There would be a tp_getattro
+        return getattrInternalGeneric<false>(obj, attr, rewrite_args, cls_only, for_call, bind_obj_out, r_bind_obj_out);
     }
 }
 
@@ -1631,12 +1665,19 @@ Box* processDescriptor(Box* obj, Box* inst, Box* owner) {
 }
 
 
+template <bool IsType>
 Box* getattrInternalGeneric(Box* obj, BoxedString* attr, GetattrRewriteArgs* rewrite_args, bool cls_only, bool for_call,
                             Box** bind_obj_out, RewriterVar** r_bind_obj_out) {
     if (for_call) {
         *bind_obj_out = NULL;
     }
 
+    if (IsType) {
+        if (!PyType_Check(obj))
+            raiseExcHelper(TypeError, "descriptor '__getattribute__' requires a 'type' object but received a '%s'",
+                           obj->cls->tp_name);
+    }
+
     assert(obj->cls != closure_cls);
 
     static BoxedString* get_str = internStringImmortal("__get__");
@@ -1791,7 +1832,7 @@ Box* getattrInternalGeneric(Box* obj, BoxedString* attr, GetattrRewriteArgs* rew
     }
 
     if (!cls_only) {
-        if (!PyType_Check(obj)) {
+        if (!IsType) {
             // Look up the val in the object's dictionary and if you find it, return it.
 
             if (unlikely(rewrite_args && !descr && obj->cls != instancemethod_cls

src/runtime/objmodel.h

@@ -157,9 +157,12 @@ Box* getattrInternal(Box* obj, BoxedString* attr, GetattrRewriteArgs* rewrite_ar
 // This is the equivalent of PyObject_GenericGetAttr, which performs the default lookup rules for getattr() (check for
 // data descriptor, check for instance attribute, check for non-data descriptor). It does not check for __getattr__ or
 // __getattribute__.
+template <bool IsType>
 Box* getattrInternalGeneric(Box* obj, BoxedString* attr, GetattrRewriteArgs* rewrite_args, bool cls_only, bool for_call,
                             Box** bind_obj_out, RewriterVar** r_bind_obj_out);
 
+extern "C" PyObject* type_getattro(PyObject* o, PyObject* name) noexcept;
+
 // This is the equivalent of _PyType_Lookup(), which calls Box::getattr() on each item in the object's MRO in the
 // appropriate order. It does not do any descriptor logic.
 Box* typeLookup(BoxedClass* cls, BoxedString* attr, GetattrRewriteArgs* rewrite_args);

src/runtime/types.cpp

@@ -2731,6 +2731,13 @@ void attrwrapperDel(Box* b, llvm::StringRef attr) {
     AttrWrapper::delitem(b, boxString(attr));
 }
 
+BoxedDict* attrwrapperToDict(Box* b) {
+    assert(b->cls == attrwrapper_cls);
+    Box* d = AttrWrapper::copy(static_cast<AttrWrapper*>(b));
+    assert(d->cls == dict_cls);
+    return static_cast<BoxedDict*>(d);
+}
+
 static int excess_args(PyObject* args, PyObject* kwds) noexcept {
     return PyTuple_GET_SIZE(args) || (kwds && PyDict_Check(kwds) && PyDict_Size(kwds));
 }
@@ -3582,6 +3589,7 @@ void setupRuntime() {
     object_cls->tp_setattro = PyObject_GenericSetAttr;
     object_cls->tp_init = object_init;
     object_cls->tp_new = object_new;
+    type_cls->tp_getattro = type_getattro;
 
     none_cls = new (0) BoxedClass(object_cls, NULL, 0, 0, sizeof(Box), false, "NoneType");
     None = new (none_cls) Box();

src/runtime/types.h

@@ -1069,6 +1069,7 @@ Box* objectSetattr(Box* obj, Box* attr, Box* value);
 Box* unwrapAttrWrapper(Box* b);
 Box* attrwrapperKeys(Box* b);
 void attrwrapperDel(Box* b, llvm::StringRef attr);
+BoxedDict* attrwrapperToDict(Box* b);
 
 Box* boxAst(AST* ast);
 AST* unboxAst(Box* b);

test/integration/sqlalchemy_smalltest.py

@@ -126,17 +126,23 @@ MODULES_TO_TEST = [
     'test.dialect.test_sybase',
     'test.engine.test_bind',
     'test.engine.test_ddlevents',
+    'test.engine.test_logging',
     'test.engine.test_parseconnect',
     'test.engine.test_pool',
     'test.engine.test_reconnect',
     'test.ext.test_compiler',
+    'test.ext.test_hybrid',
+    'test.ext.test_orderinglist',
     'test.orm.test_association',
     'test.orm.test_assorted_eager',
+    'test.orm.test_attributes',
     'test.orm.test_backref_mutations',
     'test.orm.test_bind',
     'test.orm.test_bulk',
     'test.orm.test_bundle',
+    'test.orm.test_collection',
     'test.orm.test_compile',
+    'test.orm.test_composites',
     'test.orm.test_cycles',
     'test.orm.test_defaults',
     'test.orm.test_default_strategies',
@@ -145,10 +151,18 @@ MODULES_TO_TEST = [
     'test.orm.test_descriptor',
     'test.orm.test_eager_relations',
     'test.orm.test_evaluator',
+    'test.orm.test_events',
     'test.orm.test_expire',
+    'test.orm.test_hasparent',
+    'test.orm.test_immediate_load',
     'test.orm.test_inspect',
+    'test.orm.test_joins',
+    'test.orm.test_lazy_relations',
     'test.orm.test_load_on_fks',
     'test.orm.test_lockmode',
+    'test.orm.test_manytomany',
+    'test.orm.test_naturalpks',
+    'test.orm.test_of_type',
     'test.orm.test_onetoone',
     'test.orm.test_options',
     'test.orm.test_query',
@@ -156,6 +170,10 @@ MODULES_TO_TEST = [
     'test.orm.test_scoping',
     'test.orm.test_selectable',
     'test.orm.test_sync',
+    'test.orm.test_transaction',
+    'test.orm.test_unitofworkv2',
+    'test.orm.test_update_delete',
+    'test.orm.test_utils',
     'test.orm.test_validators',
     'test.sql.test_case_statement',
     'test.sql.test_constraints',
@@ -163,6 +181,7 @@ MODULES_TO_TEST = [
     'test.sql.test_ddlemit',
     'test.sql.test_delete',
     'test.sql.test_functions',
+    'test.sql.test_generative',
     'test.sql.test_insert',
     'test.sql.test_inspect',
     'test.sql.test_join_rewriting',
@@ -170,42 +189,23 @@ MODULES_TO_TEST = [
     'test.sql.test_operators',
     'test.sql.test_query',
     'test.sql.test_rowcount',
+    'test.sql.test_selectable',
+    'test.sql.test_text',
 ]
 
 FAILING = [
     # 'test.aaa_profiling.test_memusage',   # Wants gc.get_objects
     # 'test.aaa_profiling.test_resultset',  # Wants sys.getrefcount
     # 'test.dialect.test_sqlite',           # ascii codec can't encode
-    # 'test.engine.test_logging',           # Unclear
     # 'test.ext.test_extendedattr',         # does `locals()[42] = 99` in a classdef to prove it can.  maybe we could say is_pypy to avoid it.
-    'test.ext.test_hybrid',
-    'test.ext.test_orderinglist',
-    'test.orm.test_attributes',
-    'test.orm.test_collection',
-    'test.orm.test_composites',
-    'test.orm.test_dynamic',
-    'test.orm.test_events',
-    'test.orm.test_hasparent',
-    'test.orm.test_immediate_load',
-    'test.orm.test_joins',
-    'test.orm.test_lazy_relations',
-    'test.orm.test_manytomany',
-    'test.orm.test_merge',
-    'test.orm.test_naturalpks',
-    'test.orm.test_of_type',
-    'test.orm.test_relationships',
-    'test.orm.test_session',
-    'test.orm.test_transaction',
-    'test.orm.test_unitofworkv2',
-    'test.orm.test_update_delete',
-    'test.orm.test_utils',
-    'test.orm.test_versioning',
-    'test.sql.test_compiler',
-    'test.sql.test_generative',
-    'test.sql.test_quote',
-    'test.sql.test_selectable',
-    'test.sql.test_text',
-    'test.sql.test_unicode'
+    # 'test.orm.test_dynamic',              # not sure; things end up being put in tuples
+    # 'test.orm.test_merge',                # needs PyObject_AsWriteBuffer
+    # 'test.orm.test_relationships',        # not sure; things end up being put in tuples
+    # 'test.orm.test_session',              # unclear
+    # 'test.orm.test_versioning',           # crashes in the uuid module with an AttributeError from ctypes
+    # 'test.sql.test_compiler',             # unclear
+    # 'test.sql.test_quote',                # unclear
+    # 'test.sql.test_unicode',              # "ascii codec can't encod character"
 ]
 
 # MODULES_TO_TEST = ['test.orm.test_bulk']

test/tests/class_getattr.py

+# doing object.__getattribute__(cls, "foo") should *not* do the normal
+# special rules for looking things up on types: it should not check base
+# classes, and it should not run descriptors.
+
+class Descr(object):
+    def __get__(*args):
+        print "Descr.__get__"
+        return 1
+
+class C(object):
+    d = Descr()
+
+class E(C):
+    pass
+
+# Test that it doesn't execute descriptors:
+print type(object.__getattribute__(C, 'd')) # Descr
+print type(type.__getattribute__(C, 'd')) # int
+
+# Test that it doesn't look at base classes:
+print type(E.d) # Descr
+try:
+    print type(object.__getattribute__(E, 'd'))
+    assert 0
+except AttributeError as e:
+    print e
+
+
+try:
+    print type.__getattribute__(1, 'd')
+    assert 0
+except TypeError as e:
+    print e
+
+
+# The exception messages are slightly different:
+try:
+    type.__getattribute__(C, 'x')
+    assert 0
+except AttributeError as e:
+    print e
+try:
+    object.__getattribute__(C, 'x')
+    assert 0
+except AttributeError as e:
+    print e

test/tests/class_setattr.py

+# expected: fail
+
+class C(object):
+    pass
+
+# Make sure we can't skirt the tp_slot-updating logic in type.__setattr__
+# by trying to use object.__setattr__ which wouldn't do the internal bookkeeping:
+
+def badrepr():
+    raise Exception()
+
+c = C()
+c.a = 1
+try:
+    object.__setattr__(C, '__repr__', badrepr)
+    assert 0
+except TypeError as e:
+    print e
+c.b = 2
+

test/tests/list_subclassing.py

@@ -36,3 +36,24 @@ print MyList((1,2,3)) < MyList((1,2,3,4))
 
 print MyList((1,2,3)) >= MyList((1,2,3))
 print MyList((1,2,3)) <= MyList((1,2,3))
+
+print type(MyList((1, 2, 3)) * 1)
+
+class ListWithInit(list):
+    def __init__(self, *args, **kwargs):
+        print "ListWithInit.__init__", args, kwargs
+
+l = ListWithInit(1, 2, 3, a=5)
+l.a = 1
+l.b = 2
+# Adapted from the sqlalchemy test:
+import pickle
+l2 = pickle.loads(pickle.dumps(l))
+print l == l2
+assert l.__dict__ == l2.__dict__, (l.__dict__, l2.__dict__)
+
+# Regression test:
+def f(l):
+    l *= 1
+for i in xrange(3000):
+    f(l)