View Issue Details

IDProjectCategoryView StatusLast Update
0000631LDMud 3.3Implementationpublic2018-01-29 21:57
ReporterSorcerer Assigned ToGnomi  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.3.718 
Target Version3.3.719Fixed in Version3.3.719 
Summary0000631: Inconsistent behaviour of nested inline closures
DescriptionConsider the following two functions:

void test1() {
  int i=1;

  if (i) write("Fun ok\n");

  funcall((: if (i) write("Inline 1 ok\n");
             funcall((: int i=$1; if (i) write("Inline 2 ok\n"); :),i);
          :));
}

void test2() {
  int i=1;

  if (i) write("Fun ok\n");

  funcall((: if (i) write("Inline 1 ok\n");
             funcall((: if (i) write("Inline 2 ok\n"); :));
          :));
}

Loading an object that defines these two functions results in an compiler warning "...line 7 before '$1; if (i)': Variable 'i' shadows previous declaration" which seems ok to me since i is in the implicit context of the 1st inline and therefore should also be in the implicit context of the 2nd one (calling test1() works fine).
BUT: calling test2() in this example (which causes no compile-time warnings) results in the following runtime error as soon as the 2nd inline closure is evaluated: "(eval_instruction) context_identifier: inter_context is NULL".

Now, either the implicit context of one inline closure is passed on as implicit context to another nested inline closure - in that case test2() has to work. Or the implicit context is not passed on - than test1() has to compile without compiler warnings, since i is not known in the scope of the 2nd inline closure.
TagsNo tags attached.
Attached Files
bug631.diff (26,902 bytes)   
Index: trunk.inline/test/t-mantis.c
===================================================================
--- trunk.inline/test/t-mantis.c	(Revision 2587)
+++ trunk.inline/test/t-mantis.c	(Arbeitskopie)
@@ -180,6 +180,145 @@
             return to_int("-"+num) == __INT_MIN__;
        :)
     }),
+    ({ "0000631-1", 0,
+        (:
+            int i=1;
+
+            return funcall(
+                (:
+                    if(i)
+                        return funcall((: return i; :));
+                :)) == 1;
+        :)
+    }),
+    ({ "0000631-2", 0,
+        (:
+            int i=1;
+
+            return funcall(
+                function int() : int j = 2
+                {
+                    if(i)
+                        return funcall((: return j; :));
+                }) == 2;
+        :)
+    }),
+    ({ "0000631-3", 0,
+        (:
+            int i=1;
+
+            return funcall(
+                function int()
+                {
+                    int j = 2;
+                    return funcall((: return i; :));
+                }) == 1;
+        :)
+    }),
+    ({ "0000631-4", 0,
+        (:
+            int i=1;
+
+            return funcall(
+                function int ()
+                {
+                    return funcall(
+                         function int () : int j=i
+                         {
+                             return j;
+                         });
+                });
+        :)
+    }),
+    ({ "0000631-5", 0,
+        (:
+            int a = 1;
+
+            return funcall(
+                function int() : closure cl = (: a :)
+                {
+                    return funcall(cl);
+                }) == 1;
+        :)
+    }),
+    ({ "0000631-6", 0,
+        (:
+            int a=1;
+
+            return funcall(
+                function int () : int b = a+1; int c = b+1
+                {
+                    return c;
+                }) == 3;
+        :)
+    }),
+    ({ "0000631-7", 0, /* #631 in conjunction with #537. */
+        (:
+            closure c;
+
+            {
+               int a = 1;
+            }
+
+            c = function int () : int b
+            {
+                return b;
+            };
+
+            return funcall(c) == 0;
+        :)
+    }),
+    ({ "0000631-8", 0, /* #631 in conjunction with #537. */
+        (:
+            closure c;
+
+            c = function int () : int b = 2
+            {
+                return b;
+            };
+
+            {
+                int d;
+
+                return d == 0;
+            }
+
+        :)
+    }),
+    ({ "0000631-9", 0, /* Check, that the array is only freed once. */
+        (:
+            closure c = function int() : string* a = ({ "" })
+            {
+                return 0;
+            };
+
+            return closurep(c); /* As long as it doesn't crash... */
+        :)
+    }),
+    ({ "0000631-10", 0,
+        function int(int a, int b, int c)
+        {
+
+            return funcall(funcall(funcall(
+                function :
+                    mixed i = 1;
+                    mixed j = function :
+                                 mixed j = function { return i; }
+                              { return j; };
+                { return j; }
+                ))) == 1;
+        }
+    }),
+    ({ "0000631-11", 0,
+        (:
+            return funcall(function int() :
+                    closure c = function int() : int a = 1 { return 2; };
+                    int b;
+                {
+                    return b;
+                }) == 0;
+        :)
+    }),
 });
 
 void run_test()
Index: trunk.inline/src/prolang.y
===================================================================
--- trunk.inline/src/prolang.y	(Revision 2587)
+++ trunk.inline/src/prolang.y	(Arbeitskopie)
@@ -622,6 +622,11 @@
       /* Index of the enclosing inline closure, or -1 if none.
        */
 
+    mp_int next;
+      /* Index of an enclosed inline closure.
+       * This is only used temporarily.
+       */
+
     /* --- Compilation information --- */
     mp_uint end;
       /* While compiling the closure: end of the program code before
@@ -2684,7 +2689,7 @@
 #ifdef USE_NEW_INLINES
 /*-------------------------------------------------------------------------*/
 static ident_t *
-add_context_name (ident_t *ident, fulltype_t type, int num)
+add_context_name (inline_closure_t *closure, ident_t *ident, fulltype_t type, int num)
 
 /* Declare a new context variable <ident> with the type <type> for the
  * currently compiled inline closure. The references of <type> are NOT adopted.
@@ -2697,7 +2702,7 @@
     int depth;
     block_scope_t * block;
 
-    depth = current_inline->block_depth+1;
+    depth = closure->block_depth+1;
     block = & block_scope[depth-1];
 
 #ifdef DEBUG_INLINES
@@ -2710,6 +2715,13 @@
     {
         yyerror("Too many context variables");
     }
+    else if (num < 0
+     && (block->first_local + block->num_locals >= 256
+      || block->first_local + block->num_locals >= MAX_LOCAL
+        ))
+    {
+        yyerror("Too many local variables");
+    }
     else
     {
         ref_fulltype_data(&type);
@@ -2724,9 +2736,21 @@
 
         /* Initialize the ident */
         ident->type = I_TYPE_LOCAL;
-        ident->u.local.num = num;
         ident->u.local.depth = depth;
-        ident->u.local.context = block->num_locals;
+        if (num < 0)
+        {
+            /* First initialize it as a local variable
+             * of the outer function, so they can be
+             * referenced during initialization.
+             */
+            ident->u.local.num = block->first_local + block->num_locals;
+            ident->u.local.context = -1;
+        }
+        else
+        {
+            ident->u.local.num = num;
+            ident->u.local.context = block->num_locals;
+        }
 
         /* Put the ident into the list of all locals.
          */
@@ -2752,6 +2776,8 @@
 
         /* Record the type */
         type_of_context[block->num_locals] = type;
+        if (num < 0)
+            type_of_locals[ident->u.local.num] = type;
 
         block->num_locals++;
     }
@@ -2782,46 +2808,115 @@
      && depth <= current_inline->block_depth
        )
     {
-        /* This local has been inherited - create the
-         * proper context variable.
+        inline_closure_t *closure;
+        mp_int closure_nr;
+
+        closure = current_inline;
+
+        /* Get the active enclosing closure.
+         * Skip closures whose context is currently being parsed.
          */
-        if (ident->u.local.context >= 0)
+        do
         {
-            if (!current_inline->parse_context
-             && current_inline->prev != -1
-             && depth <= INLINE_CLOSURE(current_inline->prev).block_depth
-               )
+            closure_nr = closure->prev;
+            if (closure_nr == -1)
+                break;
+            closure = &(INLINE_CLOSURE(closure_nr));
+        }
+        while (closure->parse_context);
+
+        if (current_inline->parse_context && closure_nr == -1)
+        {
+            /* We're in the context initializer of the first inline closure.
+             * So this must be a reference to a local variable.
+             */
+            *pType = LOCAL_TYPE(current_inline->full_local_type_start
+                                + ident->u.local.num
+                               );
+        }
+        else
+        {
+            /* This local has been inherited - create the
+             * proper context variable.
+             *
+             * Let's go back to the first inline closure
+             * that has to inherit this variable.
+             * And record that path in the ->next pointer.
+             */
+
+            fulltype_t type;
+
+            closure = current_inline;
+            closure_nr = closure - &(INLINE_CLOSURE(0));
+            closure->next = -1;
+
+            /* Go through all inline closures to our local variable
+             * and record this way using the .next pointers.
+             *
+             * Stop at the last inline closure before the variable's
+             * scope, because the information about that scope was
+             * saved there.
+             */
+            for (;;)
             {
-                /* Can't use outer context variables when compiling
-                 * an inline closure body.
-                 */
-                yyerrorf("Can't use context variable '%s' "
-                         "across closure boundaries", get_txt(ident->name));
-                ident = redeclare_local(ident, Type_Any, block_depth);
-                *pType = Type_Any;
+                inline_closure_t *outer_closure;
+                mp_int outer_closure_nr;
+
+                outer_closure_nr = closure->prev;
+                if (outer_closure_nr == -1)
+                    /* It is a local variable of the current function. */
+                    break;
+
+                outer_closure = &(INLINE_CLOSURE(outer_closure_nr));
+                outer_closure->next = closure_nr;
+
+                if (outer_closure->block_depth < depth)
+                    /* The variable belongs to outer_closure. */
+                    break;
+
+                closure = outer_closure;
+                closure_nr = outer_closure_nr;
             }
+
+            if (ident->u.local.context >= 0)
+            {
+                /* It's a context variable. */
+                type = LOCAL_TYPE(closure->full_context_type_start
+                                  + ident->u.local.context
+                                 );
+            }
             else
             {
-                /* We can use outer context variables when compiling
-                 * an inline closure context.
-                 */
-                *pType = LOCAL_TYPE(current_inline->full_context_type_start
-                                    + ident->u.local.context
-                                   );
+                /* It's a local variable. */
+                type = LOCAL_TYPE(closure->full_local_type_start
+                                  + ident->u.local.num
+                                 );
             }
-        }
-        else /* it's a local */
-        {
-            fulltype_t type;
 
-            type = LOCAL_TYPE(current_inline->full_local_type_start
-                              + ident->u.local.num
-                             );
-            if (!current_inline->parse_context)
+            /* Now pass this context variable through
+             * all surrounding inline closures.
+             */
+            while (MY_TRUE)
             {
-                ref_fulltype_data(&type);
-                ident = add_context_name( ident, type, ident->u.local.num);
+                /* Skip closures whose context is being parsed,
+                 * because current_inline is not created
+                 * in their runtime.
+                 */
+                if (!closure->parse_context)
+                {
+                    ref_fulltype_data(&type);
+                    ident = add_context_name(closure, ident, type,
+                         ident->u.local.context >= 0
+                         ? 256 + ident->u.local.context
+                         : ident->u.local.num);
+                }
+
+                if (closure->next == -1)
+                    break;
+
+                closure = &(INLINE_CLOSURE(closure->next));
             }
+
             *pType = type;
         }
     }
@@ -2833,6 +2928,39 @@
     return ident;
 } /* check_for_context_local() */
 
+/*-------------------------------------------------------------------------*/
+static void
+adapt_context_names (void)
+
+/* Convert all explicit context variables
+ * from local variables to context variables.
+ */
+
+{
+    int depth;
+    block_scope_t *scope;
+
+    depth = current_inline->block_depth+1;
+    scope = block_scope + depth - 1;
+
+    /* Are there locals of the given depth? */
+    if (all_locals && all_locals->u.local.depth >= depth)
+    {
+        ident_t *q = all_locals;
+
+        while (q != NULL && q->u.local.depth > depth)
+            q = q->next_all;
+
+        while (q != NULL && q->u.local.depth == depth)
+        {
+            q->u.local.context = q->u.local.num - scope->first_local;
+            q->u.local.num = -1;
+
+            q = q->next_all;
+        }
+    }
+} /* adapt_context_names() */
+
 #endif /* USE_NEW_INLINES */
 
 /*-------------------------------------------------------------------------*/
@@ -5293,6 +5421,7 @@
      * its rightful place.
      */
     {
+        int num_explicit_context = 0;
         int depth = current_inline->block_depth+1;
         block_scope_t * context = &(block_scope[depth-1]);
 
@@ -5337,8 +5466,16 @@
             {
                 if (lcmap[i] != -1)
                 {
-                    ins_f_code(F_LOCAL);
-                    ins_byte(lcmap[i]);
+                    if (lcmap[i] >= 256)
+                    {
+                        ins_f_code(F_CONTEXT_IDENTIFIER);
+                        ins_byte(lcmap[i] - 256);
+                    }
+                    else
+                    {
+                        ins_f_code(F_LOCAL);
+                        ins_byte(lcmap[i]);
+                    }
                     got_mapped = MY_TRUE;
 #ifdef DEBUG_INLINES
 printf("DEBUG:     -> F_LOCAL %d\n", lcmap[i]);
@@ -5353,16 +5490,21 @@
                     fatal("Explicite context var #%d has higher index than "
                           "implicite context variables.", i);
                 }
+                else
+                    num_explicit_context++;
             }
         } /* Push local vars */
 
         /* Add the context_closure instruction */
 #ifdef DEBUG_INLINES
-printf("DEBUG:     -> F_CONTEXT_CLOSURE %d %d\n", current_inline->function, context->num_locals);
+printf("DEBUG:     -> F_CONTEXT_CLOSURE %d %d %d\n", current_inline->function
+      , num_explicit_context, context->num_locals - num_explicit_context);
 #endif /* DEBUG_INLINES */
         ins_f_code(F_CONTEXT_CLOSURE);
         ins_short(current_inline->function);
-        ins_short(context->num_locals);
+        ins_byte(context->first_local);
+        ins_short(num_explicit_context);
+        ins_short(context->num_locals - num_explicit_context);
     } /* Complete F_CONTEXT_CLOSURE instruction */
 
 
@@ -6040,6 +6182,14 @@
           /* deactivate argument scope while parsing explicit context */
           block_scope[current_inline->block_depth+1].accessible = MY_FALSE;
           current_inline->parse_context = MY_TRUE;
+          assert(block_scope[current_inline->block_depth].first_local == 0);
+          if (current_inline->prev != -1 && INLINE_CLOSURE(current_inline->prev).parse_context)
+          {
+              block_scope_t *scope = block_scope + INLINE_CLOSURE(current_inline->prev).block_depth;
+              block_scope[current_inline->block_depth].first_local = scope->first_local + scope->num_locals;
+          }
+          else
+              block_scope[current_inline->block_depth].first_local = current_inline->num_locals;
       }
 
       inline_opt_context
@@ -6049,10 +6199,54 @@
 printf("DEBUG: After inline_opt_context: program size %"PRIuMPINT"\n", CURRENT_PROGRAM_SIZE);
 #endif /* DEBUG_INLINES */
 
+          /* Complete the F_CLEAR_LOCALS at the beginning of the context block. */
+          block_scope_t *scope = block_scope + current_inline->block_depth;
+          inline_closure_t *outer_closure;
+          int * outer_max_num_locals;
+
+          if (scope->num_locals > scope->num_cleared)
+          {
+              mem_block[A_PROGRAM].block[scope->addr+2]
+                = (char)(scope->num_locals - scope->num_cleared);
+          }
+
           /* reactivate argument scope */
           block_scope[current_inline->block_depth+1].accessible = MY_TRUE;
           current_inline->parse_context = MY_FALSE;
+          adapt_context_names();
 
+          /* Find the correct max_num_locals to update.
+           * That is the one of the last closure with parse_context set.
+           */
+          outer_max_num_locals = &(current_inline->max_num_locals);
+          outer_closure = current_inline;
+
+          while (outer_closure->prev != -1)
+          {
+              outer_closure = &(INLINE_CLOSURE(outer_closure->prev));
+              if (!outer_closure->parse_context)
+                  break;
+              outer_max_num_locals = &(outer_closure->max_num_locals);
+          }
+
+          if (scope->first_local + scope->num_locals > *outer_max_num_locals)
+              *outer_max_num_locals = scope->first_local + scope->num_locals;
+
+          /* Check whether we clobbered some other local or context variables. */
+          if (scope->num_locals || scope->clobbered)
+          {
+              if (current_inline->prev != -1)
+              {
+                  outer_closure = &(INLINE_CLOSURE(current_inline->prev));
+                  if (outer_closure->parse_context)
+                      block_scope[outer_closure->block_depth].clobbered = MY_TRUE;
+                  else
+                      block_scope[current_inline->block_depth-1].clobbered = MY_TRUE;
+              }
+              else
+                  block_scope[current_inline->block_depth-1].clobbered = MY_TRUE;
+          }
+
           if (!inline_closure_prototype($4))
               YYACCEPT;
       }
@@ -6115,7 +6309,7 @@
 printf("DEBUG: After L_END_INLINE: program size %"PRIuMPINT"\n", CURRENT_PROGRAM_SIZE);
 #endif /* DEBUG_INLINES */
 
-         /* Complete the F_CLEAR_LOCALS at the baginning of the block. */
+         /* Complete the F_CLEAR_LOCALS at the beginning of the block. */
          block_scope_t *scope = block_scope + block_depth - 1;
 
          if (use_local_scopes && scope->num_locals > scope->num_cleared)
@@ -13950,10 +14144,9 @@
             q = name;
         }
         else
-            q = add_context_name(name, actual_type, -1);
+            q = add_context_name(current_inline, name, actual_type, -1);
 
-        lv->u.simple[0] = F_PUSH_CONTEXT_LVALUE;
-        lv->u.simple[1] = q->u.local.context;
+        scope = block_scope + current_inline->block_depth;
     }
     else
     {
@@ -13961,30 +14154,32 @@
             q = redeclare_local(name, actual_type, block_depth);
         else
             q = add_local_name(name, actual_type, block_depth);
+    }
 
-        if (use_local_scopes && scope->clobbered)
-        {
-            /* finish the previous CLEAR_LOCALS, if any */
-            if (scope->num_locals - 1 > scope->num_cleared)
-                mem_block[A_PROGRAM].block[scope->addr+2]
-                  = (char)(scope->num_locals - 1 - scope->num_cleared);
-            scope->clobbered = MY_FALSE;
-            scope->num_cleared = scope->num_locals - 1;
-        }
-        if (use_local_scopes && scope->num_locals == scope->num_cleared + 1)
-        {
-            /* First definition of a local, so insert the
-             * clear_locals bytecode and remember its position
-             */
-            scope->addr = mem_block[A_PROGRAM].current_size;
-            ins_f_code(F_CLEAR_LOCALS);
-            ins_byte(scope->first_local + scope->num_cleared);
-            ins_byte(0);
-        }
+    if (use_local_scopes && scope->clobbered)
+    {
+        /* finish the previous CLEAR_LOCALS, if any */
+        if (scope->num_locals - 1 > scope->num_cleared)
+            mem_block[A_PROGRAM].block[scope->addr+2]
+              = (char)(scope->num_locals - 1 - scope->num_cleared);
+        scope->clobbered = MY_FALSE;
+        scope->num_cleared = scope->num_locals - 1;
+    }
 
-        lv->u.simple[0] = F_PUSH_LOCAL_VARIABLE_LVALUE;
-        lv->u.simple[1] = q->u.local.num;
+    if (use_local_scopes && scope->num_locals == scope->num_cleared + 1)
+    {
+        /* First definition of a local, so insert the
+         * clear_locals bytecode and remember its position
+         */
+        scope->addr = mem_block[A_PROGRAM].current_size;
+        ins_f_code(F_CLEAR_LOCALS);
+        ins_byte(scope->first_local + scope->num_cleared);
+        ins_byte(0);
     }
+
+    lv->u.simple[0] = F_PUSH_LOCAL_VARIABLE_LVALUE;
+    lv->u.simple[1] = q->u.local.num;
+
 #else /* USE_NEW_INLINES */
     if (redeclare)
         q = redeclare_local(name, actual_type, block_depth);
@@ -14024,38 +14219,16 @@
          * initializer, as the default svalue-0 is not a valid float value.
          */
 
-        Bool need_value = MY_FALSE;
 %line
-
-#ifdef USE_NEW_INLINES
-        /* When parsing context variables, the context_closure instruction
-         * expects a value on the stack. If we do a float initialization,
-         * we leave the 0.0 on the stack, otherwise we'll push a 0.
-         * For normal float locals, we'll create the bytecode to assign
-         * the float 0.
-         */
-        need_value = current_inline && current_inline->parse_context;
-#endif /* USE_NEW_INLINES */
-
         if (!(actual_type.typeflags & TYPE_MOD_POINTER)
          && (actual_type.typeflags & PRIMARY_TYPE_MASK) == TYPE_FLOAT
           )
         {
             ins_f_code(F_FCONST0);
+            if (!add_lvalue_code(lv, F_VOID_ASSIGN))
+                return;
 
-            if (!need_value)
-            {
-                if (!add_lvalue_code(lv, F_VOID_ASSIGN))
-                    return;
-            }
-              
-            need_value = MY_FALSE;
         } /* if (float variable) */
-
-        if (need_value) /* If we still need a value... */
-        {
-            ins_number(0);
-        }
     }
 } /* define_local_variable() */
 
@@ -14097,18 +14270,8 @@
     if (type2.typeflags & TYPE_MOD_REFERENCE)
         yyerror("Can't trace reference assignments");
 
-    /* If we're parsing a context variable, just leave the
-     * value on the stack for the context_closure instruction.
-     * For normal locals, add the bytecode to create the lvalue
-     * and do the assignment.
-     */
-#ifdef USE_NEW_INLINES
-    if (!current_inline || !current_inline->parse_context)
-#endif /* USE_NEW_INLINES */
-    {
-        if (!add_lvalue_code(lv, F_VOID_ASSIGN))
-            return;
-    } /* parsed context var */
+    if (!add_lvalue_code(lv, F_VOID_ASSIGN))
+        return;
 } /* init_local_variable() */
 
 /*-------------------------------------------------------------------------*/
Index: trunk.inline/src/interpret.c
===================================================================
--- trunk.inline/src/interpret.c	(Revision 2587)
+++ trunk.inline/src/interpret.c	(Arbeitskopie)
@@ -8543,7 +8543,7 @@
 
     CASE(F_CLOSURE);            /* --- closure <ix> <inhIndex> --- */
 #ifdef USE_NEW_INLINES
-    CASE(F_CONTEXT_CLOSURE); /* --- context_closure <ix> <num> --- */
+    CASE(F_CONTEXT_CLOSURE); /* --- context_closure <ix> <vix> <num_ex> <num_im> --- */
 #endif /* USE_NEW_INLINES */
     {
         /* Push the closure value <ix> and <inhIndex> onto the stack.
@@ -8564,8 +8564,10 @@
          * of the directly referenced inherited function.
 #ifdef USE_NEW_INLINES
          *
-         * If it is a context closure, the context is sized to and initialized
-         * with the uint16 <num> values on the stack.
+         * If it is a context closure, the context is sized to 
+         * uint16 <num_ex>+<num_in> values, uint16 <num_ex> values
+         * are taken from the local variables beginning at <vix>,
+         * uint16 <num_im> values are taken from the stack.
 #endif
          */
 
@@ -8573,18 +8575,21 @@
         /* TODO: int32 */ int ix;
         /* TODO: uint16 */ unsigned short inhIndex;
 #ifdef USE_NEW_INLINES
-        unsigned short context_size;
+        unsigned short explicit_context_size, implicit_context_size;
+        svalue_t * explicit_context;
 #endif /* USE_NEW_INLINES */
 
         inhIndex = 0;
 #ifdef USE_NEW_INLINES
-        context_size = 0;
+        explicit_context_size = implicit_context_size = 0;
 #endif /* USE_NEW_INLINES */
         LOAD_SHORT(tmp_ushort, pc);
 #ifdef USE_NEW_INLINES
         if (instruction == F_CONTEXT_CLOSURE)
         {
-            LOAD_SHORT(context_size, pc);
+            explicit_context = fp + LOAD_UINT8(pc);
+            LOAD_SHORT(explicit_context_size, pc);
+            LOAD_SHORT(implicit_context_size, pc);
         }
         else
         {
@@ -8603,7 +8608,7 @@
 #ifndef USE_NEW_INLINES
             closure_literal(sp, ix, inhIndex);
 #else /* USE_NEW_INLINES */
-            closure_literal(sp, ix, inhIndex, context_size);
+            closure_literal(sp, ix, inhIndex, explicit_context_size + implicit_context_size);
 #endif /* USE_NEW_INLINES */
             /* If out of memory, this will set sp to svalue-0 and
              * throw an error.
@@ -8618,13 +8623,29 @@
                       "closure type %d.\n", sp->x.closure_type);
 #endif
             /* Now copy the context values */
-            if (context_size != 0)
+            if (explicit_context_size != 0)
             {
                 unsigned short i;
-                svalue_t * arg = sp - context_size;
                 svalue_t * context = sp->u.lambda->context;
 
-                for (i = 0; i < context_size; i++)
+                for (i = 0; i < explicit_context_size; i++)
+                {
+                    transfer_svalue_no_free(context+i, explicit_context+i);
+
+                    /* Set it to T_INVALID, as it is still a variable of
+                     * the function frame and will be freed on return.
+                     */
+                    explicit_context[i].type = T_INVALID;
+                }
+            }
+
+            if (implicit_context_size != 0)
+            {
+                unsigned short i;
+                svalue_t * arg = sp - implicit_context_size;
+                svalue_t * context = sp->u.lambda->context + explicit_context_size;
+
+                for (i = 0; i < implicit_context_size; i++)
                     transfer_svalue_no_free(context+i, arg+i);
 
                 /* Now move the created closure to the new top of the stack */
Index: trunk.inline/CHANGELOG
===================================================================
--- trunk.inline/CHANGELOG	(Revision 2587)
+++ trunk.inline/CHANGELOG	(Arbeitskopie)
@@ -1,6 +1,12 @@
 This file lists all changes made to the game driver in all glory detail.
 See the file HISTORY for a user-oriented summary of all the changes.
 
+??-May-2009 (Gnomi)
+  - (prolang.y, interpret.c) Corrected the handling of implicit context
+    variables in nested inline closures and allowed the initializers
+    of explicit context variables to reference other explicit context
+    variables. (Bug #631)
+
 18-May-2009 (Gnomi)
   - (pkg-xml2.c, arraylist.c, configure) Added support for XML parsing
     using libxml2. (Bug #538, thanks, Bardioc!)
bug631.diff (26,902 bytes)   

Relationships

related to 0000632 resolvedGnomi Initializers shouldn't reference the initialized variable 

Activities

Sorcerer

2009-04-26 02:43

updater   ~0001061

See also issue 0000613, which is marked resolved for 3.3.718.

Sorcerer

2009-04-26 02:45

updater   ~0001062

Oh - I just realized it was filed against 718 but will be resolved only in 719. So this might solve this issue, too, but since I don't know for sure, please have a look at it anyways.

Gnomi

2009-04-26 17:02

manager   ~0001063

I don't know how this was supposed to work. check_for_context_local() in prolang.y seems to assume that we can access variables of an outer context. But at runtime when calling the inner closure this outer context might not exist anymore.

There is a second problem:

    funcall(
        function int() : int i = 1
        {
            return funcall((: return i; :));
        }));

This fails because only locals can be adopted as an implied context variable, not outer context variables.

This is not related to 0000613 which dealt only with lambda closures.

Gnomi

2009-04-27 00:47

manager   ~0001064

And a third problem:

    int i = 1;

    funcall(
        function int()
        {
            int j = 2;
            return funcall((: return i; :));
        }));

This returns 2 (because i is the first local variable in its block, so is j).

Sorcerer

2009-04-27 02:43

updater   ~0001065

One more - which seems to me to be in agreement with the other cases:

  funcall(function int () {
            return funcall(function int () : int i=i {
              return i;
            });
          });

throws an error, while

  funcall(function int () {
            i;
            return funcall(function int () : int i=i {
              return i;
            });
          });
  
works fine (as soon as i is used in the outer inline-closure).

Gnomi

2009-04-29 03:40

manager   ~0001066

The last one is a different problem, similar to the following:

  int x = 1; // global variable

  int fun()
  {
    int x = x;

    return x;
  }

fun() will return 0, because the initializer (= x) references the local variable not the global one. The same happens when initializing the context variable int i = i, the parser initializes the context variable i with itself but at that time the context doesn't exist yet, so an error is thrown.

I'll duplicate this bug for these problems.

Gnomi

2009-04-29 03:56

manager   ~0001067

I attached a patch that fixes these problems (except 0000632).

Gnomi

2009-05-05 14:00

manager   ~0001082

That's a somewhat different problem, which nevertheless i'd like to handle also in this bug:

  int a = 2;

  return funcall(
    function int() : int b = a; int c = b
    {
      return c;
    });

Gnomi

2009-05-09 06:04

manager   ~0001088

I uploaded a new patch that also fixes the last problem (0000631:0001065 and 0000631:0001082) by treating explicit context variables during initialization as real local variables of the outer function and let F_CONTEXT_CLOSURE take the values from there.

fufu

2009-05-10 08:45

reporter   ~0001091

Something is fishy here. Calling the following function results in an infinite loop in inl_transfer_svalue(), caused by an lvalue pointing to itself:

void t(mixed a, mixed b, mixed c)
{
  return funcall(funcall(funcall(
      function :
          mixed i = 1;
          mixed j = function :
              mixed j = function { return i; }
              { return j; };
          { return j; }
  )));
}

fufu

2009-05-10 08:47

reporter   ~0001092

I'm using r2574 + your patch, configured with ./settings/wunderland --disable-use-ipv6 --enable-debug --enable-check-object-ref --enable-check-object-gc-ref

Gnomi

2009-05-10 15:47

manager   ~0001099

I uploaded a modified patch. The old patch calculated the index of the first local variable wrongly when a closure was within the context of another closure.

Gnomi

2009-05-12 09:06

manager   ~0001103

I updated the patch to r2584.

Gnomi

2009-05-19 12:11

manager   ~0001118

Committed as r2588.

Issue History

Date Modified Username Field Change
2009-04-26 02:41 Sorcerer New Issue
2009-04-26 02:43 Sorcerer Note Added: 0001061
2009-04-26 02:45 Sorcerer Note Added: 0001062
2009-04-26 14:49 Gnomi Assigned To => Gnomi
2009-04-26 14:49 Gnomi Status new => confirmed
2009-04-26 14:49 Gnomi Target Version => 3.3.719
2009-04-26 17:02 Gnomi Note Added: 0001063
2009-04-27 00:47 Gnomi Note Added: 0001064
2009-04-27 02:43 Sorcerer Note Added: 0001065
2009-04-29 03:40 Gnomi Note Added: 0001066
2009-04-29 03:49 Gnomi Issue cloned: 0000632
2009-04-29 03:49 Gnomi Relationship added related to 0000632
2009-04-29 03:55 Gnomi File Added: bug631.diff
2009-04-29 03:56 Gnomi Note Added: 0001067
2009-05-05 14:00 Gnomi Note Added: 0001082
2009-05-09 05:22 Gnomi File Deleted: bug631.diff
2009-05-09 05:22 Gnomi File Added: bug631.diff
2009-05-09 05:51 Gnomi File Deleted: bug631.diff
2009-05-09 05:51 Gnomi File Added: bug631.diff
2009-05-09 06:04 Gnomi Note Added: 0001088
2009-05-10 08:45 fufu Note Added: 0001091
2009-05-10 08:47 fufu Note Added: 0001092
2009-05-10 15:45 Gnomi File Deleted: bug631.diff
2009-05-10 15:45 Gnomi File Added: bug631.diff
2009-05-10 15:47 Gnomi Note Added: 0001099
2009-05-11 00:32 Gnomi File Deleted: bug631.diff
2009-05-11 00:32 Gnomi File Added: bug631.diff
2009-05-12 09:04 Gnomi File Deleted: bug631.diff
2009-05-12 09:05 Gnomi File Added: bug631.diff
2009-05-12 09:06 Gnomi Note Added: 0001103
2009-05-19 03:56 Gnomi File Deleted: bug631.diff
2009-05-19 03:57 Gnomi File Added: bug631.diff
2009-05-19 12:11 Gnomi Note Added: 0001118
2009-05-19 12:11 Gnomi Status confirmed => resolved
2009-05-19 12:11 Gnomi Fixed in Version => 3.3.719
2009-05-19 12:11 Gnomi Resolution open => fixed
2010-11-16 09:42 Gnomi Source_changeset_attached => ldmud.git master 79e0cb56
2018-01-29 18:59 Gnomi Source_changeset_attached => ldmud.git master 79e0cb56
2018-01-29 21:57 Gnomi Source_changeset_attached => ldmud.git master 79e0cb56