* stack overflow equal? values @ 2007-01-16 16:57 Marco Maggi 2007-01-17 23:32 ` Kevin Ryde 0 siblings, 1 reply; 5+ messages in thread From: Marco Maggi @ 2007-01-16 16:57 UTC (permalink / raw) Dunno if this was reported before: guile> (equal? (values 1 2) (values 1 2)) ERROR: Stack overflow Guile 1.8.1 -- Marco Maggi _______________________________________________ Bug-guile mailing list Bug-guile@gnu.org http://lists.gnu.org/mailman/listinfo/bug-guile ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: stack overflow equal? values 2007-01-16 16:57 stack overflow equal? values Marco Maggi @ 2007-01-17 23:32 ` Kevin Ryde 2007-01-18 12:57 ` Ludovic Courtès 0 siblings, 1 reply; 5+ messages in thread From: Kevin Ryde @ 2007-01-17 23:32 UTC (permalink / raw) Cc: bug-guile "Marco Maggi" <marco.maggi-ipsu@poste.it> writes: > > guile> (equal? (values 1 2) (values 1 2)) > ERROR: Stack overflow Thanks, looks like a bug introduced by recursing into structs in equal?. I guess scm_i_struct_equalp should check and ignore a field type "s" ... _______________________________________________ Bug-guile mailing list Bug-guile@gnu.org http://lists.gnu.org/mailman/listinfo/bug-guile ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: stack overflow equal? values 2007-01-17 23:32 ` Kevin Ryde @ 2007-01-18 12:57 ` Ludovic Courtès 2007-01-18 22:42 ` Kevin Ryde 0 siblings, 1 reply; 5+ messages in thread From: Ludovic Courtès @ 2007-01-18 12:57 UTC (permalink / raw) Cc: bug-guile [-- Attachment #1: Type: text/plain, Size: 1074 bytes --] Hi Kevin, Kevin Ryde <user42@zip.com.au> writes: > "Marco Maggi" <marco.maggi-ipsu@poste.it> writes: >> >> guile> (equal? (values 1 2) (values 1 2)) >> ERROR: Stack overflow > > Thanks, looks like a bug introduced by recursing into structs in > equal?. I guess scm_i_struct_equalp should check and ignore a field > type "s" ... Indeed, good guess! I propose the following simple fix. Ok to apply? Actually, `scm_i_struct_equalp ()' should also compare the "tail elements" (when there are tail elements), but their semantics are a little fuzzy to me. In particular, I don't understand why the size of the tail array can be specified in both `make-vtable-vtable' and `make-struct': What does that mean? Which one should really be taken into account? It seems that the code is a bit unclear on this too: guile> (define v (make-vtable-vtable "pr" 0)) guile> (define s (make-struct v 123)) guile> (struct-ref s 10) Segmentation fault (Looks like the API is so complex that few people actually bothered to use it to its full extents. ;-)) Thanks, Ludovic. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: The patch. --] [-- Type: text/x-patch, Size: 1416 bytes --] --- orig/libguile/struct.c +++ mod/libguile/struct.c @@ -564,8 +564,11 @@ field1 = scm_struct_ref (s1, s_field_num); field2 = scm_struct_ref (s2, s_field_num); - if (scm_is_false (scm_equal_p (field1, field2))) - return SCM_BOOL_F; + /* Self-referencing fields (type `s') must be skipped to avoid infinite + recursion. */ + if (!(scm_is_eq (field1, s1) && (scm_is_eq (field2, s2)))) + if (scm_is_false (scm_equal_p (field1, field2))) + return SCM_BOOL_F; } return SCM_BOOL_T; --- orig/test-suite/tests/structs.test +++ mod/test-suite/tests/structs.test @@ -82,12 +82,18 @@ (set-owner! ball "Bill") (string=? (owner ball) "Bill"))) - (pass-if "equal?" + (pass-if "equal? (simple structs)" + (let* ((vtable (make-vtable-vtable "pr" 0)) + (s1 (make-struct vtable 0 "hello")) + (s2 (make-struct vtable 0 "hello"))) + (equal? s1 s2))) + + (pass-if "equal? (more complex structs)" (let ((first (make-ball red (string-copy "Bob"))) - (second (make-ball red (string-copy "Bob")))) + (second (make-ball red (string-copy "Bob")))) (equal? first second))) (pass-if "not-equal?" - (not (or (equal? (make-ball red "Bob") (make-ball green "Bill")) + (not (or (equal? (make-ball red "Bob") (make-ball green "Bob")) (equal? (make-ball red "Bob") (make-ball red "Bill")))))) [-- Attachment #3: Type: text/plain, Size: 137 bytes --] _______________________________________________ Bug-guile mailing list Bug-guile@gnu.org http://lists.gnu.org/mailman/listinfo/bug-guile ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: stack overflow equal? values 2007-01-18 12:57 ` Ludovic Courtès @ 2007-01-18 22:42 ` Kevin Ryde 2007-01-19 9:19 ` Ludovic Courtès 0 siblings, 1 reply; 5+ messages in thread From: Kevin Ryde @ 2007-01-18 22:42 UTC (permalink / raw) Cc: bug-guile ludovic.courtes@laas.fr (Ludovic Courtès) writes: > > I propose the following simple fix. Ok to apply? Yep, looks good. After I posted I wondered if the values struct is an actual "s" or if there's some strange extra I couldn't spot. Testing eq avoids worrying about that. Dunno why values are a struct and not a smob cell. > Actually, `scm_i_struct_equalp ()' should also compare the "tail > elements" (when there are tail elements), Yes. > but their semantics are a > little fuzzy to me. In particular, I don't understand why the size of > the tail array can be specified in both `make-vtable-vtable' and > `make-struct': What does that mean? Which one should really be taken > into account? Dunno :). > It seems that the code is a bit unclear on this too: > > guile> (define v (make-vtable-vtable "pr" 0)) > guile> (define s (make-struct v 123)) > guile> (struct-ref s 10) > Segmentation fault A segv is a bug, obviously, whichever way it's actually meant to be. > (Looks like the API is so complex that few people actually bothered to > use it to its full extents. ;-)) The records level is friendlier I guess. For a long time I couldn't understand what "vtable" meant, I still don't think I quite do. Maybe the docs should be tweaked, to help show what structs are typically meant to be. > --- orig/test-suite/tests/structs.test > +++ mod/test-suite/tests/structs.test > @@ -82,12 +82,18 @@ > (set-owner! ball "Bill") > (string=? (owner ball) "Bill"))) > > - (pass-if "equal?" You can make a with-test-prefix group if you like. An exercise of an actual values too will be good, maybe in eval.test. _______________________________________________ Bug-guile mailing list Bug-guile@gnu.org http://lists.gnu.org/mailman/listinfo/bug-guile ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: stack overflow equal? values 2007-01-18 22:42 ` Kevin Ryde @ 2007-01-19 9:19 ` Ludovic Courtès 0 siblings, 0 replies; 5+ messages in thread From: Ludovic Courtès @ 2007-01-19 9:19 UTC (permalink / raw) Cc: bug-guile Hi, Kevin Ryde <user42@zip.com.au> writes: > Yep, looks good. Applied to both branches (I also converted the change logs in 1.8 to UTF-8 :-)). > After I posted I wondered if the values struct is an actual "s" or if > there's some strange extra I couldn't spot. Testing eq avoids > worrying about that. Yes. The `s' field is actually part of the REQUIRED_VTABLE_FIELDS that gets automatically added in `make-vtable-vtable'. > Dunno why values are a struct and not a smob cell. Maybe because structs looked cool at that time. ;-) >> but their semantics are a >> little fuzzy to me. In particular, I don't understand why the size of >> the tail array can be specified in both `make-vtable-vtable' and >> `make-struct': What does that mean? Which one should really be taken >> into account? > > Dunno :). Then maybe we should "do something" about it in 1.9, and fix remaining bugs. > The records level is friendlier I guess. For a long time I couldn't > understand what "vtable" meant, I still don't think I quite do. Maybe > the docs should be tweaked, to help show what structs are typically > meant to be. Yes. Actually, I used mostly exclusively either SRFI-9 records or GOOPS classes. Then there's also Guile's built-in records as you said. Structs are nice in that they can be easily accessed both from C and Scheme, so they can probably be used in lieu of SMOBs in some cases. Still, that leaves us with 3 (or 4) record/struct APIs, and we may also want to implement R6RS records at some point... Thanks, Ludovic. _______________________________________________ Bug-guile mailing list Bug-guile@gnu.org http://lists.gnu.org/mailman/listinfo/bug-guile ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2007-01-19 9:19 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-01-16 16:57 stack overflow equal? values Marco Maggi 2007-01-17 23:32 ` Kevin Ryde 2007-01-18 12:57 ` Ludovic Courtès 2007-01-18 22:42 ` Kevin Ryde 2007-01-19 9:19 ` Ludovic Courtès
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).