From f139188d965a607a76b33d5229a5778ac87507f8 Mon Sep 17 00:00:00 2001 From: Martin Stjernholm <mast@lysator.liu.se> Date: Fri, 5 Sep 2003 17:19:20 +0200 Subject: [PATCH] Changed foreach to step the iterator after the loop instead of before it, so that the iterator points to the current element instead of the next one. Also some cleanup in iterators.cmod and a bit better error checking in get_iterator. Rev: src/builtin_functions.h:1.27 Rev: src/docode.c:1.166 Rev: src/interpret_functions.h:1.156 Rev: src/iterators.cmod:1.47 Rev: src/testsuite.in:1.673 --- src/builtin_functions.h | 7 +- src/docode.c | 18 +- src/interpret_functions.h | 27 ++- src/iterators.cmod | 346 ++++++++++++++++++++++---------------- src/testsuite.in | 9 +- 5 files changed, 247 insertions(+), 160 deletions(-) diff --git a/src/builtin_functions.h b/src/builtin_functions.h index 8fc30a5cfe..0764d3e0f8 100644 --- a/src/builtin_functions.h +++ b/src/builtin_functions.h @@ -2,7 +2,7 @@ || This file is part of Pike. For copyright information see COPYRIGHT. || Pike is distributed under GPL, LGPL and MPL. See the file COPYING || for more information. -|| $Id: builtin_functions.h,v 1.26 2003/07/21 23:34:01 mast Exp $ +|| $Id: builtin_functions.h,v 1.27 2003/09/05 15:19:20 mast Exp $ */ #ifndef BUILTIN_EFUNS_H @@ -107,9 +107,12 @@ PMOD_EXPORT void f_transpose(INT32 args); PMOD_EXPORT void f__reset_dmalloc(INT32 args); PMOD_EXPORT void f__locate_references(INT32 args); PMOD_EXPORT void f_map_array(INT32 args); -PMOD_EXPORT void f_get_iterator(INT32 args); void init_builtin_efuns(void); +/* From iterators.cmod. */ +PMOD_EXPORT void f_get_iterator(INT32 args); +int foreach_iterate(struct object *o, int do_step); + /* From builtin.cmod. */ PMOD_EXPORT void f_column(INT32 args); PMOD_EXPORT void f_trace(INT32 args); diff --git a/src/docode.c b/src/docode.c index 0bb3dfae84..86d5fb75cf 100644 --- a/src/docode.c +++ b/src/docode.c @@ -2,11 +2,11 @@ || This file is part of Pike. For copyright information see COPYRIGHT. || Pike is distributed under GPL, LGPL and MPL. See the file COPYING || for more information. -|| $Id: docode.c,v 1.165 2003/08/03 00:52:42 mast Exp $ +|| $Id: docode.c,v 1.166 2003/09/05 15:19:20 mast Exp $ */ #include "global.h" -RCSID("$Id: docode.c,v 1.165 2003/08/03 00:52:42 mast Exp $"); +RCSID("$Id: docode.c,v 1.166 2003/09/05 15:19:20 mast Exp $"); #include "las.h" #include "program.h" #include "pike_types.h" @@ -1254,15 +1254,18 @@ static int do_docode2(node *n, int flags) current_switch.jumptable=0; current_label->break_label=alloc_label(); current_label->continue_label=alloc_label(); - - tmp3=do_branch(-1); + + /* Doubt it's necessary to use a label separate from + * current_label->break_label, but I'm playing safe. /mast */ + tmp3 = alloc_label(); + do_jump(F_FOREACH_START, DO_NOT_WARN((INT32) tmp3)); tmp1=ins_label(-1); DO_CODE_BLOCK(CDR(n)); ins_label(current_label->continue_label); - low_insert_label( DO_NOT_WARN((INT32)tmp3)); - do_jump(F_NEW_FOREACH, DO_NOT_WARN((INT32)tmp1)); + do_jump(F_FOREACH_LOOP, DO_NOT_WARN((INT32)tmp1)); ins_label(current_label->break_label); - + low_insert_label( DO_NOT_WARN((INT32)tmp3)); + current_switch.jumptable = prev_switch_jumptable; POP_STATEMENT_LABEL; POP_AND_DO_CLEANUP; @@ -1279,6 +1282,7 @@ static int do_docode2(node *n, int flags) node **a2=my_get_arg(&_CAR(arr),1); if(a1 && a2 && a2[0]->token==F_CONSTANT && a2[0]->u.sval.type==T_INT && + /* FIXME: The following can never be true! */ a2[0]->u.sval.type==0x7fffffff && a1[0]->type == int_type_string) { diff --git a/src/interpret_functions.h b/src/interpret_functions.h index 4ad878f594..4c7f193a96 100644 --- a/src/interpret_functions.h +++ b/src/interpret_functions.h @@ -2,7 +2,7 @@ || This file is part of Pike. For copyright information see COPYRIGHT. || Pike is distributed under GPL, LGPL and MPL. See the file COPYING || for more information. -|| $Id: interpret_functions.h,v 1.155 2003/08/13 15:58:04 grubba Exp $ +|| $Id: interpret_functions.h,v 1.156 2003/09/05 15:19:20 mast Exp $ */ /* @@ -1407,16 +1407,29 @@ OPCODE0(F_MAKE_ITERATOR, "get_iterator", 0, { f_get_iterator(1); }); -OPCODE0_BRANCH(F_NEW_FOREACH, "foreach++", 0, { /* iterator, lvalue, lvalue */ - extern int foreach_iterate(struct object *o); +/* Stack is: iterator, index lvalue, value lvalue. */ +OPCODE0_BRANCH (F_FOREACH_START, "foreach start", 0, { + DO_IF_DEBUG ( + if(Pike_sp[-5].type != PIKE_T_OBJECT) + Pike_fatal ("Iterator gone from stack.\n"); + ); + if (foreach_iterate (Pike_sp[-5].u.object, 0)) + DONT_BRANCH(); + else { + DO_BRANCH(); + } +}); - if(Pike_sp[-5].type != PIKE_T_OBJECT) - PIKE_ERROR("foreach", "Bad argument 1.\n", Pike_sp-3, 1); - if(foreach_iterate(Pike_sp[-5].u.object)) +/* Stack is: iterator, index lvalue, value lvalue. */ +OPCODE0_BRANCH(F_FOREACH_LOOP, "foreach loop", 0, { + DO_IF_DEBUG ( + if(Pike_sp[-5].type != PIKE_T_OBJECT) + Pike_fatal ("Iterator gone from stack.\n"); + ); + if(foreach_iterate(Pike_sp[-5].u.object, 1)) { DO_BRANCH(); }else{ - /* write_to_stderr("foreach\n", 8); */ DONT_BRANCH(); } }); diff --git a/src/iterators.cmod b/src/iterators.cmod index 1160e431b3..5810e28b0d 100644 --- a/src/iterators.cmod +++ b/src/iterators.cmod @@ -2,11 +2,11 @@ || This file is part of Pike. For copyright information see COPYRIGHT. || Pike is distributed under GPL, LGPL and MPL. See the file COPYING || for more information. -|| $Id: iterators.cmod,v 1.46 2003/09/04 17:03:04 grubba Exp $ +|| $Id: iterators.cmod,v 1.47 2003/09/05 15:19:20 mast Exp $ */ #include "global.h" -RCSID("$Id: iterators.cmod,v 1.46 2003/09/04 17:03:04 grubba Exp $"); +RCSID("$Id: iterators.cmod,v 1.47 2003/09/05 15:19:20 mast Exp $"); #include "main.h" #include "object.h" #include "mapping.h" @@ -269,8 +269,7 @@ PIKECLASS mapping_iterator push_svalue(& THIS->current->val); else { - push_int(0); - Pike_sp[-1].subtype=NUMBER_UNDEFINED; + push_undefined(); } } @@ -280,8 +279,7 @@ PIKECLASS mapping_iterator push_svalue(& THIS->current->ind); else { - push_int(0); - Pike_sp[-1].subtype=NUMBER_UNDEFINED; + push_undefined(); } } @@ -530,7 +528,6 @@ PIKECLASS multiset_iterator { CVAR struct multiset *l; CVAR int lock_index; - CVAR int prestepped; /* Ugly special flag to work correctly in foreach_iterate. */ CVAR ptrdiff_t nodepos; PIKEFUN int value() @@ -690,7 +687,6 @@ PIKECLASS multiset_iterator add_ref (THIS->l = l); THIS->lock_index = 0; THIS->nodepos = multiset_first (THIS->l); - THIS->prestepped = (THIS->nodepos >= 0); } INIT @@ -825,8 +821,7 @@ PIKECLASS string_iterator { if(!THIS->s || THIS->pos < 0 || THIS->pos >= THIS->s->len) { - push_int(0); - Pike_sp[-1].subtype=NUMBER_UNDEFINED; + push_undefined(); }else{ RETURN index_shared_string(THIS->s, THIS->pos); } @@ -836,8 +831,7 @@ PIKECLASS string_iterator { if(!THIS->s || THIS->pos < 0 || THIS->pos >= THIS->s->len) { - push_int(0); - Pike_sp[-1].subtype=NUMBER_UNDEFINED; + push_undefined(); }else{ RETURN THIS->pos; } @@ -1026,16 +1020,14 @@ PIKECLASS file_line_iterator if (THIS->current) { ref_push_string(THIS->current); } else { - push_int(0); - Pike_sp[-1].subtype = NUMBER_UNDEFINED; + push_undefined(); } } PIKEFUN int index() { if (!THIS->current) { - push_int(0); - Pike_sp[-1].subtype = NUMBER_UNDEFINED; + push_undefined(); }else{ RETURN THIS->index; } @@ -1386,16 +1378,14 @@ PIKECLASS string_split_iterator if (THIS->current) { ref_push_string(THIS->current); } else { - push_int(0); - Pike_sp[-1].subtype = NUMBER_UNDEFINED; + push_undefined(); } } PIKEFUN int index() { if (!THIS->current) { - push_int(0); - Pike_sp[-1].subtype = NUMBER_UNDEFINED; + push_undefined(); }else{ RETURN THIS->index; } @@ -1842,19 +1832,29 @@ PIKEFUN object(Iterator) get_iterator(object|array|mapping|multiset|string data) return; - case PIKE_T_OBJECT: + case PIKE_T_OBJECT: { + int fun; + if(!data->u.object->prog) SIMPLE_ARG_ERROR ("get_iterator", 1, "Got a destructed object.\n"); - if(FIND_LFUN(data->u.object->prog, LFUN__GET_ITERATOR) != -1) + fun = FIND_LFUN(data->u.object->prog, LFUN__GET_ITERATOR); + if (fun != -1) { - apply_lfun(data->u.object, LFUN__GET_ITERATOR, 0); + apply_low (data->u.object, fun, 0); + if (sp[-1].type != T_OBJECT) { + /* FIXME: Ought to include what we got in the error message. */ + pop_stack(); + SIMPLE_ARG_ERROR ("get_iterator", 1, + "_get_iterator() didn't return an object.\n"); + } stack_pop_keep_top(); return; } /* Assume it already is an iterator... */ return; + } default: SIMPLE_ARG_TYPE_ERROR("get_iterator", 1, "multiset|array|string|mapping|object"); @@ -1862,105 +1862,117 @@ PIKEFUN object(Iterator) get_iterator(object|array|mapping|multiset|string data) } /* sp[-4] = index; sp[-2] = value */ -int foreach_iterate(struct object *o) +int foreach_iterate(struct object *o, int do_step) { + struct program *prog = o->prog; int fun; - if(!o->prog) + if(!prog) Pike_error("foreach on destructed iterator.\n"); - if(o->prog->flags & PROGRAM_HAS_C_METHODS) + + if(prog->flags & PROGRAM_HAS_C_METHODS) { - if(o->prog == mapping_iterator_program) + if(prog == mapping_iterator_program) { struct mapping_iterator_struct *i=OBJ2_MAPPING_ITERATOR(o); - if(i->current) - { - if(Pike_sp[-4].type != T_INT) - assign_lvalue(Pike_sp-4, & i->current->ind); - - if(Pike_sp[-2].type != T_INT) - assign_lvalue(Pike_sp-2, & i->current->val); - mi_step(i); - return 1; - }else{ - return 0; + if (!i->current) return 0; + if (do_step) { + mi_step (i); + if (!i->current) return 0; } - } else if(o->prog == string_split_iterator_program) + + if(Pike_sp[-4].type != T_INT) + assign_lvalue(Pike_sp-4, & i->current->ind); + + if(Pike_sp[-2].type != T_INT) + assign_lvalue(Pike_sp-2, & i->current->val); + + return 1; + } + + else if(prog == string_split_iterator_program) { struct string_split_iterator_struct *i=OBJ2_STRING_SPLIT_ITERATOR(o); - if(i->current) - { - if(Pike_sp[-4].type != T_INT) - { - /* Black Magic... */ - push_int(i->index); - Pike_sp--; - assign_lvalue(Pike_sp-4, Pike_sp); - } - if(Pike_sp[-2].type != T_INT) - { - /* Black Magic... */ - push_string(i->current); - dmalloc_touch_svalue(Pike_sp-1); - Pike_sp--; - assign_lvalue(Pike_sp-2, Pike_sp); - } + if (!i->current) return 0; + if (do_step) { + find_next (i); + if (!i->current) return 0; + } - find_next(i); - return 1; - }else{ - return 0; + if(Pike_sp[-4].type != T_INT) + { + /* Black Magic... */ + push_int(i->index); + Pike_sp--; + assign_lvalue(Pike_sp-4, Pike_sp); } - } else if(o->prog == file_line_iterator_program) + + if(Pike_sp[-2].type != T_INT) + { + /* Black Magic... */ + push_string(i->current); + dmalloc_touch_svalue(Pike_sp-1); + Pike_sp--; + assign_lvalue(Pike_sp-2, Pike_sp); + } + + return 1; + } + + else if(prog == file_line_iterator_program) { struct file_line_iterator_struct *i=OBJ2_FILE_LINE_ITERATOR(o); - if(i->current) - { - if(Pike_sp[-4].type != T_INT) - { - /* Black Magic... */ - push_int(i->index); - Pike_sp--; - assign_lvalue(Pike_sp-4, Pike_sp); - } - if(Pike_sp[-2].type != T_INT) - { - /* Black Magic... */ - push_string(i->current); - dmalloc_touch_svalue(Pike_sp-1); - Pike_sp--; - assign_lvalue(Pike_sp-2, Pike_sp); - } + if (!i->current) return 0; + if (do_step) { + fl_find_next (i); + if (!i->current) return 0; + } - fl_find_next(i); - return 1; - }else{ - return 0; + if(Pike_sp[-4].type != T_INT) + { + /* Black Magic... */ + push_int(i->index); + Pike_sp--; + assign_lvalue(Pike_sp-4, Pike_sp); } - } else if(o->prog == array_iterator_program) + + if(Pike_sp[-2].type != T_INT) + { + /* Black Magic... */ + push_string(i->current); + dmalloc_touch_svalue(Pike_sp-1); + Pike_sp--; + assign_lvalue(Pike_sp-2, Pike_sp); + } + + return 1; + } + + else if(prog == array_iterator_program) { struct array_iterator_struct *i=OBJ2_ARRAY_ITERATOR(o); - if(i->pos >= 0 && i->pos < i->a->size) - { - if(Pike_sp[-4].type != T_INT) - { - push_int(i->pos); - assign_lvalue(Pike_sp-5, Pike_sp-1); - pop_stack(); - } - if(Pike_sp[-2].type != T_INT) - assign_lvalue(Pike_sp-2, i->a->item + i->pos); + if (i->pos < 0 || i->pos >= i->a->size) return 0; + if (do_step) + if (++i->pos == i->a->size) return 0; - i->pos++; - return 1; - }else{ - return 0; + if(Pike_sp[-4].type != T_INT) + { + push_int(i->pos); + assign_lvalue(Pike_sp-5, Pike_sp-1); + pop_stack(); } - } else if(o->prog == multiset_iterator_program) + + if(Pike_sp[-2].type != T_INT) + assign_lvalue(Pike_sp-2, i->a->item + i->pos); + + return 1; + } + + else if(prog == multiset_iterator_program) { struct multiset_iterator_struct *i=OBJ2_MULTISET_ITERATOR(o); @@ -1968,13 +1980,7 @@ int foreach_iterate(struct object *o) struct svalue ind; if (i->nodepos < 0) return 0; - - /* Step before getting the values, to behave better if the - * multiset is changed during the iteration. Since the iterator - * got to be initialized to the first element in its create(), - * we need an ugly special flag to get it right. */ - if ((i->prestepped ? (i->prestepped = 0) : 1) || - msnode_is_deleted (i->l, i->nodepos)) { + if (do_step) { i->nodepos = multiset_next (i->l, i->nodepos); if (i->nodepos < 0) { sub_msnode_ref (i->l); @@ -1990,53 +1996,110 @@ int foreach_iterate(struct object *o) return 1; #else /* PIKE_NEW_MULTISETS */ - if(i->pos >= 0 && i->pos < i->a->size) - { - if(Pike_sp[-4].type != T_INT) - assign_lvalue(Pike_sp-4, i->a->item + i->pos); + if (i->pos < 0 || i->pos >= i->a->size) return 0; + if (do_step) + if (++i->pos == i->a->size) return 0; - if(Pike_sp[-2].type != T_INT) - { - push_int(1); - assign_lvalue(Pike_sp-3, Pike_sp-1); - pop_stack(); - } + if(Pike_sp[-4].type != T_INT) + assign_lvalue(Pike_sp-4, i->a->item + i->pos); - i->pos++; - return 1; - }else{ - return 0; + if(Pike_sp[-2].type != T_INT) + { + push_int(1); + assign_lvalue(Pike_sp-3, Pike_sp-1); + pop_stack(); } + + return 1; + #endif /* PIKE_NEW_MULTISETS */ + } - } else if(o->prog == string_iterator_program) + else if(prog == string_iterator_program) { struct string_iterator_struct *i=OBJ2_STRING_ITERATOR(o); - if(i->pos >= 0 && i->pos < i->s->len) - { - if(Pike_sp[-4].type != T_INT) - { - push_int(i->pos); - assign_lvalue(Pike_sp-5, Pike_sp-1); - pop_stack(); - } - if(Pike_sp[-2].type != T_INT) - { - push_int(index_shared_string(i->s, i->pos)); - assign_lvalue(Pike_sp-3, Pike_sp-1); - pop_stack(); - } + if (i->pos < 0 || i->pos >= i->s->len) return 0; + if (do_step) + if (++i->pos == i->s->len) return 0; - i->pos++; - return 1; - }else{ - return 0; + if(Pike_sp[-4].type != T_INT) + { + push_int(i->pos); + assign_lvalue(Pike_sp-5, Pike_sp-1); + pop_stack(); + } + + if(Pike_sp[-2].type != T_INT) + { + push_int(index_shared_string(i->s, i->pos)); + assign_lvalue(Pike_sp-3, Pike_sp-1); + pop_stack(); } + + return 1; } } /* Generic iteration */ + + if (do_step) { + fun = FIND_LFUN (prog, LFUN_ADD_EQ); + if (fun < 0) Pike_error ("Iterator object lacks `+=.\n"); + push_int(1); + apply_low(o, fun, 1); + pop_stack(); + } + +#if 0 + /* We should be able to save calls to `! this way, but there are + * iterators where index() and value() don't return UNDEFINED as + * stipulated by the interface. */ + + fun = -1; + + if(Pike_sp[-4].type != T_INT) + { + fun = find_identifier ("index", prog); + if (fun < 0) Pike_error ("Iterator object lacks index().\n"); + apply_low(o, fun, 0); + if (IS_UNDEFINED (Pike_sp - 1)) { + Pike_sp--; + return 0; + } + assign_lvalue(Pike_sp-5,Pike_sp-1); + pop_stack(); + } + + if(Pike_sp[-2].type != T_INT) + { + fun = find_identifier ("value", prog); + if (fun < 0) Pike_error ("Iterator object lacks value().\n"); + apply_low(o, fun, 0); + if (IS_UNDEFINED (Pike_sp - 1)) { + Pike_sp--; + return 0; + } + assign_lvalue(Pike_sp-3,Pike_sp-1); + pop_stack(); + } + + if (fun >= 0) + /* index() and/or value() has returned a value so we know we can + * iterate without calling `!. */ + return 1; + else { + int res; + fun = FIND_LFUN (prog, LFUN_NOT); + if (fun < 0) Pike_error ("Iterator object lacks `!.\n"); + apply_low(o, fun, 0); + res = UNSAFE_IS_ZERO(Pike_sp-1); + pop_stack(); + return res; + } + +#else + fun = FIND_LFUN (o->prog, LFUN_NOT); if (fun < 0) Pike_error ("Iterator object lacks `!.\n"); apply_low(o, fun, 0); @@ -2062,16 +2125,13 @@ int foreach_iterate(struct object *o) pop_stack(); } - push_int(1); - fun = FIND_LFUN (o->prog, LFUN_ADD_EQ); - if (fun < 0) Pike_error ("Iterator object lacks `+=.\n"); - apply_lfun(o,LFUN_ADD_EQ,1); - pop_stack(); return 1; }else{ pop_stack(); return 0; } + +#endif } diff --git a/src/testsuite.in b/src/testsuite.in index 07d0bb29af..f89d33e4fe 100644 --- a/src/testsuite.in +++ b/src/testsuite.in @@ -1,4 +1,4 @@ -test_true([["$Id: testsuite.in,v 1.672 2003/08/26 18:33:20 mast Exp $"]]); +test_true([["$Id: testsuite.in,v 1.673 2003/09/05 15:19:20 mast Exp $"]]); // This triggered a bug only if run sufficiently early. test_compile_any([[#pike 7.2]]) @@ -6840,6 +6840,13 @@ test_eval_error([[ return i; ]]) +test_any([[ + String.Iterator iter = String.Iterator ("foo"); + foreach (iter; int idx;) + if (idx != iter->index()) return 1; + return 0; +]], 0) + // do-while test_any(int e;string t=""; e=0; do{ t+=e; }while(++e<6); return t,"012345";) -- GitLab