Merge pull request #862 from kmod/class_gc

Class gc fix: integrate with the finalization logic

from_cpython/Python/dtoa.c

@@ -118,6 +118,9 @@
 
 #include "Python.h"
 
+// Pyston change: disable custom memory managment because it confuses our GC
+#define Py_USING_MEMORY_DEBUGGER 1
+
 /* if PY_NO_SHORT_FLOAT_REPR is defined, then don't even try to compile
    the following code */
 #ifndef PY_NO_SHORT_FLOAT_REPR

src/analysis/type_analysis.cpp

@@ -222,7 +222,7 @@ private:
         return rtn;
     }
 
-    bool hasFixedBinops(CompilerType* type) {
+    bool hasFixedOps(CompilerType* type) {
         // This is non-exhaustive:
         return type == STR || type == INT || type == FLOAT || type == LIST || type == DICT;
     }
@@ -230,7 +230,7 @@ private:
     void* visit_augbinop(AST_AugBinOp* node) override {
         CompilerType* left = getType(node->left);
         CompilerType* right = getType(node->right);
-        if (!hasFixedBinops(left) || !hasFixedBinops(right))
+        if (!hasFixedOps(left) || !hasFixedOps(right))
             return UNKNOWN;
 
         // TODO this isn't the exact behavior
@@ -259,7 +259,7 @@ private:
     void* visit_binop(AST_BinOp* node) override {
         CompilerType* left = getType(node->left);
         CompilerType* right = getType(node->right);
-        if (!hasFixedBinops(left) || !hasFixedBinops(right))
+        if (!hasFixedOps(left) || !hasFixedOps(right))
             return UNKNOWN;
 
         // TODO this isn't the exact behavior
@@ -505,12 +505,20 @@ private:
 
     void* visit_unaryop(AST_UnaryOp* node) override {
         CompilerType* operand = getType(node->operand);
+        if (!hasFixedOps(operand))
+            return UNKNOWN;
 
         // TODO this isn't the exact behavior
         BoxedString* name = getOpName(node->op_type);
         CompilerType* attr_type = operand->getattrType(name, true);
+
+        if (attr_type == UNDEF)
+            attr_type = UNKNOWN;
+
         std::vector<CompilerType*> arg_types;
-        return attr_type->callType(ArgPassSpec(0), arg_types, NULL);
+        CompilerType* rtn_type = attr_type->callType(ArgPassSpec(0), arg_types, NULL);
+        rtn_type = unboxedType(rtn_type->getConcreteType());
+        return rtn_type;
     }
 
     void* visit_yield(AST_Yield*) override { return UNKNOWN; }

src/codegen/compvars.cpp

@@ -824,7 +824,9 @@ public:
     struct Sig {
         std::vector<ConcreteCompilerType*> arg_types;
         CompilerType* rtn_type;
-        int ndefaults;
+        int ndefaults = 0;
+        bool takes_varargs = false;
+        bool takes_kwargs = false;
     };
 
 private:
@@ -847,14 +849,20 @@ public:
         RELEASE_ASSERT(!argspec.has_starargs, "");
         RELEASE_ASSERT(!argspec.has_kwargs, "");
         RELEASE_ASSERT(argspec.num_keywords == 0, "");
+        RELEASE_ASSERT(!keyword_names || keyword_names->empty() == 0, "");
 
         for (int i = 0; i < sigs.size(); i++) {
             Sig* sig = sigs[i];
-            if (arg_types.size() < sig->arg_types.size() - sig->ndefaults || arg_types.size() > sig->arg_types.size())
+            int num_normal_args = sig->arg_types.size() - ((sig->takes_varargs ? 1 : 0) + (sig->takes_kwargs ? 1 : 0));
+            if (arg_types.size() < num_normal_args - sig->ndefaults)
+                continue;
+            if (!sig->takes_varargs && arg_types.size() > sig->arg_types.size())
                 continue;
 
             bool works = true;
             for (int j = 0; j < arg_types.size(); j++) {
+                if (j == num_normal_args)
+                    break;
                 if (!arg_types[j]->canConvertTo(sig->arg_types[j])) {
                     works = false;
                     break;
@@ -883,6 +891,8 @@ public:
             Sig* type_sig = new Sig();
             type_sig->rtn_type = fspec->rtn_type;
             type_sig->ndefaults = clf->paramspec.num_defaults;
+            type_sig->takes_varargs = clf->paramspec.takes_varargs;
+            type_sig->takes_kwargs = clf->paramspec.takes_kwargs;
 
             if (stripfirst) {
                 assert(fspec->arg_types.size() >= 1);
@@ -1294,6 +1304,14 @@ public:
         return boolFromI1(emitter, cmp);
     }
 
+    ConcreteCompilerVariable* unaryop(IREmitter& emitter, const OpInfo& info, ConcreteCompilerVariable* var,
+                                      AST_TYPE::AST_TYPE op_type) override {
+        ConcreteCompilerVariable* converted = var->makeConverted(emitter, BOXED_FLOAT);
+        auto rtn = converted->unaryop(emitter, info, op_type);
+        converted->decvref(emitter);
+        return rtn;
+    }
+
     CompilerVariable* getitem(IREmitter& emitter, const OpInfo& info, VAR* var, CompilerVariable* slice) override {
         ConcreteCompilerVariable* converted = var->makeConverted(emitter, BOXED_FLOAT);
         CompilerVariable* rtn = converted->getitem(emitter, info, slice);
@@ -1885,6 +1903,15 @@ public:
 
     ConcreteCompilerVariable* unaryop(IREmitter& emitter, const OpInfo& info, ConcreteCompilerVariable* var,
                                       AST_TYPE::AST_TYPE op_type) override {
+        BoxedString* attr = getOpName(op_type);
+
+        bool no_attribute = false;
+        ConcreteCompilerVariable* called_constant
+            = tryCallattrConstant(emitter, info, var, attr, true, ArgPassSpec(0, 0, 0, 0), {}, NULL, &no_attribute);
+
+        if (called_constant && !no_attribute)
+            return called_constant;
+
         return UNKNOWN->unaryop(emitter, info, var, op_type);
     }
 

src/gc/collector.cpp

@@ -48,9 +48,6 @@ std::list<Box*> objects_with_ordered_finalizers;
 static std::unordered_set<void*> roots;
 static std::vector<std::pair<void*, void*>> potential_root_ranges;
 
-// BoxedClasses in the program that are still needed.
-static std::unordered_set<BoxedClass*> class_objects;
-
 static std::unordered_set<void*> nonheap_roots;
 // Track the highest-addressed nonheap root; the assumption is that the nonheap roots will
 // typically all have lower addresses than the heap roots, so this can serve as a cheap
@@ -292,9 +289,6 @@ void registerPythonObject(Box* b) {
     if (hasOrderedFinalizer(b->cls)) {
         objects_with_ordered_finalizers.push_back(b);
     }
-    if (PyType_Check(b)) {
-        class_objects.insert((BoxedClass*)b);
-    }
 }
 
 void invalidateOrderedFinalizerList() {
@@ -654,35 +648,6 @@ static void markPhase() {
 
     graphTraversalMarking(stack, visitor);
 
-    // Some classes might be unreachable. Unfortunately, we have to keep them around for
-    // one more collection, because during the sweep phase, instances of unreachable
-    // classes might still end up looking at the class. So we visit those unreachable
-    // classes remove them from the list of class objects so that it can be freed
-    // in the next collection.
-    std::vector<BoxedClass*> classes_to_remove;
-    for (BoxedClass* cls : class_objects) {
-        GCAllocation* al = GCAllocation::fromUserData(cls);
-        if (!isMarked(al)) {
-            visitor.visit(cls);
-            classes_to_remove.push_back(cls);
-        }
-    }
-
-    // We added new objects to the stack again from visiting classes so we nee to do
-    // another (mini) traversal.
-    graphTraversalMarking(stack, visitor);
-
-    for (BoxedClass* cls : classes_to_remove) {
-        class_objects.erase(cls);
-    }
-
-    // The above algorithm could fail if we have a class and a metaclass -- they might
-    // both have been added to the classes to remove. In case that happens, make sure
-    // that the metaclass is retained for at least another collection.
-    for (BoxedClass* cls : classes_to_remove) {
-        class_objects.insert(cls->cls);
-    }
-
     // Objects with finalizers cannot be freed in any order. During the call to a finalizer
     // of an object, the finalizer expects the object's references to still point to valid
     // memory. So we root objects whose finalizers need to be called by placing them in a

src/gc/heap.cpp

@@ -128,7 +128,12 @@ void _bytesAllocatedTripped() {
 /// Finalizers
 
 bool hasOrderedFinalizer(BoxedClass* cls) {
-    if (cls->has_safe_tp_dealloc) {
+    if (PyType_FastSubclass(cls, Py_TPFLAGS_TYPE_SUBCLASS)) {
+        // Class objects need to be kept alive for at least as long as any objects that point
+        // to them, even if those objects are garbage (or involved in finalizer chains).
+        // Marking class objects as having finalizers should get us this behavior.
+        return true;
+    } else if (cls->has_safe_tp_dealloc) {
         ASSERT(!cls->tp_del, "class \"%s\" with safe tp_dealloc also has tp_del?", cls->tp_name);
         return false;
     } else if (cls->hasNonDefaultTpDealloc()) {

src/runtime/types.cpp

@@ -3382,6 +3382,7 @@ void setupRuntime() {
     mem = gc_alloc(sizeof(BoxedHeapClass), gc::GCKind::PYTHON);
     type_cls = ::new (mem) BoxedHeapClass(object_cls, &BoxedHeapClass::gcHandler, offsetof(BoxedClass, attrs),
                                           offsetof(BoxedClass, tp_weaklist), sizeof(BoxedHeapClass), false, NULL);
+    type_cls->has_safe_tp_dealloc = false;
     type_cls->tp_flags |= Py_TPFLAGS_TYPE_SUBCLASS;
     type_cls->tp_itemsize = sizeof(BoxedHeapClass::SlotOffset);
     PyObject_Init(object_cls, type_cls);

test/tests/float.py

@@ -108,3 +108,6 @@ for lhs in all_args:
 
 import sys
 print sys.float_info
+
+if 1:
+    x = -2.0

test/tests/str_functions.py

@@ -173,3 +173,11 @@ class C(object):
     def __str__(self):
         return "my class"
 print "{0}".format(C())
+
+def irgen_error():
+	for i in range(1):
+		fail = "test".format()
+		print fail
+		fail = "test {0} {1} {2}".format(1, 2, 3)
+		print fail
+irgen_error()