Add getattrInternalGeneric to support non-overwridden lookups

This is the analogue of 9f63b62e but for getattr instead of setattr:
previously there was no way to run the default getattr logic, since
getattrInternalGeneral would always check for a custom __getattr__
or __getattribute__.  This meant that object.__getattribute__ was
the same thing as getattr() and that PyObject_GenericGetAttr was the
same as PyObject_GetAttr, which were not correct.

src/asm_writing/assembler.cpp

@@ -737,12 +737,15 @@ void Assembler::cmp(Indirect mem, Immediate imm) {
     emitRex(rex);
     emitByte(0x81);
 
-    assert(-0x80 <= mem.offset && mem.offset < 0x80);
     if (mem.offset == 0) {
         emitModRM(0b00, 7, src_idx);
-    } else {
+    } else if (-0x80 <= mem.offset && mem.offset < 0x80) {
         emitModRM(0b01, 7, src_idx);
         emitByte(mem.offset);
+    } else {
+        assert((-1L << 31) <= mem.offset && mem.offset < (1L << 31) - 1);
+        emitModRM(0b10, 7, src_idx);
+        emitInt(mem.offset, 4);
     }
 
     emitInt(val, 4);

src/capi/typeobject.cpp

@@ -707,7 +707,7 @@ static PyObject* call_attribute(PyObject* self, PyObject* attr, PyObject* name)
             attr = descr;
     }
     try {
-        res = runtimeCall(attr, ArgPassSpec(1, 0, true, true), name, NULL, NULL, NULL, NULL);
+        res = runtimeCall(attr, ArgPassSpec(1, 0, false, false), name, NULL, NULL, NULL, NULL);
     } catch (ExcInfo e) {
         setCAPIException(e);
         Py_XDECREF(descr);
@@ -748,7 +748,7 @@ static PyObject* slot_tp_getattr_hook(PyObject* self, PyObject* name) noexcept {
     }
     if (res == NULL) {
         try {
-            res = runtimeCall(getattr, ArgPassSpec(2, 0, true, true), self, name, NULL, NULL, NULL);
+            res = runtimeCall(getattr, ArgPassSpec(2, 0, false, false), self, name, NULL, NULL, NULL);
         } catch (ExcInfo e) {
             setCAPIException(e);
             return NULL;
@@ -1197,7 +1197,8 @@ static void** slotptr(BoxedClass* type, int offset) noexcept {
     ETSLOT(NAME, as_number.SLOT, FUNCTION, wrap_binaryfunc_r, "x." NAME "(y) <==> " DOC)
 
 static slotdef slotdefs[]
-    = { TPSLOT("__getattr__", tp_getattr, NULL, NULL, ""),
+    = { TPSLOT("__getattribute__", tp_getattr, NULL, NULL, ""),
+        TPSLOT("__getattr__", tp_getattr, NULL, NULL, ""),
         TPSLOT("__setattr__", tp_setattr, NULL, NULL, ""),
         TPSLOT("__delattr__", tp_setattr, NULL, NULL, ""),
 
@@ -1207,6 +1208,8 @@ static slotdef slotdefs[]
                PyWrapperFlag_KEYWORDS),
         TPSLOT("__str__", tp_str, slot_tp_str, wrap_unaryfunc, "x.__str__() <==> str(x)"),
 
+        TPSLOT("__getattribute__", tp_getattro, slot_tp_getattr_hook, wrap_binaryfunc,
+               "x.__getattribute__('name') <==> x.name"),
         TPSLOT("__getattr__", tp_getattro, slot_tp_getattr_hook, NULL, ""),
         TPSLOT("__setattr__", tp_setattro, slot_tp_setattro, wrap_setattr,
                "x.__setattr__('name', value) <==> x.name = value"),
@@ -1509,7 +1512,7 @@ static void add_tp_new_wrapper(BoxedClass* type) noexcept {
                    new BoxedCApiFunction(METH_VARARGS | METH_KEYWORDS, type, "__new__", (PyCFunction)tp_new_wrapper));
 }
 
-static void add_operators(BoxedClass* cls) noexcept {
+void add_operators(BoxedClass* cls) noexcept {
     init_slotdefs();
 
     for (const slotdef& p : slotdefs) {

src/capi/typeobject.h

@@ -22,6 +22,7 @@ namespace pyston {
 // Returns if a slot was updated
 bool update_slot(BoxedClass* self, const std::string& attr) noexcept;
 
+void add_operators(BoxedClass* self) noexcept;
 void fixup_slot_dispatchers(BoxedClass* self) noexcept;
 void commonClassSetup(BoxedClass* cls);
 

src/capi/types.h

@@ -98,6 +98,7 @@ public:
     DEFAULT_CLASS(wrapperdescr_cls);
 
     static Box* __get__(BoxedWrapperDescriptor* self, Box* inst, Box* owner);
+    static Box* __call__(BoxedWrapperDescriptor* descr, PyObject* self, BoxedTuple* args, Box** _args);
 };
 
 class BoxedWrapperObject : public Box {

src/runtime/capi.cpp

@@ -88,6 +88,19 @@ Box* BoxedWrapperDescriptor::__get__(BoxedWrapperDescriptor* self, Box* inst, Bo
     return new BoxedWrapperObject(self, inst);
 }
 
+Box* BoxedWrapperDescriptor::__call__(BoxedWrapperDescriptor* descr, PyObject* self, BoxedTuple* args, Box** _args) {
+    RELEASE_ASSERT(descr->cls == wrapperdescr_cls, "");
+
+    BoxedDict* kw = static_cast<BoxedDict*>(_args[0]);
+
+    if (!isSubclass(self->cls, descr->type))
+        raiseExcHelper(TypeError, "descriptor '' requires a '%s' object but received a '%s'",
+                       getFullNameOfClass(descr->type).c_str(), getFullTypeName(self).c_str());
+
+    auto wrapper = new BoxedWrapperObject(descr, self);
+    return BoxedWrapperObject::__call__(wrapper, args, kw);
+}
+
 extern "C" void _PyErr_BadInternalCall(const char* filename, int lineno) noexcept {
     Py_FatalError("unimplemented");
 }
@@ -264,12 +277,11 @@ extern "C" PyObject* PyObject_GetAttr(PyObject* o, PyObject* attr_name) noexcept
 }
 
 extern "C" PyObject* PyObject_GenericGetAttr(PyObject* o, PyObject* name) noexcept {
-    try {
-        return getattr(o, static_cast<BoxedString*>(name)->s.c_str());
-    } catch (ExcInfo e) {
-        setCAPIException(e);
-        return NULL;
-    }
+    Box* r = getattrInternalGeneric(o, static_cast<BoxedString*>(name)->s.c_str(), 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));
+    return r;
 }
 
 extern "C" PyObject* _PyObject_GenericGetAttrWithDict(PyObject* obj, PyObject* name, PyObject* dict) noexcept {
@@ -1300,10 +1312,10 @@ void setupCAPI() {
     method_cls->giveAttr("__doc__", new (pyston_getset_cls) BoxedGetsetDescriptor(methodGetDoc, NULL, NULL));
     method_cls->freeze();
 
-    wrapperdescr_cls = BoxedHeapClass::create(type_cls, object_cls, NULL, 0, 0, sizeof(BoxedWrapperDescriptor), false,
-                                              "wrapper_descriptor");
     wrapperdescr_cls->giveAttr("__get__",
                                new BoxedFunction(boxRTFunction((void*)BoxedWrapperDescriptor::__get__, UNKNOWN, 3)));
+    wrapperdescr_cls->giveAttr("__call__", new BoxedFunction(boxRTFunction((void*)BoxedWrapperDescriptor::__call__,
+                                                                           UNKNOWN, 2, 0, true, true)));
     wrapperdescr_cls->freeze();
 
     wrapperobject_cls

src/runtime/objmodel.cpp

@@ -1103,10 +1103,39 @@ Box* dataDescriptorInstanceSpecialCases(GetattrRewriteArgs* rewrite_args, const
     return NULL;
 }
 
-inline Box* getclsattr_internal(Box* obj, const std::string& attr, GetattrRewriteArgs* rewrite_args) {
-    return getattrInternalGeneral(obj, attr, rewrite_args,
-                                  /* cls_only */ true,
-                                  /* for_call */ false, NULL, NULL);
+Box* getattrInternalEx(Box* obj, const std::string& attr, GetattrRewriteArgs* rewrite_args, bool cls_only,
+                       bool for_call, Box** bind_obj_out, RewriterVar** r_bind_obj_out) {
+    if (!cls_only) {
+        BoxedClass* cls = obj->cls;
+        if (obj->cls->tp_getattro && obj->cls->tp_getattro != PyObject_GenericGetAttr) {
+            Box* r = obj->cls->tp_getattro(obj, new BoxedString(attr));
+            if (!r)
+                throwCAPIException();
+            return r;
+        }
+
+        if (obj->cls->tp_getattr) {
+            Box* r = obj->cls->tp_getattr(obj, const_cast<char*>(attr.c_str()));
+            if (!r)
+                throwCAPIException();
+            return r;
+        }
+
+        // We could also use the old invalidation-based approach here:
+        if (rewrite_args) {
+            auto r_cls = rewrite_args->obj->getAttr(offsetof(Box, cls));
+            r_cls->addAttrGuard(offsetof(BoxedClass, tp_getattr), (uint64_t)obj->cls->tp_getattr);
+            r_cls->addAttrGuard(offsetof(BoxedClass, tp_getattro), (uint64_t)obj->cls->tp_getattro);
+        }
+    }
+
+    return getattrInternalGeneric(obj, attr, rewrite_args, cls_only, for_call, bind_obj_out, r_bind_obj_out);
+}
+
+inline Box* getclsattrInternal(Box* obj, const std::string& attr, GetattrRewriteArgs* rewrite_args) {
+    return getattrInternalEx(obj, attr, rewrite_args,
+                             /* cls_only */ true,
+                             /* for_call */ false, NULL, NULL);
 }
 
 extern "C" Box* getclsattr(Box* obj, const char* attr) {
@@ -1121,7 +1150,7 @@ extern "C" Box* getclsattr(Box* obj, const char* attr) {
     if (rewriter.get()) {
         //rewriter->trap();
         GetattrRewriteArgs rewrite_args(rewriter.get(), rewriter->getArg(0));
-        gotten = getclsattr_internal(obj, attr, &rewrite_args, NULL);
+        gotten = getclsattrInternal(obj, attr, &rewrite_args, NULL);
 
         if (rewrite_args.out_success && gotten) {
             rewrite_args.out_rtn.move(-1);
@@ -1134,7 +1163,7 @@ extern "C" Box* getclsattr(Box* obj, const char* attr) {
     if (rewriter.get()) {
         // rewriter->trap();
         GetattrRewriteArgs rewrite_args(rewriter.get(), rewriter->getArg(0), rewriter->getReturnDestination());
-        gotten = getclsattr_internal(obj, attr, &rewrite_args);
+        gotten = getclsattrInternal(obj, attr, &rewrite_args);
 
         if (rewrite_args.out_success && gotten) {
             rewriter->commitReturning(rewrite_args.out_rtn);
@@ -1142,7 +1171,7 @@ extern "C" Box* getclsattr(Box* obj, const char* attr) {
 #endif
 }
 else {
-    gotten = getclsattr_internal(obj, attr, NULL);
+    gotten = getclsattrInternal(obj, attr, NULL);
 }
 RELEASE_ASSERT(gotten, "%s:%s", getTypeName(obj), attr);
 
@@ -1173,12 +1202,13 @@ static Box* (*runtimeCall2)(Box*, ArgPassSpec, Box*, Box*) = (Box * (*)(Box*, Ar
 static Box* (*runtimeCall3)(Box*, ArgPassSpec, Box*, Box*, Box*)
     = (Box * (*)(Box*, ArgPassSpec, Box*, Box*, Box*))runtimeCall;
 
-Box* getattrInternalGeneral(Box* obj, const std::string& attr, GetattrRewriteArgs* rewrite_args, bool cls_only,
+Box* getattrInternalGeneric(Box* obj, const std::string& 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;
     }
 
+    // TODO this should be a custom getattr
     if (obj->cls == closure_cls) {
         Box* val = NULL;
         if (rewrite_args) {
@@ -1213,23 +1243,6 @@ Box* getattrInternalGeneral(Box* obj, const std::string& attr, GetattrRewriteArg
         raiseExcHelper(NameError, "free variable '%s' referenced before assignment in enclosing scope", attr.c_str());
     }
 
-    if (!cls_only) {
-        // Don't need to pass icentry args, since we special-case __getattribtue__ and __getattr__ to use
-        // invalidation rather than guards
-        // TODO since you changed this to typeLookup you need to guard
-        Box* getattribute = typeLookup(obj->cls, getattribute_str, NULL);
-        if (getattribute) {
-            // TODO this is a good candidate for interning?
-            Box* boxstr = boxString(attr);
-            Box* rtn = runtimeCall2(getattribute, ArgPassSpec(2), obj, boxstr);
-            return rtn;
-        }
-
-        if (rewrite_args) {
-            rewrite_args->rewriter->addDependenceOn(obj->cls->dependent_icgetattrs);
-        }
-    }
-
     // Handle descriptor logic here.
     // A descriptor is either a data descriptor or a non-data descriptor.
     // data descriptors define both __get__ and __set__. non-data descriptors
@@ -1499,35 +1512,11 @@ Box* getattrInternalGeneral(Box* obj, const std::string& attr, GetattrRewriteArg
     // TODO this shouldn't go here; it should be in instancemethod_cls->tp_getattr[o]
     if (obj->cls == instancemethod_cls) {
         assert(!rewrite_args || !rewrite_args->out_success);
-        return getattrInternalGeneral(static_cast<BoxedInstanceMethod*>(obj)->func, attr, NULL, cls_only, for_call,
-                                      bind_obj_out, NULL);
+        return getattrInternalEx(static_cast<BoxedInstanceMethod*>(obj)->func, attr, NULL, cls_only, for_call,
+                                 bind_obj_out, NULL);
     }
     // Finally, check __getattr__
 
-    if (!cls_only) {
-        // Don't need to pass icentry args, since we special-case __getattribute__ and __getattr__ to use
-        // invalidation rather than guards
-        rewrite_args = NULL;
-        REWRITE_ABORTED("");
-
-        if (obj->cls->tp_getattr) {
-            Box* rtn = obj->cls->tp_getattr(obj, const_cast<char*>(attr.c_str()));
-            if (rtn == NULL)
-                throwCAPIException();
-            return rtn;
-        }
-        Box* getattr = typeLookup(obj->cls, getattr_str, NULL);
-        if (getattr) {
-            Box* boxstr = boxString(attr);
-            Box* rtn = runtimeCall2(getattr, ArgPassSpec(2), obj, boxstr);
-            return rtn;
-        }
-
-        if (rewrite_args) {
-            rewrite_args->rewriter->addDependenceOn(obj->cls->dependent_icgetattrs);
-        }
-    }
-
     if (rewrite_args) {
         rewrite_args->out_success = true;
     }
@@ -1535,9 +1524,9 @@ Box* getattrInternalGeneral(Box* obj, const std::string& attr, GetattrRewriteArg
 }
 
 Box* getattrInternal(Box* obj, const std::string& attr, GetattrRewriteArgs* rewrite_args) {
-    return getattrInternalGeneral(obj, attr, rewrite_args,
-                                  /* cls_only */ false,
-                                  /* for_call */ false, NULL, NULL);
+    return getattrInternalEx(obj, attr, rewrite_args,
+                             /* cls_only */ false,
+                             /* for_call */ false, NULL, NULL);
 }
 
 extern "C" Box* getattr(Box* obj, const char* attr) {
@@ -1878,9 +1867,9 @@ extern "C" bool nonzero(Box* obj) {
     // Stats::log(id);
 
     // go through descriptor logic
-    Box* func = getclsattr_internal(obj, "__nonzero__", NULL);
+    Box* func = getclsattrInternal(obj, "__nonzero__", NULL);
     if (!func)
-        func = getclsattr_internal(obj, "__len__", NULL);
+        func = getclsattrInternal(obj, "__len__", NULL);
 
     if (func == NULL) {
         ASSERT(isUserDefined(obj->cls) || obj->cls == classobj_cls || obj->cls == type_cls
@@ -2001,7 +1990,7 @@ extern "C" BoxedInt* hash(Box* obj) {
     slowpath_hash.log();
 
     // goes through descriptor logic
-    Box* hash = getclsattr_internal(obj, "__hash__", NULL);
+    Box* hash = getclsattrInternal(obj, "__hash__", NULL);
 
     if (hash == NULL) {
         ASSERT(isUserDefined(obj->cls) || obj->cls == function_cls || obj->cls == object_cls || obj->cls == classobj_cls
@@ -2241,20 +2230,20 @@ extern "C" Box* callattrInternal(Box* obj, const std::string* attr, LookupScope
 
     // Look up the argument. Pass in the arguments to getattrInternalGeneral or getclsattr_general
     // that will shortcut functions by not putting them into instancemethods
-    Box* bind_obj;
+    Box* bind_obj = NULL; // Initialize this to NULL to allow getattrInternalEx to ignore it
     RewriterVar* r_bind_obj;
     Box* val;
     RewriterVar* r_val = NULL;
     if (rewrite_args) {
         GetattrRewriteArgs grewrite_args(rewrite_args->rewriter, rewrite_args->obj, Location::any());
-        val = getattrInternalGeneral(obj, *attr, &grewrite_args, scope == CLASS_ONLY, true, &bind_obj, &r_bind_obj);
+        val = getattrInternalEx(obj, *attr, &grewrite_args, scope == CLASS_ONLY, true, &bind_obj, &r_bind_obj);
         if (!grewrite_args.out_success) {
             rewrite_args = NULL;
         } else if (val) {
             r_val = grewrite_args.out_rtn;
         }
     } else {
-        val = getattrInternalGeneral(obj, *attr, NULL, scope == CLASS_ONLY, true, &bind_obj, &r_bind_obj);
+        val = getattrInternalEx(obj, *attr, NULL, scope == CLASS_ONLY, true, &bind_obj, &r_bind_obj);
     }
 
     if (val == NULL) {
@@ -3448,7 +3437,7 @@ extern "C" Box* unaryop(Box* operand, int op_type) {
 
     const std::string& op_name = getOpName(op_type);
 
-    Box* attr_func = getclsattr_internal(operand, op_name, NULL);
+    Box* attr_func = getclsattrInternal(operand, op_name, NULL);
 
     ASSERT(attr_func, "%s.%s", getTypeName(operand), op_name.c_str());
 

src/runtime/objmodel.h

@@ -121,7 +121,7 @@ extern "C" void delattr_internal(Box* obj, const std::string& attr, bool allow_c
 struct CompareRewriteArgs;
 Box* compareInternal(Box* lhs, Box* rhs, int op_type, CompareRewriteArgs* rewrite_args);
 Box* getattrInternal(Box* obj, const std::string& attr, GetattrRewriteArgs* rewrite_args);
-Box* getattrInternalGeneral(Box* obj, const std::string& attr, GetattrRewriteArgs* rewrite_args, bool cls_only,
+Box* getattrInternalGeneric(Box* obj, const std::string& attr, GetattrRewriteArgs* rewrite_args, bool cls_only,
                             bool for_call, Box** bind_obj_out, RewriterVar** r_bind_obj_out);
 
 Box* typeLookup(BoxedClass* cls, const std::string& attr, GetattrRewriteArgs* rewrite_args);

src/runtime/types.cpp

@@ -104,7 +104,7 @@ extern "C" PyObject* PystonType_GenericAlloc(BoxedClass* cls, Py_ssize_t nitems)
     }
 #endif
     if (!cls->tp_mro) {
-        assert(!attrwrapper_cls); // the last class to be set up during bootstrapping
+        assert(!wrapperdescr_cls); // the last class to be set up during bootstrapping
     } else {
         assert(cls->tp_mro && "maybe we should just skip these checks if !mro");
         assert(cls->tp_mro->cls == tuple_cls);
@@ -1735,6 +1735,8 @@ void setupRuntime() {
         = new BoxedHeapClass(object_cls, NULL, 0, 0, sizeof(BoxedGetsetDescriptor), false, boxStrConstant("getset"));
     attrwrapper_cls = new BoxedHeapClass(object_cls, &AttrWrapper::gcHandler, 0, 0, sizeof(AttrWrapper), false,
                                          new BoxedString("attrwrapper"));
+    wrapperdescr_cls = new BoxedHeapClass(object_cls, NULL, 0, 0, sizeof(BoxedWrapperDescriptor), false,
+                                          new BoxedString("wrapper_descriptor"));
 
 
     // Kind of hacky, but it's easier to manually construct the mro for a couple key classes
@@ -1756,6 +1758,10 @@ void setupRuntime() {
     list_cls->finishInitialization();
     pyston_getset_cls->finishInitialization();
     attrwrapper_cls->finishInitialization();
+    wrapperdescr_cls->finishInitialization();
+
+    object_cls->tp_getattro = PyObject_GenericGetAttr;
+    add_operators(object_cls);
 
     str_cls->tp_flags |= Py_TPFLAGS_HAVE_NEWBUFFER;
 

test/tests/object_default_behavior.py

@@ -6,8 +6,15 @@ class C(object):
         else:
             object.__setattr__(self, attr, value)
 
+    def __getattribute__(self, attr):
+        print "getattribute", attr
+        if attr.startswith("c"):
+            return "yum"
+        return object.__getattribute__(self, attr)
+
 c = C()
 c.a = 1
 c.b = 2
 c.c = 3
 print sorted(c.__dict__.items())
+print c.a, c.b, c.c