oceani: free variables as soon as they go out of scope.
authorNeilBrown <neil@brown.name>
Sat, 6 Nov 2021 04:54:52 +0000 (15:54 +1100)
committerNeilBrown <neil@brown.name>
Mon, 8 Nov 2021 09:56:36 +0000 (20:56 +1100)
Each 'exec' now keeps track of the variables that go out-of-scope when
the exec completes.
CondScope variables need to be re-linked when they get merged.

We now poison a variable when it is freed to ensure it doesn't get used
again by mistake.

The final cleanup now only needs to handle global variables

Signed-off-by: NeilBrown <neil@brown.name>
csrc/oceani-tests.mdc
csrc/oceani.mdc

index 776c1a67dddc990ff67c8e1405b41db34c2ed0b2..dccfb2a47bc2bdb50f7f3a39e7c3adf09ccf1b01 100644 (file)
@@ -80,7 +80,7 @@ arguments separated from the name by commas.  For each test, there is a section
                @mv *.gcov coverage ; [ -f .gcov ] && mv .gcov coverage || true
                @ awk '/NOTEST/ { next } /^ *[1-9]/ {ran+=1} /^ *###/ {skip+=1} \
                    END {printf "coverage: %6.2f%%\n", ran * 100 / (ran + skip); \
-                        if (ran < (ran + skip) *0.958) exit(1) }' \
+                        if (ran < (ran + skip) *0.959) exit(1) }' \
                        coverage/oceani.mdc.gcov
                @rm -f .tmp*
 
index 215a9a709bca6b4ebe568a0b6a1e3ca2e5e252d5..821775c6c0dec0e11106dc1205462fbeccbf6286 100644 (file)
@@ -263,7 +263,7 @@ structures can be used.
                        free(s);
                        s = t;
                }
-               ## free context vars
+               ## free global vars
                ## free context types
                ## free context storage
                exit(context.parse_error ? 1 : 0);
@@ -498,8 +498,10 @@ Named type are stored in a simple linked list.  Objects of each type are
 
        static void free_value(struct type *type, struct value *v)
        {
-               if (type && v)
+               if (type && v) {
                        type->free(type, v);
+                       memset(v, 0x5a, type->size);
+               }
        }
 
        static void type_print(struct type *type, FILE *f)
@@ -846,6 +848,42 @@ cannot nest, so a declaration while a name is in-scope is an error.
                ## variable fields
        };
 
+When a scope closes, the values of the variables might need to be freed.
+This happens in the context of some `struct exec` and each `exec` will
+need to know which variables need to be freed when it completes.
+
+####### exec fields
+       struct variable *to_free;
+
+####### variable fields
+       struct exec *cleanup_exec;
+       struct variable *next_free;
+
+####### interp exec cleanup
+       {
+               struct variable *v;
+               for (v = e->to_free; v; v = v->next_free) {
+                       struct value *val = var_value(c, v);
+                       free_value(v->type, val);
+               }
+       }
+
+###### ast functions
+       static void variable_unlink_exec(struct variable *v)
+       {
+               struct variable **vp;
+               if (!v->cleanup_exec)
+                       return;
+               for (vp = &v->cleanup_exec->to_free;
+                   *vp; vp = &(*vp)->next_free) {
+                       if (*vp != v)
+                               continue;
+                       *vp = v->next_free;
+                       v->cleanup_exec = NULL;
+                       break;
+               }
+       }
+
 While the naming seems strange, we include local constants in the
 definition of variables.  A name declared `var := value` can
 subsequently be changed, but a name declared `var ::= value` cannot -
@@ -1010,13 +1048,14 @@ need to be freed.  For this we need to be able to find it, so assume that
                            v->merged == secondary->merged) {
                                v->scope = OutScope;
                                v->merged = primary;
+                               variable_unlink_exec(v);
                        }
        }
 
 ###### forward decls
        static struct value *var_value(struct parse_context *c, struct variable *v);
 
-###### free context vars
+###### free global vars
 
        while (context.varlist) {
                struct binding *b = context.varlist;
@@ -1027,10 +1066,11 @@ need to be freed.  For this we need to be able to find it, so assume that
                        struct variable *t = v;
 
                        v = t->previous;
-                       free_value(t->type, var_value(&context, t));
-                       if (t->depth == 0)
-                               // This is a global constant
-                               free_exec(t->where_decl);
+                       if (t->global) {
+                               free_value(t->type, var_value(&context, t));
+                               if (t->depth == 0)
+                                       free_exec(t->where_decl);
+                       }
                        free(t);
                }
        }
@@ -1065,7 +1105,7 @@ switch.  Other scopes are "sequential".
 
 When exiting a parallel scope we check if there are any variables that
 were previously pending and are still visible. If there are, then
-there weren't redeclared in the most recent scope, so they cannot be
+they weren't redeclared in the most recent scope, so they cannot be
 merged and must become out-of-scope.  If it is not the first of
 parallel scopes (based on `child_count`), we check that there was a
 previous binding that is still pending-scope.  If there isn't, the new
@@ -1139,7 +1179,8 @@ all pending-scope variables become conditionally scoped.
                return v;
        }
 
-       static void var_block_close(struct parse_context *c, enum closetype ct)
+       static void var_block_close(struct parse_context *c, enum closetype ct,
+                                   struct exec *e)
        {
                /* Close off all variables that are in_scope.
                 * Some variables in c->scope may already be not-in-scope,
@@ -1163,6 +1204,14 @@ all pending-scope variables become conditionally scoped.
                                 * closed the scope.
                                 */
                                continue;
+                       v->min_depth = c->scope_depth;
+                       if (v->scope == InScope) {
+                               /* This variable gets cleaned up when 'e' finishes */
+                               variable_unlink_exec(v);
+                               v->cleanup_exec = e;
+                               v->next_free = e->to_free;
+                               e->to_free = v;
+                       }
                        switch (ct) {
                        case CloseElse:
                        case CloseParallel: /* handle PendingScope */
@@ -1195,7 +1244,7 @@ all pending-scope variables become conditionally scoped.
                                         * parallel scope to be nested immediately
                                         * in a parallel scope, and that never
                                         * happens.
-                                        */
+                                        */                     // NOTEST
                                case OutScope:
                                        /* Not possible as we already tested for
                                         * OutScope
@@ -1219,9 +1268,9 @@ all pending-scope variables become conditionally scoped.
                                        for (v2 = v;
                                             v2 && v2->scope == PendingScope;
                                             v2 = v2->previous)
-                                               if (v2->type == Tlabel) {
+                                               if (v2->type == Tlabel)
                                                        v2->scope = CondScope;
-                                               else
+                                               else
                                                        v2->scope = OutScope;
                                        break;
                                case CondScope:
@@ -1261,7 +1310,7 @@ is started, so there is no need to allocate until the size is known.
        {
                if (!v->global) {
                        if (!c->local || !v->type)
-                               return NULL;
+                               return NULL;                    // NOTEST
                        if (v->frame_pos + v->type->size > c->local_size) {
                                printf("INVALID frame_pos\n");  // NOTEST
                                exit(2);                        // NOTEST
@@ -1377,6 +1426,7 @@ from the `exec_types` enum.
        struct exec {
                enum exec_types type;
                int line, column;
+               ## exec fields
        };
        struct binode {
                struct exec;
@@ -1480,6 +1530,15 @@ also want to know what sort of bracketing to use.
                        print_binode(cast(binode, e), indent, bracket); break;
                ## print exec cases
                }
+               if (e->to_free) {
+                       struct variable *v;
+                       do_indent(indent, "/* FREE");
+                       for (v = e->to_free; v; v = v->next_free)
+                               printf(" %.*s(%c%d+%d)", v->name->name.len, v->name->name.txt,
+                                      v->global ? 'G':'L',
+                                      v->frame_pos, v->type ? v->type->size:0);
+                       printf(" */\n");
+               }
        }
 
 ###### forward decls
@@ -1625,6 +1684,7 @@ in `rval`.
                ret.lval = lrv;
                ret.rval = rv;
                ret.type = rvtype;
+               ## interp exec cleanup
                return ret;
        }
 
@@ -2296,7 +2356,7 @@ or as an indented list of one parameter per line
                        $0->op = Func;
                        $0->left = reorder_bilist($<Ar);
                        $0->right = $<Bl;
-                       var_block_close(c, CloseSequential);
+                       var_block_close(c, CloseSequential, $0);
                        if (c->scope_stack && !c->parse_error) abort();
                }$
                | func main IN OpenScope OptNL Args OUT OptNL do Block Newlines ${
@@ -2304,7 +2364,7 @@ or as an indented list of one parameter per line
                        $0->op = Func;
                        $0->left = reorder_bilist($<Ar);
                        $0->right = $<Bl;
-                       var_block_close(c, CloseSequential);
+                       var_block_close(c, CloseSequential, $0);
                        if (c->scope_stack && !c->parse_error) abort();
                }$
                | func main NEWLINE OpenScope OptNL do Block Newlines ${
@@ -2312,7 +2372,7 @@ or as an indented list of one parameter per line
                        $0->op = Func;
                        $0->left = NULL;
                        $0->right = $<Bl;
-                       var_block_close(c, CloseSequential);
+                       var_block_close(c, CloseSequential, $0);
                        if (c->scope_stack && !c->parse_error) abort();
                }$
 
@@ -3665,7 +3725,6 @@ it is declared, and error will be raised as the name is created as
                struct value *val;
                v = v->merged;
                val = var_value(c, v);
-               free_value(v->type, val);
                if (v->type->prepare_type)
                        v->type->prepare_type(c, v->type, 0);
                if (b->right) {
@@ -3872,13 +3931,13 @@ casepart` to track a list of case parts.
                        $0->forpart = $<FP;
                        $0->thenpart = $<TP;
                        $0->looppart = $<WP;
-                       var_block_close(c, CloseSequential);
+                       var_block_close(c, CloseSequential, $0);
                        }$
                | ForPart OptNL WhilePart CondSuffix ${
                        $0 = $<CS;
                        $0->forpart = $<FP;
                        $0->looppart = $<WP;
-                       var_block_close(c, CloseSequential);
+                       var_block_close(c, CloseSequential, $0);
                        }$
                | WhilePart CondSuffix ${
                        $0 = $<CS;
@@ -3889,21 +3948,21 @@ casepart` to track a list of case parts.
                        $0->condpart = $<SP;
                        $CP->next = $0->casepart;
                        $0->casepart = $<CP;
-                       var_block_close(c, CloseSequential);
+                       var_block_close(c, CloseSequential, $0);
                        }$
                | SwitchPart : IN OptNL CasePart CondSuffix OUT Newlines ${
                        $0 = $<CS;
                        $0->condpart = $<SP;
                        $CP->next = $0->casepart;
                        $0->casepart = $<CP;
-                       var_block_close(c, CloseSequential);
+                       var_block_close(c, CloseSequential, $0);
                        }$
                | IfPart IfSuffix ${
                        $0 = $<IS;
                        $0->condpart = $IP.condpart; $IP.condpart = NULL;
                        $0->thenpart = $IP.thenpart; $IP.thenpart = NULL;
                        // This is where we close an "if" statement
-                       var_block_close(c, CloseSequential);
+                       var_block_close(c, CloseSequential, $0);
                        }$
 
        CondSuffix -> IfSuffix ${
@@ -3927,12 +3986,12 @@ casepart` to track a list of case parts.
        ElsePart -> else OpenBlock Newlines ${
                        $0 = new(cond_statement);
                        $0->elsepart = $<OB;
-                       var_block_close(c, CloseElse);
+                       var_block_close(c, CloseElse, $0->elsepart);
                }$
                | else OpenScope CondStatement ${
                        $0 = new(cond_statement);
                        $0->elsepart = $<CS;
-                       var_block_close(c, CloseElse);
+                       var_block_close(c, CloseElse, $0->elsepart);
                }$
 
        $*casepart
@@ -3940,7 +3999,7 @@ casepart` to track a list of case parts.
                        $0 = calloc(1,sizeof(struct casepart));
                        $0->value = $<Ex;
                        $0->action = $<Bl;
-                       var_block_close(c, CloseParallel);
+                       var_block_close(c, CloseParallel, $0->action);
                }$
 
        $*exec
@@ -3951,7 +4010,7 @@ casepart` to track a list of case parts.
 
        ThenPart -> then OpenBlock ${
                        $0 = $<OB;
-                       var_block_close(c, CloseSequential);
+                       var_block_close(c, CloseSequential, $0);
                }$
 
        $*binode
@@ -3961,33 +4020,33 @@ casepart` to track a list of case parts.
                        $0->op = Loop;
                        $0->left = $<UB;
                        $0->right = $<OB;
-                       var_block_close(c, CloseSequential);
-                       var_block_close(c, CloseSequential);
+                       var_block_close(c, CloseSequential, $0->right);
+                       var_block_close(c, CloseSequential, $0);
                }$
                | while OpenScope Expression OpenScope ColonBlock ${
                        $0 = new(binode);
                        $0->op = Loop;
                        $0->left = $<Exp;
                        $0->right = $<CB;
-                       var_block_close(c, CloseSequential);
-                       var_block_close(c, CloseSequential);
+                       var_block_close(c, CloseSequential, $0->right);
+                       var_block_close(c, CloseSequential, $0);
                }$
 
        $cond_statement
        IfPart -> if UseBlock OptNL then OpenBlock ${
                        $0.condpart = $<UB;
                        $0.thenpart = $<OB;
-                       var_block_close(c, CloseParallel);
+                       var_block_close(c, CloseParallel, $0.thenpart);
                }$
                | if OpenScope Expression OpenScope ColonBlock ${
                        $0.condpart = $<Ex;
                        $0.thenpart = $<CB;
-                       var_block_close(c, CloseParallel);
+                       var_block_close(c, CloseParallel, $0.thenpart);
                }$
                | if OpenScope Expression OpenScope OptNL then Block ${
                        $0.condpart = $<Ex;
                        $0.thenpart = $<Bl;
-                       var_block_close(c, CloseParallel);
+                       var_block_close(c, CloseParallel, $0.thenpart);
                }$
 
        $*exec