3197

Replace an integer with a boolean across two layers of function calls.

It has long been one of the ugliest consequences of my approach with
layers that functions might need to be introduced with unnecessary
arguments simply because we have no clean way to add parameters to a
function definition after the fact -- or to add the default argument
corresponding to that parameter in calls. This problem is exacerbated by
the redundant argument having to be passed in through multiple layers of
functions. In this instance:

In layer 20 we define write_memory with an argument called
'saving_instruction_products' which isn't used yet.

In layer 36 we reveal that we use this argument in a call to
should_update_refcounts_in_write_memory() -- where it is again not used
yet.

Layer 43 finally clarifies what we're shooting for:

  a) In general when we need to update some memory, we always want to
  update refcounts.

  b) The only exception is when we're reclaiming locals in a function
  that set up its stack frame using 'local-scope' (signalling that it
  wants immediate reclamation). At that point we avoid decrementing
  refcounts of 'escaping' addresses that are being returned, and we also
  avoid incrementing refcounts of products in the caller instruction.
  The latter case is basically why we need this boolean and its dance
  across 3 layers.

In summary, write_memory() needs to update refcounts except if:
  we're writing products for an instruction,
  the instruction is not a primitive, and
  the (callee) recipe for the instruction starts with 'local-scope'.

4 files changed, 9 insertions, 10 deletions

diff --git a/020run.cc b/020run.cc
index 143cc07c..77a132a9 100644
--- a/020run.cc
+++ b/020run.cc
@@ -91,7 +91,7 @@ void run_current_routine()
     }
     else {
       for (int i = 0; i < SIZE(current_instruction().products); ++i)
-        write_memory(current_instruction().products.at(i), products.at(i), i);
+        write_memory(current_instruction().products.at(i), products.at(i), /*saving instruction products rather than some other internal uses*/true);
     }
     // End of Instruction
     finish_instruction:;
@@ -277,7 +277,7 @@ vector<double> read_memory(reagent/*copy*/ x) {
   return result;
 }
 
-void write_memory(reagent/*copy*/ x, const vector<double>& data, const int /*only when called in the run loop above to save results; -1 otherwise*/ product_index) {
+void write_memory(reagent/*copy*/ x, const vector<double>& data, const bool saving_instruction_products) {
   // Begin Preprocess write_memory(x, data)
   if (!x.type) {
     raise << "can't write to '" << to_string(x) << "'; no type\n" << end();
diff --git a/036refcount.cc b/036refcount.cc
index 638aed24..51a0945a 100644
--- a/036refcount.cc
+++ b/036refcount.cc
@@ -18,7 +18,7 @@ def main [
 +mem: decrementing refcount of 1000: 1 -> 0
 
 :(before "End write_memory(x) Special-cases")
-if (should_update_refcounts_in_write_memory(product_index)) {
+if (should_update_refcounts_in_write_memory(saving_instruction_products)) {
   if (is_mu_address(x)) {
     assert(scalar(data));
     assert(x.value);
@@ -30,7 +30,7 @@ if (should_update_refcounts_in_write_memory(product_index)) {
 
 :(code)
 //: hook for a later layer
-bool should_update_refcounts_in_write_memory(int product_index) {
+bool should_update_refcounts_in_write_memory(bool conditional_update) {
   return true;
 }
 
diff --git a/043space.cc b/043space.cc
index 275640cd..bb221201 100644
--- a/043space.cc
+++ b/043space.cc
@@ -247,7 +247,7 @@ void try_reclaim_locals() {
       if (escaping(inst.products.at(i))) continue;
       trace(9999, "mem") << "clearing " << inst.products.at(i).original_string << end();
       zeros.resize(size_of(inst.products.at(i)));
-      write_memory(inst.products.at(i), zeros, /*always update refcounts*/-1);
+      write_memory(inst.products.at(i), zeros, /*always update refcounts*/false);
     }
   }
   trace(9999, "mem") << "automatically abandoning " << current_call().default_space << end();
@@ -259,11 +259,10 @@ void try_reclaim_locals() {
 //: since we don't decrement refcounts for escaping values above, make sure we
 //: don't increment them when the caller saves them either
 
-:(replace{} "bool should_update_refcounts_in_write_memory(int product_index)")
-bool should_update_refcounts_in_write_memory(int product_index) {
+:(replace{} "bool should_update_refcounts_in_write_memory(bool conditional_update)")
+bool should_update_refcounts_in_write_memory(bool conditional_update) {
   assert(Current_routine);  // run-time only
-  if (product_index == -1) return true;
-  assert(product_index >= 0);
+  if (!conditional_update) return true;
   const instruction& inst = current_instruction();
   if (inst.operation < MAX_PRIMITIVE_RECIPES) return true;
   if (!contains_key(Recipe, inst.operation)) return true;
diff --git a/074deep_copy.cc b/074deep_copy.cc
index 59f71286..34100de5 100644
--- a/074deep_copy.cc
+++ b/074deep_copy.cc
@@ -264,7 +264,7 @@ int deep_copy_address(const reagent& canonized_in, map<int, int>& addresses_copi
   payload.value = tmp.value;  // now modified for output
   vector<double> old_data = read_memory(payload);
   trace(9991, "run") << "deep-copy: really writing to " << payload.value << ' ' << to_string(payload) << " (old value " << to_string(old_data) << " new value " << to_string(data) << ")" << end();
-  write_memory(payload, data, -1);
+  write_memory(payload, data, /*always update refcounts*/false);
   trace(9991, "run") << "deep-copy: output is " << to_string(data) << end();
   return out;
 }