* Weak Vectors Patch @ 2010-04-01 17:18 Clinton Ebadi 2010-04-05 17:03 ` Ludovic Courtès 0 siblings, 1 reply; 6+ messages in thread From: Clinton Ebadi @ 2010-04-01 17:18 UTC (permalink / raw) To: bug-guile [-- Attachment #1: Type: text/plain, Size: 647 bytes --] Attached is a patch that should improve weak vectors and prevent `vector-ref' from segfaulting on them after a GC. The root of the problem was that weak vectors were being allocated as containing no pointers *and* disappearing links were not being registered for initial elements. If you, however, created an empty weak vector and then `vector-set!' the elements things were fine. I'm not sure that weak vectors should have their allocated memory marked as containing no pointers, but it seems to work so I left that alone in case there was some magic going on that I didn't quite grok. CC any replies to me; I'm not subscribed to bug-guile. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Fix-weak-vectors.patch --] [-- Type: text/x-diff, Size: 4810 bytes --] From a20475f1f9643093ea96118a71826623ecf15220 Mon Sep 17 00:00:00 2001 From: Clinton Ebadi <clinton@unknownlamer.org> Date: Thu, 1 Apr 2010 13:11:10 -0400 Subject: [PATCH] Fix weak vectors * libguile/vectors.c (scm_i_make_weak_vector, scm_i_make_weak_vector_from_list): Register elements as disappearing links. * libguile/vectors.c (do_weak_vector_ref): New function to access an element of a weak vector while the allocator lock is held. * libguile/vectors.c (scm_c_vector_ref): Use `do_weak_vector_ref' when accessing weak vectors. * libguile/vectors.c (scm_c_vector_set_x): Deregister link before registering disappearing link to new object. Might not be needed. * libguile/vectors.c (scm_c_vector_ref, scm_c_vector_set_x): Only check for NULLified pointers in weak vectors and not in weak hash vectors (the elements of a WHVEC are not weak). --- libguile/vectors.c | 66 ++++++++++++++++++++++++++++++++++++++------------- 1 files changed, 49 insertions(+), 17 deletions(-) diff --git a/libguile/vectors.c b/libguile/vectors.c index 6ac5acb..f72c927 100644 --- a/libguile/vectors.c +++ b/libguile/vectors.c @@ -203,22 +203,46 @@ scm_vector_ref (SCM v, SCM k) } #undef FUNC_NAME +struct t_weak_vector_ref_args +{ + SCM vector; + size_t k; +}; + +/* Accessing weak pointers *must* be done with the allocator lock held */ +static void* +do_weak_vector_ref (void* data) +{ + struct t_weak_vector_ref_args *args = (struct t_weak_vector_ref_args *) data; + register SCM elt = (SCM_I_VECTOR_ELTS(args->vector))[args->k]; + + if (elt == SCM_PACK (NULL)) + /* ELT was a weak pointer and got nullified by the GC. */ + return SCM_BOOL_F; + + return elt; +} + SCM scm_c_vector_ref (SCM v, size_t k) { if (SCM_I_IS_VECTOR (v)) { - register SCM elt; - if (k >= SCM_I_VECTOR_LENGTH (v)) scm_out_of_range (NULL, scm_from_size_t (k)); - elt = (SCM_I_VECTOR_ELTS(v))[k]; - if ((elt == SCM_PACK (NULL)) && SCM_I_WVECTP (v)) - /* ELT was a weak pointer and got nullified by the GC. */ - return SCM_BOOL_F; + if (SCM_I_WVECTP (v) && !SCM_IS_WHVEC (v)) + { + struct t_weak_vector_ref_args args; + args.vector = v; + args.k = k; - return elt; + return GC_call_with_alloc_lock (do_weak_vector_ref, &args); + } + else + { + return (SCM_I_VECTOR_ELTS(v))[k]; + } } else if (SCM_I_ARRAYP (v) && SCM_I_ARRAY_NDIM (v) == 1) { @@ -226,18 +250,20 @@ scm_c_vector_ref (SCM v, size_t k) SCM vv = SCM_I_ARRAY_V (v); if (SCM_I_IS_VECTOR (vv)) { - register SCM elt; - if (k >= dim->ubnd - dim->lbnd + 1) scm_out_of_range (NULL, scm_from_size_t (k)); k = SCM_I_ARRAY_BASE (v) + k*dim->inc; - elt = (SCM_I_VECTOR_ELTS (vv))[k]; - if ((elt == SCM_PACK (NULL)) && (SCM_I_WVECTP (vv))) - /* ELT was a weak pointer and got nullified by the GC. */ - return SCM_BOOL_F; + if (SCM_I_WVECTP (vv) && !SCM_IS_WHVEC (vv)) + { + struct t_weak_vector_ref_args args; + args.vector = vv; + args.k = k; - return elt; + return GC_call_with_alloc_lock (do_weak_vector_ref, &args); + } + + return (SCM_I_VECTOR_ELTS (vv))[k]; } scm_wrong_type_arg_msg (NULL, 0, v, "non-uniform vector"); } @@ -275,10 +301,11 @@ scm_c_vector_set_x (SCM v, size_t k, SCM obj) if (k >= SCM_I_VECTOR_LENGTH (v)) scm_out_of_range (NULL, scm_from_size_t (k)); (SCM_I_VECTOR_WELTS(v))[k] = obj; - if (SCM_I_WVECTP (v)) + if (SCM_I_WVECTP (v) && !SCM_IS_WHVEC (v)) { /* Make it a weak pointer. */ GC_PTR link = (GC_PTR) & ((SCM_I_VECTOR_WELTS (v))[k]); + GC_unregister_disappearing_link (link); SCM_I_REGISTER_DISAPPEARING_LINK (link, obj); } } @@ -293,10 +320,11 @@ scm_c_vector_set_x (SCM v, size_t k, SCM obj) k = SCM_I_ARRAY_BASE (v) + k*dim->inc; (SCM_I_VECTOR_WELTS (vv))[k] = obj; - if (SCM_I_WVECTP (vv)) + if (SCM_I_WVECTP (vv) && !SCM_IS_WHVEC (vv)) { /* Make it a weak pointer. */ GC_PTR link = (GC_PTR) & ((SCM_I_VECTOR_WELTS (vv))[k]); + GC_unregister_disappearing_link (link); SCM_I_REGISTER_DISAPPEARING_LINK (link, obj); } } @@ -421,7 +449,10 @@ scm_i_make_weak_vector (scm_t_bits type, SCM size, SCM fill) base = SCM_I_WVECT_GC_WVELTS (wv); for (j = 0; j != c_size; ++j) - base[j] = fill; + { + base[j] = fill; + SCM_I_REGISTER_DISAPPEARING_LINK((GC_PTR) base + j, (GC_PTR) SCM_UNPACK(fill)); + } return wv; } @@ -443,6 +474,7 @@ scm_i_make_weak_vector_from_list (scm_t_bits type, SCM lst) scm_is_pair (lst); lst = SCM_CDR (lst), elt++) { + SCM_I_REGISTER_DISAPPEARING_LINK((GC_PTR) elt, (GC_PTR) SCM_UNPACK(SCM_CAR(lst))); *elt = SCM_CAR (lst); } -- 1.6.5.7 [-- Attachment #3: Type: text/plain, Size: 33 bytes --] -- Mike: I WAS NOT MICROWAVED. ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Weak Vectors Patch 2010-04-01 17:18 Weak Vectors Patch Clinton Ebadi @ 2010-04-05 17:03 ` Ludovic Courtès 2010-04-05 17:26 ` Andy Wingo 2010-04-17 18:36 ` Clinton Ebadi 0 siblings, 2 replies; 6+ messages in thread From: Ludovic Courtès @ 2010-04-05 17:03 UTC (permalink / raw) To: Clinton Ebadi; +Cc: bug-guile Hi! Clinton Ebadi <clinton@unknownlamer.org> writes: > Attached is a patch that should improve weak vectors and prevent > `vector-ref' from segfaulting on them after a GC. > > The root of the problem was that weak vectors were being allocated as > containing no pointers *and* disappearing links were not being > registered for initial elements. If you, however, created an empty weak > vector and then `vector-set!' the elements things were fine. Indeed, good catch! Also, disappearing links were not being unregistered when setting a new value with ‘vector-set!’, which your patch fixes. Lastly, disappearing links where consulted without having the alloc lock, which your patch fixes as well. For these last two things, we’d need to modify the ‘vector-ref’ and ‘vector-set’ instructions so that they do the right thing. The best solution would be to just call scm_c_vector_{ref,set_x} when SCM_I_WVECTP, so that the overhead remains low for regular vectors. Can you look into this? > I'm not sure that weak vectors should have their allocated memory marked > as containing no pointers, but it seems to work so I left that alone in > case there was some magic going on that I didn't quite grok. Yes, weak vectors are to live in pointerless memory, otherwise the disappearing links would not disappear. The patch looks good to me, modulo a few things: - Could you make it 3 (or 4?) different patches, each with a test case showing what is being fixed? - If that’s fine with you, you’ll need to assign copyright to the FSF. We can arrange this off-list. Minor remarks in-line: > From a20475f1f9643093ea96118a71826623ecf15220 Mon Sep 17 00:00:00 2001 > From: Clinton Ebadi <clinton@unknownlamer.org> > Date: Thu, 1 Apr 2010 13:11:10 -0400 > Subject: [PATCH] Fix weak vectors > * libguile/vectors.c (scm_i_make_weak_vector, > scm_i_make_weak_vector_from_list): Register elements as disappearing links. > * libguile/vectors.c (do_weak_vector_ref): New function to access an > element of a weak vector while the allocator lock is held. “New function” is enough. > +/* Accessing weak pointers *must* be done with the allocator lock held */ > +static void* > +do_weak_vector_ref (void* data) Please add a space before ‘*’ and punctuate sentences. > + SCM_I_REGISTER_DISAPPEARING_LINK((GC_PTR) base + j, (GC_PTR) SCM_UNPACK(fill)); Please leave a space before ‘(‘. Thanks! Ludo’. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Weak Vectors Patch 2010-04-05 17:03 ` Ludovic Courtès @ 2010-04-05 17:26 ` Andy Wingo 2010-04-17 18:36 ` Clinton Ebadi 1 sibling, 0 replies; 6+ messages in thread From: Andy Wingo @ 2010-04-05 17:26 UTC (permalink / raw) To: Ludovic Courtès; +Cc: Clinton Ebadi, bug-guile On Mon 05 Apr 2010 19:03, ludo@gnu.org (Ludovic Courtès) writes: > For these last two things, we’d need to modify the ‘vector-ref’ and > ‘vector-set’ instructions so that they do the right thing. The best > solution would be to just call scm_c_vector_{ref,set_x} when > SCM_I_WVECTP, so that the overhead remains low for regular vectors. Can > you look into this? I did that last week I think, also at Clinton's urging :) -- http://wingolog.org/ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Weak Vectors Patch 2010-04-05 17:03 ` Ludovic Courtès 2010-04-05 17:26 ` Andy Wingo @ 2010-04-17 18:36 ` Clinton Ebadi 2010-04-17 20:48 ` Ludovic Courtès 1 sibling, 1 reply; 6+ messages in thread From: Clinton Ebadi @ 2010-04-17 18:36 UTC (permalink / raw) To: Ludovic Courtès; +Cc: bug-guile [-- Attachment #1.1: Type: text/plain, Size: 1449 bytes --] ludo@gnu.org (Ludovic Courtès) writes: > For these last two things, we’d need to modify the ‘vector-ref’ and > ‘vector-set’ instructions so that they do the right thing. The best > solution would be to just call scm_c_vector_{ref,set_x} when > SCM_I_WVECTP, so that the overhead remains low for regular vectors. Can > you look into this? Wingo did this. >> I'm not sure that weak vectors should have their allocated memory marked >> as containing no pointers, but it seems to work so I left that alone in >> case there was some magic going on that I didn't quite grok. > > Yes, weak vectors are to live in pointerless memory, otherwise the > disappearing links would not disappear. > > The patch looks good to me, modulo a few things: > > - Could you make it 3 (or 4?) different patches, each with a test case > showing what is being fixed? > > - If that’s fine with you, you’ll need to assign copyright to the > FSF. We can arrange this off-list. I have attached a series of three patches with reformatted GNU style commit logs and the gnu coding style violations fixed. I talked to wingo, and he suggested that (equal? WEAK-VECTOR VECTOR) => #f; I have made this so by unregistering weak vectors as arrays, and doing additional type checking in scm_eq. I am unsure if this has ramifications for the rest of the system however. How do you want to handle copyright assignment? [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1.2: 0001-Forbid-comparison-of-weak-and-normal-vectors.patch --] [-- Type: text/x-diff, Size: 3148 bytes --] From 51c78363c827ff7433bf619a4dd0c9afdbd79696 Mon Sep 17 00:00:00 2001 From: Clinton Ebadi <clinton@unknownlamer.org> Date: Sat, 17 Apr 2010 14:16:30 -0400 Subject: [PATCH 1/3] Forbid comparison of weak and normal vectors * libguile/vectors.c (scm_i_weak_vector_equal_p): New Function. * libguile/eq.c (scm_equal_p): Dispatch scm_tc7_wvect to `scm_i_weak_vector_equal_p', and forbid comparison of normal and weak vectors. --- libguile/eq.c | 11 +++++++++-- libguile/vectors.c | 15 ++++++++++++++- libguile/vectors.h | 1 + 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/libguile/eq.c b/libguile/eq.c index 923fa77..7debae7 100644 --- a/libguile/eq.c +++ b/libguile/eq.c @@ -342,9 +342,16 @@ scm_equal_p (SCM x, SCM y) case scm_tc16_fraction: return scm_i_fraction_equalp (x, y); } - case scm_tc7_vector: case scm_tc7_wvect: - return scm_i_vector_equal_p (x, y); + if (SCM_LIKELY (SCM_TYP7 (y) == scm_tc7_wvect)) + return scm_i_weak_vector_equal_p (x, y); + else + return SCM_BOOL_F; + case scm_tc7_vector: + if (SCM_LIKELY (SCM_TYP7 (y) == scm_tc7_vector)) + return scm_i_vector_equal_p (x, y); + else + return SCM_BOOL_F; } /* Check equality between structs of equal type (see cell-type test above). */ if (SCM_STRUCTP (x)) diff --git a/libguile/vectors.c b/libguile/vectors.c index 321b499..e60e2c3 100644 --- a/libguile/vectors.c +++ b/libguile/vectors.c @@ -509,6 +509,19 @@ scm_i_vector_equal_p (SCM x, SCM y) return SCM_BOOL_T; } +SCM +scm_i_weak_vector_equal_p (SCM x, SCM y) +{ + /* This is slower, but necessary to catch NULLified weak references and + correctly hold the collector lock when accessing each element. */ + long i; + for (i = SCM_I_VECTOR_LENGTH (x) - 1; i >= 0; i--) + if (scm_is_false (scm_equal_p (scm_c_vector_ref (x, i), + scm_c_vector_ref (y, i)))) + return SCM_BOOL_F; + return SCM_BOOL_T; +} + SCM_DEFINE (scm_vector_move_left_x, "vector-move-left!", 5, 0, 0, (SCM vec1, SCM start1, SCM end1, SCM vec2, SCM start2), @@ -622,7 +635,7 @@ vector_get_handle (SCM v, scm_t_array_handle *h) /* the & ~2 allows catching scm_tc7_wvect as well. needs changing if you change tags.h. */ -SCM_ARRAY_IMPLEMENTATION (scm_tc7_vector, 0x7f & ~2, +SCM_ARRAY_IMPLEMENTATION (scm_tc7_vector, 0x7f, vector_handle_ref, vector_handle_set, vector_get_handle) SCM_VECTOR_IMPLEMENTATION (SCM_ARRAY_ELEMENT_TYPE_SCM, scm_make_vector) diff --git a/libguile/vectors.h b/libguile/vectors.h index 3746e90..f426d0c 100644 --- a/libguile/vectors.h +++ b/libguile/vectors.h @@ -74,6 +74,7 @@ SCM_API SCM *scm_vector_writable_elements (SCM vec, #define SCM_I_VECTOR_LENGTH(x) (((size_t) SCM_CELL_WORD_0 (x)) >> 8) SCM_INTERNAL SCM scm_i_vector_equal_p (SCM x, SCM y); +SCM_INTERNAL SCM scm_i_weak_vector_equal_p (SCM x, SCM y); /* Weak vectors share implementation details with ordinary vectors, but no one else should. */ -- 1.6.5.7 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1.3: 0002-Register-disappearing-links-when-initializing-weak-v.patch --] [-- Type: text/x-diff, Size: 2350 bytes --] From cecef73d94a59f5932e28e938145700019203801 Mon Sep 17 00:00:00 2001 From: Clinton Ebadi <clinton@unknownlamer.org> Date: Sat, 17 Apr 2010 14:21:43 -0400 Subject: [PATCH 2/3] Register disappearing links when initializing weak vectors * libguile/vectors.c (scm_c_vector_set_x): Unregister old weak link when assigning to a weak vector. (scm_i_make_weak_vector): Register elements as disappearing links to fill value. (scm_i_make_weak_vector_from_list): Register elements as disappearing links. --- libguile/vectors.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-) diff --git a/libguile/vectors.c b/libguile/vectors.c index e60e2c3..cca57cd 100644 --- a/libguile/vectors.c +++ b/libguile/vectors.c @@ -274,10 +274,11 @@ scm_c_vector_set_x (SCM v, size_t k, SCM obj) if (k >= SCM_I_VECTOR_LENGTH (v)) scm_out_of_range (NULL, scm_from_size_t (k)); (SCM_I_VECTOR_WELTS(v))[k] = obj; - if (SCM_I_WVECTP (v)) + if (SCM_I_WVECTP (v) && !SCM_IS_WHVEC (v)) { /* Make it a weak pointer. */ GC_PTR link = (GC_PTR) & ((SCM_I_VECTOR_WELTS (v))[k]); + GC_unregister_disappearing_link (link); SCM_I_REGISTER_DISAPPEARING_LINK (link, obj); } } @@ -292,10 +293,11 @@ scm_c_vector_set_x (SCM v, size_t k, SCM obj) k = SCM_I_ARRAY_BASE (v) + k*dim->inc; (SCM_I_VECTOR_WELTS (vv))[k] = obj; - if (SCM_I_WVECTP (vv)) + if (SCM_I_WVECTP (vv) && !SCM_IS_WHVEC (vv)) { /* Make it a weak pointer. */ GC_PTR link = (GC_PTR) & ((SCM_I_VECTOR_WELTS (vv))[k]); + GC_unregister_disappearing_link (link); SCM_I_REGISTER_DISAPPEARING_LINK (link, obj); } } @@ -420,7 +422,10 @@ scm_i_make_weak_vector (scm_t_bits type, SCM size, SCM fill) base = SCM_I_WVECT_GC_WVELTS (wv); for (j = 0; j != c_size; ++j) - base[j] = fill; + { + base[j] = fill; + SCM_I_REGISTER_DISAPPEARING_LINK ((GC_PTR) base + j, (GC_PTR) SCM_UNPACK(fill)); + } return wv; } @@ -442,6 +447,7 @@ scm_i_make_weak_vector_from_list (scm_t_bits type, SCM lst) scm_is_pair (lst); lst = SCM_CDR (lst), elt++) { + SCM_I_REGISTER_DISAPPEARING_LINK((GC_PTR) elt, (GC_PTR) SCM_UNPACK(SCM_CAR(lst))); *elt = SCM_CAR (lst); } -- 1.6.5.7 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1.4: 0003-Reference-weak-vectors-with-the-allocator-lock-held.patch --] [-- Type: text/x-diff, Size: 2906 bytes --] From af6252f00326542cfa052ec9c2a99477cd1b1d7e Mon Sep 17 00:00:00 2001 From: Clinton Ebadi <clinton@unknownlamer.org> Date: Sat, 17 Apr 2010 14:23:16 -0400 Subject: [PATCH 3/3] Reference weak vectors with the allocator lock held * libguile/vectors.c (do_weak_vector_ref): New function. (scm_c_vector_ref): Call `do_weak_vector_ref' with the allocator lock held when referencing weak vectors. --- libguile/vectors.c | 54 ++++++++++++++++++++++++++++++++++++++------------- 1 files changed, 40 insertions(+), 14 deletions(-) diff --git a/libguile/vectors.c b/libguile/vectors.c index cca57cd..5788ec1 100644 --- a/libguile/vectors.c +++ b/libguile/vectors.c @@ -202,22 +202,46 @@ scm_vector_ref (SCM v, SCM k) } #undef FUNC_NAME +struct t_weak_vector_ref_args +{ + SCM vector; + size_t k; +}; + +/* Accessing weak pointers *must* be done with the allocator lock held. */ +static void * +do_weak_vector_ref (void* data) +{ + struct t_weak_vector_ref_args *args = (struct t_weak_vector_ref_args *) data; + register SCM elt = (SCM_I_VECTOR_ELTS(args->vector))[args->k]; + + if (elt == SCM_PACK (NULL)) + /* ELT was a weak pointer and got nullified by the GC. */ + return SCM_BOOL_F; + + return elt; +} + SCM scm_c_vector_ref (SCM v, size_t k) { if (SCM_I_IS_VECTOR (v)) { - register SCM elt; - if (k >= SCM_I_VECTOR_LENGTH (v)) scm_out_of_range (NULL, scm_from_size_t (k)); - elt = (SCM_I_VECTOR_ELTS(v))[k]; - if ((elt == SCM_PACK (NULL)) && SCM_I_WVECTP (v)) - /* ELT was a weak pointer and got nullified by the GC. */ - return SCM_BOOL_F; + if (SCM_I_WVECTP (v) && !SCM_IS_WHVEC (v)) + { + struct t_weak_vector_ref_args args; + args.vector = v; + args.k = k; - return elt; + return GC_call_with_alloc_lock (do_weak_vector_ref, &args); + } + else + { + return (SCM_I_VECTOR_ELTS(v))[k]; + } } else if (SCM_I_ARRAYP (v) && SCM_I_ARRAY_NDIM (v) == 1) { @@ -225,18 +249,20 @@ scm_c_vector_ref (SCM v, size_t k) SCM vv = SCM_I_ARRAY_V (v); if (SCM_I_IS_VECTOR (vv)) { - register SCM elt; - if (k >= dim->ubnd - dim->lbnd + 1) scm_out_of_range (NULL, scm_from_size_t (k)); k = SCM_I_ARRAY_BASE (v) + k*dim->inc; - elt = (SCM_I_VECTOR_ELTS (vv))[k]; - if ((elt == SCM_PACK (NULL)) && (SCM_I_WVECTP (vv))) - /* ELT was a weak pointer and got nullified by the GC. */ - return SCM_BOOL_F; + if (SCM_I_WVECTP (vv) && !SCM_IS_WHVEC (vv)) + { + struct t_weak_vector_ref_args args; + args.vector = vv; + args.k = k; + + return GC_call_with_alloc_lock (do_weak_vector_ref, &args); + } - return elt; + return (SCM_I_VECTOR_ELTS (vv))[k]; } scm_wrong_type_arg_msg (NULL, 0, v, "non-uniform vector"); } -- 1.6.5.7 [-- Attachment #1.5: Type: text/plain, Size: 267 bytes --] -- Corinne: this is why we should have designated bath buddies Corinne: to get places you cant reach because youre slippery and in case you get a lil tooo slippery and crack your head open someone can call the coast guard and save you [-- Attachment #2: Type: application/pgp-signature, Size: 229 bytes --] ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Weak Vectors Patch 2010-04-17 18:36 ` Clinton Ebadi @ 2010-04-17 20:48 ` Ludovic Courtès 2010-04-18 11:48 ` Andy Wingo 0 siblings, 1 reply; 6+ messages in thread From: Ludovic Courtès @ 2010-04-17 20:48 UTC (permalink / raw) To: Clinton Ebadi; +Cc: bug-guile [-- Attachment #1: Type: text/plain, Size: 2274 bytes --] Hi! Clinton Ebadi <clinton@unknownlamer.org> writes: > ludo@gnu.org (Ludovic Courtès) writes: [...] >> The patch looks good to me, modulo a few things: >> >> - Could you make it 3 (or 4?) different patches, each with a test case >> showing what is being fixed? >> >> - If that’s fine with you, you’ll need to assign copyright to the >> FSF. We can arrange this off-list. > > I have attached a series of three patches with reformatted GNU style > commit logs and the gnu coding style violations fixed. Cool, thanks! > I talked to wingo, and he suggested that (equal? WEAK-VECTOR VECTOR) => > #f; Hmm what’s the motivation? It seems more intuitive to me to have, e.g., (equal? (make-vector 3 4) (make-weak-vector 3 4)) => #t > How do you want to handle copyright assignment? The usual way. ;-) So, would you be willing to assign the copyright to the Free Software Foundation, so that we could install it in Guile? If so, please send the form below to assign@gnu.org. They’ll then send you the actual copyright assignment form by snail mail, which you’ll have to sign and send back, after which we can integrate these and future changes by you in Guile. Thanks! Ludo’. Please email the following information to assign@gnu.org, and we will send you the assignment form for your past and future changes. Please use your full legal name (in ASCII characters) as the subject line of the message. ---------------------------------------------------------------------- REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES [What is the name of the program or package you're contributing to?] [Did you copy any files or text written by someone else in these changes? Even if that material is free software, we need to know about it.] [Do you have an employer who might have a basis to claim to own your changes? Do you attend a school which might make such a claim?] [For the copyright registration, what country are you a citizen of?] [What year were you born?] [Please write your email address here.] [Please write your postal address here.] [Which files have you changed so far, and which new files have you written so far?] [-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Weak Vectors Patch 2010-04-17 20:48 ` Ludovic Courtès @ 2010-04-18 11:48 ` Andy Wingo 0 siblings, 0 replies; 6+ messages in thread From: Andy Wingo @ 2010-04-18 11:48 UTC (permalink / raw) To: Ludovic Courtès; +Cc: Clinton Ebadi, bug-guile Hi, On Sat 17 Apr 2010 22:48, ludo@gnu.org (Ludovic Courtès) writes: > Clinton Ebadi <clinton@unknownlamer.org> writes: > >> I talked to wingo, and he suggested that (equal? WEAK-VECTOR VECTOR) => >> #f; > > Hmm what’s the motivation? It seems more intuitive to me to have, e.g., > > (equal? (make-vector 3 4) (make-weak-vector 3 4)) > => #t Because these have different tc7's, but they were arrays, this comparison happened via the array handle mechanism; but the array handle implementation wasn't quite right. So perhaps the proper fix is to keep weak vectors as being arrays, so they can be equal? to vectors, but fix up their implementation, to something like this: static SCM weak_vector_handle_ref (scm_t_array_handle *h, size_t idx) { return scm_c_vector_ref (h->array, idx); } static void weak_vector_handle_set (scm_t_array_handle *h, size_t idx, SCM val) { scm_c_vector_set_x (h->array, idx, val); } static void weak_vector_get_handle (SCM v, scm_t_array_handle *h) { h->array = v; h->ndims = 1; h->dims = &h->dim0; h->dim0.lbnd = 0; h->dim0.ubnd = scm_c_vector_length (v) - 1; h->dim0.inc = 1; h->element_type = SCM_ARRAY_ELEMENT_TYPE_SCM; h->elements = h->writable_elements = NULL; /* disallow direct access */ } SCM_ARRAY_IMPLEMENTATION (scm_tc7_wvect, 0x7f, weak_vector_handle_ref, weak_vector_handle_set, weak_vector_get_handle) Clinton can you try this one? Andy -- http://wingolog.org/ ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-04-18 11:48 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-04-01 17:18 Weak Vectors Patch Clinton Ebadi 2010-04-05 17:03 ` Ludovic Courtès 2010-04-05 17:26 ` Andy Wingo 2010-04-17 18:36 ` Clinton Ebadi 2010-04-17 20:48 ` Ludovic Courtès 2010-04-18 11:48 ` Andy Wingo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).