develooper Front page | perl.cvs.parrot | Postings from January 2009

[svn:parrot] r35109 - in trunk: src t/pmc

From:
cotto
Date:
January 7, 2009 02:41
Subject:
[svn:parrot] r35109 - in trunk: src t/pmc
Message ID:
20090107104104.D6AFECB9F9@x12.develooper.com
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'



nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About