type.__getattribute__ and object.__getattribute__ are different

Calling object.__getattribute__ on a class will skip the normal
special type attribute-lookup rules (checking base classes, executing
descriptors).

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/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

@@ -3582,6 +3582,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();

test/integration/sqlalchemy_smalltest.py

@@ -126,6 +126,7 @@ 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',
@@ -176,7 +177,6 @@ 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/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
+