Merge pull request #1182 from kmod/check_refcounting

Fix remaining "check refcounting" assertions

src/capi/typeobject.cpp

@@ -3456,6 +3456,7 @@ extern "C" void PyType_RequestHcAttrs(PyTypeObject* cls, int offset) noexcept {
 extern "C" void PyType_GiveHcAttrsDictDescr(PyTypeObject* cls) noexcept {
     static BoxedString* dict_str = getStaticString("__dict__");
     assert(!cls->hasattr(dict_str));
+    assert(cls->instancesHaveHCAttrs() && !cls->instancesHaveDictAttrs());
     cls->giveAttr(incref(dict_str), incref(dict_descr));
 }
 

src/codegen/ast_interpreter.cpp

@@ -166,12 +166,6 @@ public:
 
     void setCurrentStatement(AST_stmt* stmt) { frame_info.stmt = stmt; }
 
-    Box* getGlobals() {
-        assert(0 && "check refcounting (of callers)");
-        assert(frame_info.globals);
-        return incref(frame_info.globals);
-    }
-
     FunctionMetadata* getMD() { return frame_info.md; }
     FrameInfo* getFrameInfo() { return &frame_info; }
     BoxedClosure* getPassedClosure() { return frame_info.passed_closure; }
@@ -1754,7 +1748,11 @@ Value ASTInterpreter::visit_list(AST_List* node) {
             items.push_back(v);
             listAppendInternalStolen(list, v.o);
         } catch (ExcInfo e) {
-            RELEASE_ASSERT(0, "check refcounting");
+            // The CFG currently converts all list expressions to something like
+            // #0 = <expr1>
+            // #1 = <expr2>
+            // #2 = [#0, #1]
+            RELEASE_ASSERT(0, "list elements should not be throwing");
         }
     }
 

src/codegen/irgen.cpp

@@ -456,6 +456,8 @@ static void emitBBs(IRGenState* irstate, TypeAnalysis* types, const OSREntryDesc
             ConcreteCompilerType* phi_type;
             phi_type = getTypeAtBlockStart(types, p.first, target_block);
 
+            irstate->getRefcounts()->setType(from_arg, RefType::BORROWED);
+
             ConcreteCompilerVariable* var = new ConcreteCompilerVariable(p.second, from_arg);
             (*initial_syms)[p.first] = var;
 
@@ -467,9 +469,8 @@ static void emitBBs(IRGenState* irstate, TypeAnalysis* types, const OSREntryDesc
             if (p.second == phi_type) {
                 // good to go
                 v = from_arg;
-                irstate->getRefcounts()->setType(v, RefType::BORROWED);
             } else if (p.second->canConvertTo(phi_type)) {
-                assert(0 && "check refcounting");
+                RELEASE_ASSERT(0, "this hasn't been hit in a very long time -- check refcounting");
                 // not sure if/when this happens, but if there's a type mismatch but one we know
                 // can be handled (such as casting from a subclass to a superclass), handle it:
                 ConcreteCompilerVariable* converted = var->makeConverted(*unbox_emitter, phi_type);
@@ -903,7 +904,7 @@ static void emitBBs(IRGenState* irstate, TypeAnalysis* types, const OSREntryDesc
             }
         }
         for (auto t : phi_args) {
-            assert(0 && "check refcounting");
+            RELEASE_ASSERT(0, "this hasn't been hit in a very long time -- check refcounting");
             std::get<0>(t)->addIncoming(std::get<1>(t), std::get<2>(t));
         }
     }

src/core/types.h

@@ -760,7 +760,6 @@ public:
     void setDictBacked(STOLEN(Box*) d);
     // For instances with dict attrs:
     BORROWED(BoxedDict*) getDict();
-    void setDict(STOLEN(BoxedDict*) d);
 
 
     // Note, setattr does *not* steal a reference, but it probably should

src/runtime/builtin_modules/builtins.cpp

@@ -1274,71 +1274,6 @@ public:
     }
 };
 
-Box* exceptionNew(BoxedClass* cls, BoxedTuple* args) {
-    assert(0 && "check refcounting");
-    if (!PyType_Check(cls))
-        raiseExcHelper(TypeError, "exceptions.__new__(X): X is not a type object (%s)", getTypeName(cls));
-
-    if (!isSubclass(cls, BaseException))
-        raiseExcHelper(TypeError, "BaseException.__new__(%s): %s is not a subtype of BaseException",
-                       getNameOfClass(cls), getNameOfClass(cls));
-
-    BoxedException* rtn = new (cls) BoxedException();
-
-    // TODO: this should be a MemberDescriptor and set during init
-    if (args->size() == 1)
-        rtn->giveAttr("message", args->elts[0]);
-    else
-        rtn->giveAttr("message", boxString(""));
-    return rtn;
-}
-
-Box* exceptionStr(Box* b) {
-    assert(0 && "check refcounting");
-    // TODO In CPython __str__ and __repr__ pull from an internalized message field, but for now do this:
-    static BoxedString* message_str = getStaticString("message");
-    Box* message = b->getattr(message_str);
-    assert(message);
-    message = str(message);
-    assert(message->cls == str_cls);
-
-    return message;
-}
-
-Box* exceptionRepr(Box* b) {
-    assert(0 && "check refcounting");
-    // TODO In CPython __str__ and __repr__ pull from an internalized message field, but for now do this:
-    static BoxedString* message_str = getStaticString("message");
-    Box* message = b->getattr(message_str);
-    assert(message);
-    message = repr(message);
-    assert(message->cls == str_cls);
-
-    BoxedString* message_s = static_cast<BoxedString*>(message);
-    return boxStringTwine(llvm::Twine(getTypeName(b)) + "(" + message_s->s() + ",)");
-}
-
-static BoxedClass* makeBuiltinException(BoxedClass* base, const char* name, int size = 0) {
-    assert(0 && "check refcounting");
-    if (size == 0)
-        size = base->tp_basicsize;
-
-    BoxedClass* cls = BoxedClass::create(type_cls, base, offsetof(BoxedException, attrs), 0, size, false, name);
-    cls->giveAttr("__module__", boxString("exceptions"));
-
-    if (base == object_cls) {
-        cls->giveAttr("__new__",
-                      new BoxedFunction(FunctionMetadata::create((void*)exceptionNew, UNKNOWN, 1, true, true)));
-        cls->giveAttr("__str__", new BoxedFunction(FunctionMetadata::create((void*)exceptionStr, STR, 1)));
-        cls->giveAttr("__repr__", new BoxedFunction(FunctionMetadata::create((void*)exceptionRepr, STR, 1)));
-    }
-
-    cls->freeze();
-
-    builtins_module->giveAttr(name, cls);
-    return cls;
-}
-
 BoxedClass* enumerate_cls;
 class BoxedEnumerate : public Box {
 private:

src/runtime/descr.cpp

@@ -743,42 +743,9 @@ static Box* wrapperObjectRepr(Box* _o) {
                                getTypeName(wp->obj), wp->obj);
 }
 
-Box* BoxedWrapperObject::__call__(BoxedWrapperObject* self, Box* args, Box* kwds) {
-    assert(0 && "check refcounting");
-    STAT_TIMER(t0, "us_timer_boxedwrapperobject_call", (self->cls->is_user_defined ? 10 : 20));
-
-    assert(self->cls == wrapperobject_cls);
-    assert(args->cls == tuple_cls);
-    assert(!kwds || kwds->cls == dict_cls);
-
-    int flags = self->descr->wrapper->flags;
-    wrapperfunc wrapper = self->descr->wrapper->wrapper;
-    assert(self->descr->wrapper->offset > 0);
-
-    Box* rtn;
-    if (flags == PyWrapperFlag_KEYWORDS) {
-        wrapperfunc_kwds wk = (wrapperfunc_kwds)wrapper;
-        rtn = (*wk)(self->obj, args, self->descr->wrapped, kwds);
-    } else if (flags == PyWrapperFlag_PYSTON || flags == 0) {
-        rtn = (*wrapper)(self->obj, args, self->descr->wrapped);
-    } else if (flags == PyWrapperFlag_1ARG) {
-        if (PyTuple_GET_SIZE(args) != 0)
-            raiseExcHelper(TypeError, "%s() takes exactly 1 arguments (%ld given)", self->descr->wrapper->name.data(),
-                           PyTuple_GET_SIZE(args) + 1);
-        wrapperfunc_1arg wrapper_1arg = (wrapperfunc_1arg)wrapper;
-        rtn = (*wrapper_1arg)(self->obj, self->descr->wrapped);
-    } else if (flags == PyWrapperFlag_2ARG) {
-        if (PyTuple_GET_SIZE(args) != 1)
-            raiseExcHelper(TypeError, "%s() takes exactly 2 arguments (%ld given)", self->descr->wrapper->name.data(),
-                           PyTuple_GET_SIZE(args) + 1);
-        rtn = (*wrapper)(self->obj, PyTuple_GET_ITEM(args, 0), self->descr->wrapped);
-    } else {
-        RELEASE_ASSERT(0, "%d", flags);
-    }
-
-    if (!rtn)
-        throwCAPIException();
-    return rtn;
+// TODO this should be auto-generated as a slot wrapper:
+Box* BoxedWrapperObject::__call__(BoxedWrapperObject* self, Box* args, Box* kwargs) {
+    return BoxedWrapperObject::tppCall<CXX>(self, NULL, ArgPassSpec(0, 0, true, true), args, kwargs, NULL, NULL, NULL);
 }
 
 template <ExceptionStyle S>

src/runtime/objmodel.cpp

@@ -1009,13 +1009,6 @@ BoxedDict** Box::getDictPtr() {
     return d_ptr;
 }
 
-void Box::setDict(STOLEN(BoxedDict*) d) {
-    assert(0 && "check refcounting");
-    assert(cls->instancesHaveDictAttrs());
-
-    *getDictPtr() = d;
-}
-
 BORROWED(BoxedDict*) Box::getDict() {
     assert(cls->instancesHaveDictAttrs());
 
@@ -6985,7 +6978,12 @@ Box* _typeNew(BoxedClass* metatype, BoxedString* name, BoxedTuple* bases, BoxedD
         assert(!final_slot_names.size()); // would need to decref them here
     }
 
-    if (made->instancesHaveHCAttrs() || made->instancesHaveDictAttrs()) {
+    if ((made->instancesHaveHCAttrs() || made->instancesHaveDictAttrs())
+        && !(base->instancesHaveHCAttrs() || base->instancesHaveDictAttrs())) {
+
+        // We shouldn't be adding dict attrs to anything that didn't already have them:
+        assert(!made->instancesHaveDictAttrs());
+
         static BoxedString* dict_str = getStaticString("__dict__");
         made->setattr(dict_str, dict_descr, NULL);
     }

src/runtime/types.cpp

@@ -286,30 +286,6 @@ Box* Box::hasnextOrNullIC() {
     return this->cls->callHasnextIC(this, true);
 }
 
-extern "C" BoxedFunctionBase::BoxedFunctionBase(FunctionMetadata* md)
-    : weakreflist(NULL),
-      md(md),
-      closure(NULL),
-      defaults(NULL),
-      can_change_defaults(false),
-      modname(NULL),
-      name(NULL),
-      doc(NULL) {
-
-    if (md->source) {
-        assert(md->source->scoping->areGlobalsFromModule());
-        Box* globals_for_name = md->source->parent_module;
-
-        assert(0 && "check the refcounting here");
-        static BoxedString* name_str = getStaticString("__name__");
-        this->modname = xincref(globals_for_name->getattr(name_str));
-        this->doc = md->source->getDocString();
-    } else {
-        this->modname = PyString_InternFromString("__builtin__");
-        this->doc = incref(None);
-    }
-}
-
 extern "C" BoxedFunctionBase::BoxedFunctionBase(FunctionMetadata* md, std::initializer_list<Box*> defaults,
                                                 BoxedClosure* closure, Box* globals, bool can_change_defaults)
     : weakreflist(NULL),
@@ -499,7 +475,7 @@ BORROWED(BoxedString*) BoxedModule::getStringConstant(llvm::StringRef ast_str, b
         // that we would keep the previously-created string alive.
         // So, make sure to put it onto the keep_alive list.
         if (r && !PyString_CHECK_INTERNED(r)) {
-            assert(0 && "check refcounting");
+            RELEASE_ASSERT(0, "this codepath has been dead for a little while, make sure it still works");
             keep_alive.push_back(r);
             r = NULL;
         }
@@ -1388,28 +1364,18 @@ static Box* typeDict(Box* obj, void* context) {
 }
 
 static Box* typeSubDict(Box* obj, void* context) {
-    if (obj->cls->instancesHaveHCAttrs())
-        return incref(obj->getAttrWrapper());
-    if (obj->cls->instancesHaveDictAttrs())
-        return obj->getDict();
-    abort();
+    // This should only be getting called for hc-backed classes:
+    assert(obj->cls->instancesHaveHCAttrs());
+    return incref(obj->getAttrWrapper());
 }
 
 static void typeSubSetDict(BORROWED(Box*) obj, BORROWED(Box*) val, void* context) {
-    Py_INCREF(val);
-    if (obj->cls->instancesHaveDictAttrs()) {
-        RELEASE_ASSERT(val->cls == dict_cls, "");
-        obj->setDict(static_cast<BoxedDict*>(val));
-        return;
-    }
+    // This should only be getting called for hc-backed classes:
+    assert(obj->cls->instancesHaveHCAttrs());
 
-    if (obj->cls->instancesHaveHCAttrs()) {
-        obj->setDictBacked(val);
-        return;
-    }
+    Py_INCREF(val);
 
-    // This should have thrown an exception rather than get here:
-    abort();
+    obj->setDictBacked(val);
 }
 
 extern "C" void PyType_SetDict(PyTypeObject* type, STOLEN(PyObject*) dict) noexcept {

src/runtime/types.h

@@ -1035,7 +1035,6 @@ public:
     BoxedString* name; // __name__ (should be here or in one of the derived classes?)
     Box* doc;          // __doc__
 
-    BoxedFunctionBase(FunctionMetadata* md);
     BoxedFunctionBase(FunctionMetadata* md, std::initializer_list<Box*> defaults, BoxedClosure* closure = NULL,
                       Box* globals = NULL, bool can_change_defaults = false);
 

test/tests/dict_attrs.py

+# functools.partial has dict attrs
+import functools
+
+p = functools.partial(list)
+p.__dict__ = dict(a=3)
+print p.a
+
+class P(functools.partial):
+    pass
+p = P(list)
+p.__dict__ = dict(a=7)

test/tests/wrapperdesc.py

@@ -6,3 +6,4 @@ print type(None).__str__ is object.__str__
 print type(None).__str__ is None.__str__
 print type(None.__str__)
 print type(type(None).__str__.__get__(None, type(None)))
+print u"".__len__.__call__()