Make use of the tp_descr_get slot

from_cpython/Include/object.h

@@ -460,6 +460,7 @@ struct _typeobject {
     void* _dtor;
     int _attrs_offset;
     bool _flags[3];
+    void* _tpp_descr_get;
     void* _tpp_hasnext;
 };
 

src/capi/typeobject.cpp

@@ -733,6 +733,16 @@ static PyObject* slot_tp_descr_get(PyObject* self, PyObject* obj, PyObject* type
     return PyObject_CallFunctionObjArgs(get, self, obj, type, NULL);
 }
 
+static PyObject* slot_tp_tpp_descr_get(PyObject* self, PyObject* obj, PyObject* type) noexcept {
+    assert(self->cls->tpp_descr_get);
+    try {
+        return self->cls->tpp_descr_get(self, obj, type);
+    } catch (ExcInfo e) {
+        setCAPIException(e);
+        return NULL;
+    }
+}
+
 static PyObject* slot_tp_getattro(PyObject* self, PyObject* name) noexcept {
     static PyObject* getattribute_str = NULL;
     return call_method(self, "__getattribute__", &getattribute_str, "(O)", name);
@@ -1502,6 +1512,10 @@ static const slotdef* update_one_slot(BoxedClass* type, const slotdef* p) noexce
                sanity checks.  I'll buy the first person to
                point out a bug in this reasoning a beer. */
 #endif
+        } else if (offset == offsetof(BoxedClass, tp_descr_get) && descr->cls == function_cls
+                   && static_cast<BoxedFunction*>(descr)->f->always_use_version) {
+            type->tpp_descr_get = (descrgetfunc) static_cast<BoxedFunction*>(descr)->f->always_use_version->code;
+            specific = (void*)slot_tp_tpp_descr_get;
         } else if (descr == Py_None && ptr == (void**)&type->tp_hash) {
             /* We specifically allow __hash__ to be set to None
                to prevent inheritance of the default

src/core/types.h

@@ -308,7 +308,12 @@ public:
         assert(compiled->is_interpreted == (compiled->code == NULL));
         assert(compiled->is_interpreted == (compiled->llvm_code == NULL));
         compiled->clfunc = this;
+
         if (compiled->entry_descriptor == NULL) {
+            if (versions.size() == 0 && compiled->effort == EffortLevel::MAXIMAL && compiled->spec->accepts_all_inputs
+                && compiled->spec->boxed_return_value)
+                always_use_version = compiled;
+
             assert(compiled->spec->arg_types.size() == num_args + (takes_varargs ? 1 : 0) + (takes_kwargs ? 1 : 0));
             versions.push_back(compiled);
         } else {

src/runtime/objmodel.cpp

@@ -62,6 +62,12 @@
 #define BOOL_B_OFFSET ((char*)&(((BoxedBool*)0x01)->n) - (char*)0x1)
 #define INT_N_OFFSET ((char*)&(((BoxedInt*)0x01)->n) - (char*)0x1)
 
+#ifndef NDEBUG
+#define DEBUG 1
+#else
+#define DEBUG 0
+#endif
+
 namespace pyston {
 
 static const std::string all_str("__all__");
@@ -1193,9 +1199,15 @@ return gotten;
 // this function is useful for custom getattribute implementations that already know whether the descriptor
 // came from the class or not.
 Box* processDescriptorOrNull(Box* obj, Box* inst, Box* owner) {
-    Box* descr_r
-        = callattrInternal(obj, &get_str, LookupScope::CLASS_ONLY, NULL, ArgPassSpec(2), inst, owner, NULL, NULL, NULL);
-    return descr_r;
+    if (DEBUG >= 2)
+        assert((obj->cls->tp_descr_get == NULL) == (typeLookup(obj->cls, get_str, NULL) == NULL));
+    if (obj->cls->tp_descr_get) {
+        Box* r = obj->cls->tp_descr_get(obj, inst, owner);
+        if (!r)
+            throwCAPIException();
+        return r;
+    }
+    return NULL;
 }
 
 Box* processDescriptor(Box* obj, Box* inst, Box* owner) {
@@ -1286,9 +1298,14 @@ Box* getattrInternalGeneric(Box* obj, const std::string& attr, GetattrRewriteArg
     }
 
     // Check if it's a data descriptor
+    descrgetfunc descr_get = NULL;
+    // Note: _get_ will only be retrieved if we think it will be profitable to try calling that as opposed to
+    // the descr_get function pointer.
     Box* _get_ = NULL;
     RewriterVar* r_get = NULL;
     if (descr) {
+        descr_get = descr->cls->tp_descr_get;
+
         if (rewrite_args)
             r_descr->addAttrGuard(BOX_CLS_OFFSET, (uint64_t)descr->cls);
 
@@ -1304,22 +1321,35 @@ Box* getattrInternalGeneric(Box* obj, const std::string& attr, GetattrRewriteArg
         // we can immediately know to skip this part if it's one of the
         // special case nondata descriptors.
         if (!isNondataDescriptorInstanceSpecialCase(descr)) {
-            // Check if __get__ exists
             if (rewrite_args) {
                 RewriterVar* r_descr_cls = r_descr->getAttr(BOX_CLS_OFFSET, Location::any());
-                GetattrRewriteArgs grewrite_args(rewrite_args->rewriter, r_descr_cls, Location::any());
-                _get_ = typeLookup(descr->cls, get_str, &grewrite_args);
-                if (!grewrite_args.out_success) {
-                    rewrite_args = NULL;
-                } else if (_get_) {
-                    r_get = grewrite_args.out_rtn;
+                r_descr_cls->addAttrGuard(offsetof(BoxedClass, tp_descr_get), (intptr_t)descr_get);
+            }
+
+            // Check if __get__ exists
+            if (descr_get) {
+                if (rewrite_args) {
+                    RewriterVar* r_descr_cls = r_descr->getAttr(BOX_CLS_OFFSET, Location::any());
+                    GetattrRewriteArgs grewrite_args(rewrite_args->rewriter, r_descr_cls, Location::any());
+                    _get_ = typeLookup(descr->cls, get_str, &grewrite_args);
+                    assert(_get_);
+                    if (!grewrite_args.out_success) {
+                        rewrite_args = NULL;
+                    } else if (_get_) {
+                        r_get = grewrite_args.out_rtn;
+                    }
+                } else {
+                    // Don't look up __get__ if we can't rewrite under the assumption that it will
+                    // usually be faster to just call tp_descr_get:
+                    //_get_ = typeLookup(descr->cls, get_str, NULL);
                 }
             } else {
-                _get_ = typeLookup(descr->cls, get_str, NULL);
+                if (DEBUG >= 2)
+                    assert(typeLookup(descr->cls, get_str, NULL) == NULL);
             }
 
             // As an optimization, don't check for __set__ if we're in cls_only mode, since it won't matter.
-            if (_get_ && !cls_only) {
+            if (descr_get && !cls_only) {
                 // Check if __set__ exists
                 Box* _set_ = NULL;
                 if (rewrite_args) {
@@ -1357,7 +1387,9 @@ Box* getattrInternalGeneric(Box* obj, const std::string& attr, GetattrRewriteArg
                             rewrite_args->out_rtn = crewrite_args.out_rtn;
                         }
                     } else {
-                        res = runtimeCallInternal(_get_, NULL, ArgPassSpec(3), descr, obj, obj->cls, NULL, NULL);
+                        res = descr_get(descr, obj, obj->cls);
+                        if (!res)
+                            throwCAPIException();
                     }
                     return res;
                 }
@@ -1485,7 +1517,7 @@ Box* getattrInternalGeneric(Box* obj, const std::string& attr, GetattrRewriteArg
 
         // We looked up __get__ above. If we found it, call it and return
         // the result.
-        if (_get_) {
+        if (descr_get) {
             // this could happen for the callattr path...
             if (for_call) {
                 rewrite_args = NULL;
@@ -1494,6 +1526,7 @@ Box* getattrInternalGeneric(Box* obj, const std::string& attr, GetattrRewriteArg
 
             Box* res;
             if (rewrite_args) {
+                assert(_get_);
                 CallRewriteArgs crewrite_args(rewrite_args->rewriter, r_get, rewrite_args->destination);
                 crewrite_args.arg1 = r_descr;
                 crewrite_args.arg2 = rewrite_args->obj;
@@ -1506,7 +1539,9 @@ Box* getattrInternalGeneric(Box* obj, const std::string& attr, GetattrRewriteArg
                     rewrite_args->out_rtn = crewrite_args.out_rtn;
                 }
             } else {
-                res = runtimeCallInternal(_get_, NULL, ArgPassSpec(3), descr, obj, obj->cls, NULL, NULL);
+                res = descr_get(descr, obj, obj->cls);
+                if (!res)
+                    throwCAPIException();
             }
             return res;
         }
@@ -2456,11 +2491,8 @@ static CompiledFunction* pickVersion(CLFunction* f, int num_output_args, Box* oa
         if (!cf->spec->boxed_return_value)
             continue;
 
-        if (cf->spec->accepts_all_inputs) {
-            if (cf == f->versions[0] && cf->effort == EffortLevel::MAXIMAL)
-                f->always_use_version = cf;
+        if (cf->spec->accepts_all_inputs)
             return cf;
-        }
 
         assert(cf->spec->rtn_type->llvmType() == UNKNOWN->llvmType());
 
@@ -3592,8 +3624,8 @@ void Box::delattr(const std::string& attr, DelattrRewriteArgs* rewrite_args) {
     abort();
 }
 
-extern "C" void delattr_internal(Box* obj, const std::string& attr, bool allow_custom,
-                                 DelattrRewriteArgs* rewrite_args) {
+extern "C" void delattrInternal(Box* obj, const std::string& attr, bool allow_custom,
+                                DelattrRewriteArgs* rewrite_args) {
     // custom __delattr__
     if (allow_custom) {
         Box* delAttr = typeLookup(obj->cls, delattr_str, NULL);
@@ -3628,6 +3660,31 @@ extern "C" void delattr_internal(Box* obj, const std::string& attr, bool allow_c
             raiseAttributeError(obj, attr.c_str());
         }
     }
+
+    // TODO this should be in type_setattro
+    if (isSubclass(obj->cls, type_cls)) {
+        BoxedClass* self = static_cast<BoxedClass*>(obj);
+
+        if (attr == getattr_str || attr == getattribute_str) {
+            if (rewrite_args)
+                REWRITE_ABORTED("");
+            // Will have to embed the clear in the IC, so just disable the patching for now:
+            rewrite_args = NULL;
+
+            // TODO should put this clearing behavior somewhere else, since there are probably more
+            // cases in which we want to do it.
+            self->dependent_icgetattrs.invalidateAll();
+        }
+
+        if (attr == "__base__" && self->getattr("__base__"))
+            raiseExcHelper(TypeError, "readonly attribute");
+
+        bool touched_slot = update_slot(self, attr);
+        if (touched_slot) {
+            rewrite_args = NULL;
+            REWRITE_ABORTED("");
+        }
+    }
 }
 
 // del target.attr
@@ -3643,7 +3700,7 @@ extern "C" void delattr(Box* obj, const char* attr) {
     }
 
 
-    delattr_internal(obj, attr, true, NULL);
+    delattrInternal(obj, attr, true, NULL);
 }
 
 extern "C" Box* createBoxedIterWrapper(Box* o) {

src/runtime/types.h

@@ -191,6 +191,11 @@ public:
     bool is_pyston_class;
 
     typedef bool (*pyston_inquiry)(Box*);
+
+    // tpp_descr_get is currently just a cache only for the use of tp_descr_get, and shouldn't
+    // be called or examined by clients:
+    descrgetfunc tpp_descr_get;
+
     pyston_inquiry tpp_hasnext;
 
     bool hasGenericGetattr() { return tp_getattr == NULL; }

test/test_extension/slots_test.c

@@ -636,6 +636,12 @@ call_funcs(PyObject* _module, PyObject* args) {
 
     // we aren't checking for tp_setattro.  it's set in cpython and not in pyston
 
+    if (cls->tp_descr_get) {
+        printf("tp_descr_get exists\n");
+    } else {
+        printf("tp_descr_get doesnt exist\n");
+    }
+
     if (cls->tp_as_mapping) {
         printf("tp_as_mapping exists\n");
         PyMappingMethods* map = cls->tp_as_mapping;

test/tests/capi_slots.py

@@ -211,3 +211,9 @@ try:
     del c.bar
 except Exception as e:
     pass
+
+slots_test.call_funcs(C())
+C.__get__ = lambda *args: None
+slots_test.call_funcs(C())
+del C.__get__
+slots_test.call_funcs(C())

test/tests/descriptors_guards.py

@@ -11,7 +11,7 @@ def get(self, obj, typ):
     return self.elem
 
 def set(self, obj, typ):
-    print '__get__ called'
+    print '__set__ called'
     print type(self)
     print type(obj)
     print typ
@@ -29,17 +29,17 @@ class C(object):
 c = C()
 
 def f():
-    print c.a
-    print C.a
+    print "c.a:", c.a
+    print "C.a:", C.a
 
 def g():
     try:
-        print c.b()
+        print "c.b():", c.b()
     except TypeError:
         print 'got TypeError'
 
     try:
-        print C.b()
+        print "C.b():", C.b()
     except TypeError:
         print 'got TypeError'
 
@@ -47,6 +47,8 @@ def h():
     c.c = 10
 
 for i in xrange(2000):
+    print i
+
     f()
     g()
     h()