Hello.
I didn't realise how much code depends on Hash keys order. Shame on me.
Almost all this code was fixed but no all.
Last bit is handling "attributes" in Class.pmc which causes 2 failures
in Rakudo. Problems are in this snippet of code:
class AttrInitTest {
has $.a = 1;
has $.b = 2;
has $.c = $.a + $.b;
};
say AttrInitTest.new.c;
Because hash keys are in some random order now (which is correct from my
point of view) attribute $!c some time initialised before other
attributes. I can see 3 ways of solving this issue.
1. Unmerge keys_revamp branch, but big warning in release notes that
Hash keys order will be random, merge it after release and implement
dependency tracking of attributes in Rakudo. "warning", not
"deprecation" because current behaviour isn't specified.
2. "Fix" Class PMC to use OrderedHash for "attributes" before release.
This is actually very big change in current timeframe.
3. Polish and apply attached patch which restore ordering of Hash keys.
Revert this patch after release. Put big warning message from item 1.
Patch is wrong in many ways because it restores "original" behaviour of
iterating over hashes. E.g.
a) it doesn't handle INTVAL keys properly.
b) it will hide original problem with attributes. May be not applicable
to Rakudo, but for other languages which allows to remove "attribute"
in runtime we can't guarantee order of them.
c) we continue to support bad habit of relying on Hash keys order.
But for purpose of release it's probably best (fsvo) solution.
--
Bacek
diff --git a/src/pmc/hashiterator.pmc b/src/pmc/hashiterator.pmc
index a811faa..3ab5e6b 100644
--- a/src/pmc/hashiterator.pmc
+++ b/src/pmc/hashiterator.pmc
@@ -51,6 +51,31 @@ Generic iterator for traversing Hash.
/*
+Advance to start. On chopping block.
+
+*/
+static HashBucket*
+advance_to_nonempty(PARROT_INTERP, PMC *self) {
+ Parrot_HashIterator_attributes * const attrs = PARROT_HASHITERATOR(self);
+ HashBucket *bucket;
+ const INTVAL size = (INTVAL)N_BUCKETS(attrs->parrot_hash->mask + 1);
+
+ if (!attrs->elements)
+ return bucket;
+
+ /* Get bucket and move to next bucket */
+ for (bucket = attrs->parrot_hash->bs + attrs->pos;
+ attrs->pos < size;
+ ++attrs->pos, ++bucket) {
+ if (bucket->key) {
+ break;
+ }
+ }
+ return bucket;
+}
+
+/*
+
Advance to next position. Return found (if any) HashBucket.
*/
@@ -105,14 +130,10 @@ Defaults iteration mode to iterate from start.
attrs->pos = 0;
/* Will be decreased on initial advance_to_next */
/* XXX Do we really need to support this use-case ? */
- attrs->elements = attrs->parrot_hash->entries + 1;
+ attrs->elements = attrs->parrot_hash->entries;
PMC_data(SELF) = attrs;
PObj_custom_mark_destroy_SETALL(SELF);
-
- /* Initial state of iterator is "before start" */
- /* So, advance to first element */
- advance_to_next(INTERP, SELF);
}
/*
@@ -172,7 +193,8 @@ Marks the hash as live.
/* Restart iterator */
attrs->bucket = 0;
attrs->pos = 0;
- advance_to_next(INTERP, SELF);
+ attrs->elements = attrs->parrot_hash->entries;
+ advance_to_nonempty(INTERP, SELF);
return;
}
@@ -203,7 +225,7 @@ Returns true if there is more elements to iterate over.
*/
VTABLE INTVAL get_bool() {
- return PARROT_HASHITERATOR(SELF)->bucket != 0;
+ return PARROT_HASHITERATOR(SELF)->elements != 0;
}
/*
@@ -239,19 +261,23 @@ the next one.
Parrot_HashIterator_attributes * const attrs =
PARROT_HASHITERATOR(SELF);
- PMC *ret;
+ PMC *ret;
+ HashBucket *bucket;
+ const INTVAL size = (INTVAL)N_BUCKETS(attrs->parrot_hash->mask + 1);
- if (!attrs->bucket)
+ if (!attrs->elements)
Parrot_ex_throw_from_c_args(INTERP, NULL, EXCEPTION_OUT_OF_BOUNDS,
"StopIteration");
+ bucket = advance_to_nonempty(INTERP, SELF);
+ PARROT_ASSERT(bucket);
+ attrs->pos++;
+ attrs->elements--;
ret = pmc_new(INTERP, enum_class_HashIteratorKey);
+
/* Poke directly into HIK. We don't want to create any kind of public API for this */
PARROT_HASHITERATORKEY(ret)->parrot_hash = attrs->parrot_hash;
- PARROT_HASHITERATORKEY(ret)->bucket = attrs->bucket;
-
- /* Move to next bucket */
- advance_to_next(INTERP, SELF);
+ PARROT_HASHITERATORKEY(ret)->bucket = bucket;
return ret;
}
@@ -262,6 +288,8 @@ the next one.
VTABLE STRING* shift_string() {
PMC * const key = SELF.shift_pmc();
+ if (PMC_IS_NULL(key))
+ return NULL;
return VTABLE_get_string(INTERP, key);
}
_______________________________________________
http://lists.parrot.org/mailman/listinfo/parrot-dev
