Add RewriterVar::replaceAttr()

Previously, to replace a refcounted object in memory, we would do something like

array->getAttr(offset)->setType(OWNED);
array->setAttr(offset, new_val);

The problem is that this ends up emitting something like
x = array[offset];
Py_DECREF(x);
array[offset] = new_val;

which is not safe, since x can have a destructor that runs.
In this particular case, the destructor changed the value of array[offset].

It's actually pretty hard to get the right behavior (decref after setting the
new value) from outside the rewriter class, so add a new replaceAttr that does
these steps.

src/asm_writing/rewriter.cpp

@@ -693,6 +693,19 @@ void RewriterVar::setAttr(int offset, RewriterVar* val, SetattrType type) {
     rewriter->addAction([=]() { rewriter->_setAttr(this, offset, val); }, { this, val }, ActionType::MUTATION);
 }
 
+void RewriterVar::replaceAttr(int offset, RewriterVar* val, bool prev_nullable) {
+    RewriterVar* prev = this->getAttr(offset);
+
+    this->setAttr(offset, val, SetattrType::HANDED_OFF);
+    val->refConsumed();
+
+    if (prev_nullable) {
+        prev->setNullable(true);
+        prev->xdecref();
+    } else
+        prev->decref();
+}
+
 void Rewriter::_setAttr(RewriterVar* ptr, int offset, RewriterVar* val) {
     if (LOG_IC_ASSEMBLY)
         assembler->comment("_setAttr");

src/asm_writing/rewriter.h

@@ -155,6 +155,15 @@ public:
         REFUSED,
     };
     void setAttr(int offset, RewriterVar* other, SetattrType type = SetattrType::UNKNOWN);
+
+    // Replaces an owned ref with another one.  Does the equivalent of:
+    // Box* prev = this[offset];
+    // this[offset] = new_val;
+    // Py_[X]DECREF(prev);
+    //
+    // Calls new_val->refConsumed() for you.
+    void replaceAttr(int offset, RewriterVar* new_val, bool prev_nullable);
+
     RewriterVar* cmp(AST_TYPE::AST_TYPE cmp_type, RewriterVar* other, Location loc = Location::any());
     RewriterVar* toBool(Location loc = Location::any());
 

src/codegen/baseline_jit.cpp

@@ -678,16 +678,13 @@ void JitFragmentWriter::emitSetLocal(InternedString s, int vreg, bool set_closur
              v);
         v->refConsumed();
     } else {
-        RewriterVar* prev = vregs_array->getAttr(8 * vreg)->setNullable(true);
-        vregs_array->setAttr(8 * vreg, v, RewriterVar::SetattrType::HANDED_OFF);
-        v->refConsumed();
-
-        // TODO With definedness analysis, we could know whether we can skip this check (definitely defined)
-        // or not even load the previous value (definitely undefined).
+        // TODO With definedness analysis, we could know whether we needed to emit an decref/xdecref/neither.
         // The issue is that definedness analysis is somewhat expensive to compute, so we don't compute it
         // for the bjit.  We could try calculating it (which would require some re-plumbing), which might help
         // but I suspect is not that big a deal as long as the llvm jit implements this kind of optimization.
-        prev->xdecref();
+        bool prev_nullable = true;
+
+        vregs_array->replaceAttr(8 * vreg, v, prev_nullable);
     }
     if (LOG_BJIT_ASSEMBLY)
         comment("BJIT: emitSetLocal() end");

src/runtime/objmodel.cpp

@@ -1512,13 +1512,8 @@ void Box::setattr(BoxedString* attr, BORROWED(Box*) val, SetattrRewriteArgs* rew
                     RewriterVar* r_hattrs
                         = rewrite_args->obj->getAttr(cls->attrs_offset + offsetof(HCAttrs, attr_list), Location::any());
 
-                    // Don't need to do anything: just getting it and setting it to OWNED
-                    // will tell the auto-refcount system to decref it.
-                    r_hattrs->getAttr(offset * sizeof(Box*) + offsetof(HCAttrs::AttrList, attrs))
-                        ->setType(RefType::OWNED);
-                    r_hattrs->setAttr(offset * sizeof(Box*) + offsetof(HCAttrs::AttrList, attrs), rewrite_args->attrval,
-                                      RewriterVar::SetattrType::HANDED_OFF);
-                    rewrite_args->attrval->refConsumed();
+                    r_hattrs->replaceAttr(offset * sizeof(Box*) + offsetof(HCAttrs::AttrList, attrs),
+                                          rewrite_args->attrval, /* prev_nullable */ false);
 
                     rewrite_args->out_success = true;
                 }

test/tests/resurrection.py

-# expected: fail
 # Objects are allowed to resurrect themselves in their __del__ methods...
 
 x = None