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'.

1 files changed, 4 insertions, 5 deletions

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;