Convert tvSet family to take TypedValues by reference

Turns out I need to use tvSet with temporaries.  Also removes
unused tvSetObject*, and replaces some tvSetNull* callsites with using
make_tv temporaries.

hphp/runtime/base/tv_helpers.h

@@ -217,7 +217,6 @@ inline void tvDup(const TypedValue& fr, TypedValue& to) {
   tvRefcountedIncRef(&to);
 }
 
-
 /*
  * Duplicate a Cell from one location to another.  Is equivalent to
  * tvDup, with some added assertions.
@@ -269,74 +268,46 @@ inline const Cell* tvToCell(const TypedValue* tv) {
   return LIKELY(tv->m_type != KindOfRef) ? tv : tv->m_data.pref->tv();
 }
 
-template <bool respectRef>
-inline void tvSetImpl(const TypedValue* fr, TypedValue* to) {
-  assert(fr->m_type != KindOfRef);
-  if (respectRef) to = tvToCell(to);
-  DataType oldType = to->m_type;
-  uint64_t oldDatum = to->m_data.num;
-  cellDup(*fr, *to);
+/*
+ * Assign the value of the Cell in `fr' to `to', with appropriate
+ * reference count modifications.
+ *
+ * If `to' is KindOfRef, places the value of `fr' in the RefData
+ * pointed to by `to'.
+ *
+ * `to' must contain a live php value; use cellDup when it doesn't.
+ */
+inline void tvSet(const Cell& fr, TypedValue& inTo) {
+  assert(cellIsPlausible(&fr));
+  Cell* to = tvToCell(&inTo);
+  auto const oldType = to->m_type;
+  auto const oldDatum = to->m_data.num;
+  cellDup(fr, *to);
   tvRefcountedDecRefHelper(oldType, oldDatum);
 }
 
-// Assumes 'to' and 'fr' are live
-// Assumes that 'fr->m_type != KindOfRef'
-// If 'to->m_type == KindOfRef', this will perform the set
-// operation on the inner cell (to->m_data.pref)
-inline void tvSet(const TypedValue* fr, TypedValue* to) {
-  tvSetImpl<true>(fr, to);
-}
-
-// Same as tvSet, but does not dereference 'to' if it's KindOfRef.
-inline void tvSetIgnoreRef(const TypedValue* fr, TypedValue* to) {
-  tvSetImpl<false>(fr, to);
-}
-
-template <bool respectRef>
-inline void tvSetNullImpl(TypedValue* to) {
-  if (respectRef) to = tvToCell(to);
-  DataType oldType = to->m_type;
-  uint64_t oldDatum = to->m_data.num;
-  tvWriteNull(to);
+/*
+ * Assign the value of the Cell in `fr' to `to', with appropriate
+ * reference count modifications.
+ *
+ * If `to' is KindOfRef, this function will decref the RefData and
+ * replace it with the value in `fr', unlike tvSet.
+ *
+ * `to' must contain a live php value; use cellDup when it doesnt.
+ *
+ * Post: `to' is a Cell.
+ */
+inline void tvSetIgnoreRef(const Cell& fr, TypedValue& to) {
+  assert(cellIsPlausible(&fr));
+  auto const oldType = to.m_type;
+  auto const oldDatum = to.m_data.num;
+  cellDup(fr, to);
   tvRefcountedDecRefHelper(oldType, oldDatum);
 }
 
-// Assumes 'to' is live
-// If 'to->m_type == KindOfRef', this will perform the set
-// operation on the inner cell (to->m_data.pref)
-inline void tvSetNull(TypedValue* to) {
-  tvSetNullImpl<true>(to);
-}
-
-// Same as tvSetNull, but does not dereference 'to' if it's KindOfRef.
-inline void tvSetNullIgnoreRef(TypedValue* to) {
-  tvSetNullImpl<false>(to);
-}
-
-template <bool respectRef>
-inline void tvSetObjectImpl(ObjectData* pobj, TypedValue* to) {
-  if (respectRef) to = tvToCell(to);
-  DataType oldType = to->m_type;
-  uint64_t oldDatum = to->m_data.num;
-  tvWriteObject(pobj, to);
-  tvRefcountedDecRefHelper(oldType, oldDatum);
-}
-
-// Assumes 'to' is live
-// If 'to->m_type == KindOfRef', this will perform the set
-// operation on the inner cell (to->m_data.pref)
-inline void tvSetObject(ObjectData* pobj, TypedValue* to) {
-  tvSetObjectImpl<true>(pobj, to);
-}
-
-// Same as tvSetObject, but does not dereference 'to' if it's KindOfRef.
-inline void tvSetObjectIgnoreRef(ObjectData* pobj, TypedValue* to) {
-  tvSetObjectImpl<false>(pobj, to);
-}
-
 // Assumes 'to' and 'fr' are live
 // Assumes that 'fr->m_type == KindOfRef'
-inline void tvBind(TypedValue * fr, TypedValue * to) {
+inline void tvBind(TypedValue* fr, TypedValue* to) {
   assert(fr->m_type == KindOfRef);
   DataType oldType = to->m_type;
   uint64_t oldDatum = to->m_data.num;

hphp/runtime/ext/asio/gen_array_wait_handle.cpp

@@ -95,7 +95,7 @@ Object c_GenArrayWaitHandle::ti_create(CArrRef dependencies) {
     auto child = static_cast<c_WaitHandle*>(current->m_data.pobj);
 
     if (child->isSucceeded()) {
-      tvSetIgnoreRef(child->getResult(), current);
+      tvSetIgnoreRef(*child->getResult(), *current);
     } else if (child->isFailed()) {
       putException(exception, child->getException());
     } else {
@@ -154,7 +154,7 @@ void c_GenArrayWaitHandle::onUnblocked() {
     auto child = static_cast<c_WaitHandle*>(current->m_data.pobj);
 
     if (child->isSucceeded()) {
-      tvSetIgnoreRef(child->getResult(), current);
+      tvSetIgnoreRef(*child->getResult(), *current);
     } else if (child->isFailed()) {
       putException(m_exception, child->getException());
     } else {

hphp/runtime/ext/asio/gen_map_wait_handle.cpp

@@ -91,7 +91,7 @@ Object c_GenMapWaitHandle::ti_create(CVarRef dependencies) {
     auto child = static_cast<c_WaitHandle*>(current->m_data.pobj);
 
     if (child->isSucceeded()) {
-      tvSetIgnoreRef(child->getResult(), current);
+      tvSetIgnoreRef(*child->getResult(), *current);
     } else if (child->isFailed()) {
       putException(exception, child->getException());
     } else {
@@ -144,7 +144,7 @@ void c_GenMapWaitHandle::onUnblocked() {
     auto child = static_cast<c_WaitHandle*>(current->m_data.pobj);
 
     if (child->isSucceeded()) {
-      tvSetIgnoreRef(child->getResult(), current);
+      tvSetIgnoreRef(*child->getResult(), *current);
     } else if (child->isFailed()) {
       putException(m_exception, child->getException());
     } else {

hphp/runtime/ext/asio/gen_vector_wait_handle.cpp

@@ -87,7 +87,7 @@ Object c_GenVectorWaitHandle::ti_create(CVarRef dependencies) {
     auto child = static_cast<c_WaitHandle*>(current->m_data.pobj);
 
     if (child->isSucceeded()) {
-      tvSetIgnoreRef(child->getResult(), current);
+      tvSetIgnoreRef(*child->getResult(), *current);
     } else if (child->isFailed()) {
       putException(exception, child->getException());
     } else {
@@ -136,7 +136,7 @@ void c_GenVectorWaitHandle::onUnblocked() {
     auto child = static_cast<c_WaitHandle*>(current->m_data.pobj);
 
     if (child->isSucceeded()) {
-      tvSetIgnoreRef(child->getResult(), current);
+      tvSetIgnoreRef(*child->getResult(), *current);
     } else if (child->isFailed()) {
       putException(m_exception, child->getException());
     } else {

hphp/runtime/ext/asio/set_result_to_ref_wait_handle.cpp

@@ -56,7 +56,7 @@ void c_SetResultToRefWaitHandle::ti_setoncreatecallback(CVarRef callback) {
 Object c_SetResultToRefWaitHandle::ti_create(CObjRef wait_handle, VRefParam ref) {
   TypedValue* var_or_cell = ref->asTypedValue();
   if (wait_handle.isNull()) {
-    tvSetNull(var_or_cell);
+    tvSet(make_tv<KindOfNull>(), *var_or_cell);
     return wait_handle;
   }
 
@@ -70,13 +70,13 @@ Object c_SetResultToRefWaitHandle::ti_create(CObjRef wait_handle, VRefParam ref)
 
   // succeeded? set result to ref and give back succeeded wait handle
   if (wh->isSucceeded()) {
-    tvSet(wh->getResult(), var_or_cell);
+    tvSet(*wh->getResult(), *var_or_cell);
     return wh;
   }
 
   // failed? reset ref and give back failed wait handle
   if (wh->isFailed()) {
-    tvSetNull(var_or_cell);
+    tvSet(make_tv<KindOfNull>(), *var_or_cell);
     return wh;
   }
 
@@ -128,7 +128,7 @@ void c_SetResultToRefWaitHandle::markAsSucceeded(const TypedValue* result) {
   RefData* ref = m_ref;
 
   m_ref = nullptr;
-  tvSetIgnoreRef(result, ref->tv());
+  tvSetIgnoreRef(*result, *ref->tv());
   decRefRef(ref);
 
   setResult(result);
@@ -139,7 +139,7 @@ void c_SetResultToRefWaitHandle::markAsFailed(CObjRef exception) {
   RefData* ref = m_ref;
 
   m_ref = nullptr;
-  tvSetNullIgnoreRef(ref->tv());
+  tvSetIgnoreRef(make_tv<KindOfNull>(), *ref->tv());
   decRefRef(ref);
 
   setException(exception.get());

hphp/runtime/vm/bytecode.cpp

@@ -4718,7 +4718,7 @@ inline void OPTBLD_INLINE VMExecutionContext::iopSetL(PC& pc) {
   assert(local < m_fp->m_func->numLocals());
   Cell* fr = m_stack.topC();
   TypedValue* to = frame_local(m_fp, local);
-  tvSet(fr, to);
+  tvSet(*fr, *to);
 }
 
 inline void OPTBLD_INLINE VMExecutionContext::iopSetN(PC& pc) {
@@ -4729,7 +4729,7 @@ inline void OPTBLD_INLINE VMExecutionContext::iopSetN(PC& pc) {
   TypedValue* to = nullptr;
   lookupd_var(m_fp, name, tv2, to);
   assert(to != nullptr);
-  tvSet(fr, to);
+  tvSet(*fr, *to);
   memcpy((void*)tv2, (void*)fr, sizeof(TypedValue));
   m_stack.discard();
   decRefStr(name);
@@ -4743,7 +4743,7 @@ inline void OPTBLD_INLINE VMExecutionContext::iopSetG(PC& pc) {
   TypedValue* to = nullptr;
   lookupd_gbl(m_fp, name, tv2, to);
   assert(to != nullptr);
-  tvSet(fr, to);
+  tvSet(*fr, *to);
   memcpy((void*)tv2, (void*)fr, sizeof(TypedValue));
   m_stack.discard();
   decRefStr(name);
@@ -4764,7 +4764,7 @@ inline void OPTBLD_INLINE VMExecutionContext::iopSetS(PC& pc) {
                 classref->m_data.pcls->name()->data(),
                 name->data());
   }
-  tvSet(tv1, val);
+  tvSet(*tv1, *val);
   tvRefcountedDecRefCell(propn);
   memcpy(output, tv1, sizeof(TypedValue));
   m_stack.ndiscard(2);
@@ -6846,7 +6846,7 @@ inline void OPTBLD_INLINE VMExecutionContext::iopContRetC(PC& pc) {
   NEXT();
   c_Continuation* cont = frame_continuation(m_fp);
   cont->setDone();
-  tvSetIgnoreRef(m_stack.topC(), cont->m_value.asTypedValue());
+  tvSetIgnoreRef(*m_stack.topC(), *cont->m_value.asTypedValue());
   m_stack.popC();
 
   EventHook::FunctionExit(m_fp);

hphp/runtime/vm/instance.cpp

@@ -352,7 +352,7 @@ TypedValue* Instance::setProp(Class* ctx, const StringData* key,
       if (UNLIKELY(bindingAssignment)) {
         tvBind(val, propVal);
       } else {
-        tvSet(val, propVal);
+        tvSet(*val, *propVal);
       }
     }
     // Return a pointer to the property if it's a declared property
@@ -562,7 +562,7 @@ void Instance::unsetProp(Class* ctx, const StringData* key) {
     Slot propInd = declPropInd(propVal);
     if (propInd != kInvalidSlot) {
       // Declared property.
-      tvSetIgnoreRef((TypedValue*)&null_variant, propVal);
+      tvSetIgnoreRef(*null_variant.asTypedValue(), *propVal);
     } else {
       // Dynamic property.
       assert(o_properties.get() != nullptr);

hphp/runtime/vm/name_value_table.h

@@ -167,8 +167,8 @@ struct NameValueTable : private boost::noncopyable {
    */
   TypedValue* set(const StringData* name, const TypedValue* val) {
     TypedValue* target = findTypedValue(name);
-    tvSet(val->m_type == KindOfRef ? val->m_data.pref->tv() : val,
-          target);
+    tvSet(*(val->m_type == KindOfRef ? val->m_data.pref->tv() : val),
+          *target);
     return target;
   }
 

hphp/runtime/vm/unit.cpp

@@ -1031,7 +1031,7 @@ void Unit::defDynamicSystemConstant(const StringData* cnsName,
 
 static void setGlobal(void* cacheAddr, TypedValue *value,
                       StringData *name) {
-  tvSet(value, TargetCache::GlobalCache::lookupCreateAddr(cacheAddr, name));
+  tvSet(*value, *TargetCache::GlobalCache::lookupCreateAddr(cacheAddr, name));
 }
 
 void Unit::merge() {