Merge branch 'getattr'

Things are getting pretty messy, but it's an incremental step towards doing things more
like CPython which should ultimately make things simpler.

The use of the tp_getattro slots (even though they're the CAPI slots) seems to lead to
about a ~7% improvement in interpreter-only speed.

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.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/bool.cpp

@@ -93,12 +93,6 @@ void setupBool() {
     bool_cls->giveAttr("__xor__", new BoxedFunction(boxRTFunction((void*)boolXor, BOXED_BOOL, 2)));
 
     bool_cls->freeze();
-
-    True = new BoxedBool(true);
-    False = new BoxedBool(false);
-
-    gc::registerPermanentRoot(True);
-    gc::registerPermanentRoot(False);
 }
 
 void teardownBool() {

src/runtime/builtin_modules/sys.cpp

@@ -194,6 +194,7 @@ public:
 
     BoxedSysFlags() {
         auto zero = boxInt(0);
+        assert(zero);
         division_warning = zero;
         bytes_warning = zero;
         no_user_site = zero;
@@ -297,8 +298,8 @@ void setupSys() {
 
     sys_module->giveAttr("maxint", boxInt(PYSTON_INT_MAX));
 
-    sys_flags_cls = BoxedHeapClass::create(type_cls, object_cls, BoxedSysFlags::gcHandler, 0, 0, sizeof(BoxedSysFlags),
-                                           false, "flags");
+    sys_flags_cls = new BoxedHeapClass(object_cls, BoxedSysFlags::gcHandler, 0, 0, sizeof(BoxedSysFlags), false,
+                                       new BoxedString("flags"));
     sys_flags_cls->giveAttr("__new__",
                             new BoxedFunction(boxRTFunction((void*)BoxedSysFlags::__new__, UNKNOWN, 1, 0, true, true)));
 #define ADD(name)                                                                                                      \
@@ -309,6 +310,7 @@ void setupSys() {
     ADD(no_user_site);
 #undef ADD
 
+    sys_flags_cls->tp_mro = new BoxedTuple({ sys_flags_cls, object_cls });
     sys_flags_cls->freeze();
 
     sys_module->giveAttr("flags", new BoxedSysFlags());
@@ -324,5 +326,6 @@ void setupSysEnd() {
                                                               PyLt());
 
     sys_module->giveAttr("builtin_module_names", new BoxedTuple(std::move(builtin_module_names)));
+    sys_flags_cls->finishInitialization();
 }
 }

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 {
@@ -1279,9 +1291,6 @@ static Box* methodGetDoc(Box* b, void*) {
 }
 
 void setupCAPI() {
-    capifunc_cls
-        = BoxedHeapClass::create(type_cls, object_cls, NULL, 0, 0, sizeof(BoxedCApiFunction), false, "capifunc");
-
     capifunc_cls->giveAttr("__repr__",
                            new BoxedFunction(boxRTFunction((void*)BoxedCApiFunction::__repr__, UNKNOWN, 1)));
 
@@ -1291,8 +1300,6 @@ void setupCAPI() {
 
     capifunc_cls->freeze();
 
-    method_cls
-        = BoxedHeapClass::create(type_cls, object_cls, NULL, 0, 0, sizeof(BoxedMethodDescriptor), false, "method");
     method_cls->giveAttr("__get__",
                          new BoxedFunction(boxRTFunction((void*)BoxedMethodDescriptor::__get__, UNKNOWN, 3)));
     method_cls->giveAttr("__call__", new BoxedFunction(boxRTFunction((void*)BoxedMethodDescriptor::__call__, UNKNOWN, 2,
@@ -1300,14 +1307,12 @@ 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
-        = BoxedHeapClass::create(type_cls, object_cls, NULL, 0, 0, sizeof(BoxedWrapperObject), false, "method-wrapper");
     wrapperobject_cls->giveAttr(
         "__call__", new BoxedFunction(boxRTFunction((void*)BoxedWrapperObject::__call__, UNKNOWN, 1, 0, true, true)));
     wrapperobject_cls->freeze();

src/runtime/objmodel.cpp

@@ -1103,8 +1103,37 @@ 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,
+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);
 }
@@ -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,
+        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,7 +1524,7 @@ 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,
+    return getattrInternalEx(obj, attr, rewrite_args,
                              /* cls_only */ false,
                              /* for_call */ false, NULL, NULL);
 }
@@ -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) {
@@ -2343,14 +2332,6 @@ extern "C" Box* callattrInternal(Box* obj, const std::string* attr, LookupScope
         }
     } else {
         Box* rtn;
-        // I *think* this check is here to limit the recursion nesting for rewriting, and originates
-        // from a time when we didn't have silent-abort-when-patchpoint-full.
-        if (val->cls != function_cls && val->cls != builtin_function_or_method_cls && val->cls != instancemethod_cls
-            && val->cls != capifunc_cls) {
-            rewrite_args = NULL;
-            REWRITE_ABORTED("");
-        }
-
         if (rewrite_args) {
             CallRewriteArgs srewrite_args(rewrite_args->rewriter, r_val, rewrite_args->destination);
             if (npassed_args >= 1)
@@ -3456,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/stacktrace.cpp

@@ -144,7 +144,11 @@ extern "C" void abort() {
         // that object, _printStackTrace will hang waiting for the first construction
         // to finish.)
         alarm(1);
+        try {
             _printStacktrace();
+        } catch (ExcInfo) {
+            fprintf(stderr, "error printing stack trace during abort()");
+        }
 
         // Cancel the alarm.
         // This is helpful for when running in a debugger, since the debugger will catch the

src/runtime/types.h

@@ -204,6 +204,7 @@ protected:
                bool is_user_defined);
 
     friend void setupRuntime();
+    friend void setupSysEnd();
 };
 
 class BoxedHeapClass : public BoxedClass {
@@ -232,6 +233,7 @@ private:
                    bool is_user_defined, BoxedString* name);
 
     friend void setupRuntime();
+    friend void setupSys();
 
     DEFAULT_CLASS(type_cls);
 };

test/tests/55.py

-# expected: fail
-# - object.__getattribute__ doesn't exist
-
 # attr-getting resolution.
 
 class M(type):

test/tests/descriptors_double.py

@@ -53,8 +53,6 @@ else:
     print 'False'
 
 # __getattr__ and __setattr__
-# Looks like __getattr__ and __setattr__ should *not* be looked up with
-# the descriptor protocol
 class DescriptorGetattr(object):
     def __get__(self, obj, type):
         print 'getattr __get__ called'
@@ -68,8 +66,8 @@ class DescriptorSetattr(object):
         def setattr(attr, val):
             print 'setattr called for attr', attr, val
 class D(object):
-    __getattr__ = DescriptorGetattr
-    __setattr__ = DescriptorSetattr
+    __getattr__ = DescriptorGetattr()
+    __setattr__ = DescriptorSetattr()
 
 d = D()
 try:

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