Author: cotto
Date: Wed Jan 7 02:41:03 2009
New Revision: 35109
Modified:
trunk/src/hash.c
trunk/t/pmc/hash.t
Log:
[hash] make clone and freeze iterate by internal order rather than bucket order
this fixes TT #116 and causes no new test failures
Modified: trunk/src/hash.c
==============================================================================
--- trunk/src/hash.c (original)
+++ trunk/src/hash.c Wed Jan 7 02:41:03 2009
@@ -574,35 +574,32 @@
size_t i;
IMAGE_IO * const io = info->image_io;
- for (i = 0; i <= hash->mask; i++) {
- HashBucket *b = hash->bi[i];
+ for (i = 0; i < hash->entries; i++) {
+ HashBucket *b = hash->bs+i;
- while (b) {
- switch (hash->key_type) {
- case Hash_key_type_STRING:
- VTABLE_push_string(interp, io, (STRING *)b->key);
- break;
- case Hash_key_type_int:
- VTABLE_push_integer(interp, io, (INTVAL)b->key);
- break;
- default:
- Parrot_ex_throw_from_c_args(interp, NULL, 1,
- "unimplemented key type");
- break;
- }
- switch (hash->entry_type) {
- case enum_hash_pmc:
- (info->visit_pmc_now)(interp, (PMC *)b->value, info);
- break;
- case enum_hash_int:
- VTABLE_push_integer(interp, io, (INTVAL)b->value);
- break;
- default:
- Parrot_ex_throw_from_c_args(interp, NULL, 1,
- "unimplemented value type");
- break;
- }
- b = b->next;
+ switch (hash->key_type) {
+ case Hash_key_type_STRING:
+ VTABLE_push_string(interp, io, (STRING *)b->key);
+ break;
+ case Hash_key_type_int:
+ VTABLE_push_integer(interp, io, (INTVAL)b->key);
+ break;
+ default:
+ Parrot_ex_throw_from_c_args(interp, NULL, 1,
+ "unimplemented key type");
+ break;
+ }
+ switch (hash->entry_type) {
+ case enum_hash_pmc:
+ (info->visit_pmc_now)(interp, (PMC *)b->value, info);
+ break;
+ case enum_hash_int:
+ VTABLE_push_integer(interp, io, (INTVAL)b->value);
+ break;
+ default:
+ Parrot_ex_throw_from_c_args(interp, NULL, 1,
+ "unimplemented value type");
+ break;
}
}
}
@@ -1362,40 +1359,38 @@
parrot_hash_clone(PARROT_INTERP, ARGIN(const Hash *hash), ARGOUT(Hash *dest))
{
ASSERT_ARGS(parrot_hash_clone)
- UINTVAL i;
+ UINTVAL i, entries;
+ entries = hash->entries;
- for (i = 0; i <= hash->mask; i++) {
- HashBucket *b = hash->bi[i];
- while (b) {
- void * const key = b->key;
- void *valtmp;
-
- switch (hash->entry_type) {
- case enum_type_undef:
- case enum_type_ptr:
- case enum_type_INTVAL:
- valtmp = (void *)b->value;
- break;
+ for (i = 0; i < entries; i++) {
+ HashBucket *b = hash->bs+i;
+ void * const key = b->key;
+ void *valtmp;
+
+ switch (hash->entry_type) {
+ case enum_type_undef:
+ case enum_type_ptr:
+ case enum_type_INTVAL:
+ valtmp = (void *)b->value;
+ break;
- case enum_type_STRING:
- valtmp = (void *)string_copy(interp, (STRING *)b->value);
- break;
+ case enum_type_STRING:
+ valtmp = (void *)string_copy(interp, (STRING *)b->value);
+ break;
- case enum_type_PMC:
- if (PMC_IS_NULL((PMC *)b->value))
- valtmp = (void *)PMCNULL;
- else
- valtmp = (void *)VTABLE_clone(interp, (PMC*)b->value);
- break;
+ case enum_type_PMC:
+ if (PMC_IS_NULL((PMC *)b->value))
+ valtmp = (void *)PMCNULL;
+ else
+ valtmp = (void *)VTABLE_clone(interp, (PMC*)b->value);
+ break;
- default:
- valtmp = NULL; /* avoid warning */
- Parrot_ex_throw_from_c_args(interp, NULL, -1,
- "hash corruption: type = %d\n", hash->entry_type);
- };
- parrot_hash_put(interp, dest, key, valtmp);
- b = b->next;
- }
+ default:
+ valtmp = NULL; /* avoid warning */
+ Parrot_ex_throw_from_c_args(interp, NULL, -1,
+ "hash corruption: type = %d\n", hash->entry_type);
+ };
+ parrot_hash_put(interp, dest, key, valtmp);
}
}
Modified: trunk/t/pmc/hash.t
==============================================================================
--- trunk/t/pmc/hash.t (original)
+++ trunk/t/pmc/hash.t Wed Jan 7 02:41:03 2009
@@ -22,7 +22,7 @@
.include 'test_more.pir'
.include 'except_types.pasm'
- plan(143)
+ plan(145)
initial_hash_tests()
more_than_one_hash()
@@ -43,6 +43,8 @@
getting_values_from_undefined_keys()
setting_and_getting_non_scalar_pmcs()
testing_clone()
+ clone_preserves_order()
+ freeze_thaw_preserves_order()
compound_keys()
getting_pmcs_from_compound_keys()
getting_pmcs_from_string_int_compound_keys()
@@ -584,6 +586,163 @@
# print "ok 6\n"
.end
+# TT #116
+# This test failure depends on the value if the hash seed, which is randomized.
+# To try to ensure that the test fails reliably if there's a regression, it's
+# run 3 times with different hash keys.
+.sub clone_preserves_order
+ .local pmc h, cloned
+ .local string s1, s2
+ .local int all_ok
+
+ all_ok = 1
+ h = new 'Hash'
+
+ h['a'] = 1
+ h['b'] = 2
+ h['c'] = 3
+ h['d'] = 4
+ h['e'] = 5
+ h['f'] = 6
+ h['g'] = 7
+ h['h'] = 8
+ h['i'] = 9
+ h['j'] = 10
+ h['k'] = 11
+ h['l'] = 12
+
+ cloned = clone h
+ #If the bug is present, the order of elements in the get_repr string will
+ #be different.
+ s1 = get_repr h
+ s2 = get_repr cloned
+
+ if s1 != s2 goto fail
+
+ h = new 'Hash'
+
+ h['aa'] = 1
+ h['bb'] = 2
+ h['cc'] = 3
+ h['dd'] = 4
+ h['ee'] = 5
+ h['ff'] = 6
+ h['gg'] = 7
+ h['hh'] = 8
+ h['ii'] = 9
+ h['jj'] = 10
+ h['kk'] = 11
+ h['ll'] = 12
+
+ cloned = clone h
+ s1 = get_repr h
+ s2 = get_repr cloned
+ if s1 != s2 goto fail
+
+ h = new 'Hash'
+
+ h['one'] = 1
+ h['two'] = 2
+ h['three'] = 3
+ h['four'] = 4
+ h['five'] = 5
+ h['six'] = 6
+ h['seven'] = 7
+ h['eight'] = 8
+ h['nine'] = 9
+ h['ten'] = 10
+ h['eleven'] = 11
+ h['twelve'] = 12
+
+ cloned = clone h
+ s1 = get_repr h
+ s2 = get_repr cloned
+ if s1 != s2 goto fail
+
+ goto end
+fail:
+ all_ok = 0
+end:
+ ok(all_ok, "clone preserves hash internal order")
+.end
+
+.sub freeze_thaw_preserves_order
+ .local pmc h, cloned
+ .local string s1, s2
+ .local int all_ok
+
+ all_ok = 1
+ h = new 'Hash'
+
+ h['a'] = 1
+ h['b'] = 2
+ h['c'] = 3
+ h['d'] = 4
+ h['e'] = 5
+ h['f'] = 6
+ h['g'] = 7
+ h['h'] = 8
+ h['i'] = 9
+ h['j'] = 10
+ h['k'] = 11
+ h['l'] = 12
+
+ $S0 = freeze h
+ cloned = thaw $S0
+ s1 = get_repr h
+ s2 = get_repr cloned
+
+ if s1 != s2 goto fail
+
+ h = new 'Hash'
+
+ h['aa'] = 1
+ h['bb'] = 2
+ h['cc'] = 3
+ h['dd'] = 4
+ h['ee'] = 5
+ h['ff'] = 6
+ h['gg'] = 7
+ h['hh'] = 8
+ h['ii'] = 9
+ h['jj'] = 10
+ h['kk'] = 11
+ h['ll'] = 12
+
+ $S0 = freeze h
+ cloned = thaw $S0
+ s1 = get_repr h
+ s2 = get_repr cloned
+ if s1 != s2 goto fail
+
+ h = new 'Hash'
+
+ h['one'] = 1
+ h['two'] = 2
+ h['three'] = 3
+ h['four'] = 4
+ h['five'] = 5
+ h['six'] = 6
+ h['seven'] = 7
+ h['eight'] = 8
+ h['nine'] = 9
+ h['ten'] = 10
+ h['eleven'] = 11
+ h['twelve'] = 12
+
+ $S0 = freeze h
+ cloned = thaw $S0
+ s1 = get_repr h
+ s2 = get_repr cloned
+ if s1 != s2 goto fail
+
+ goto end
+fail:
+ all_ok = 0
+end:
+ ok(all_ok, "freeze/thaw preserves hash internal order")
+.end
+
.sub compound_keys
new $P0, 'Hash'
new $P1, 'Hash'
