Re: [PATCH] In string-split, add support for character sets and predicates.
Daniel Hartwig mand...@gmail.com writes: One final thing before I resubmit. I just notice that most of this file has uses hard tabs, though there are places with soft. Is it preferable to change the patch to also use hard tabs? IMO, even when editing code that currently uses hard tabs, all new lines should use soft tabs, and ideally all *changed* lines should also be converted to soft tabs. Makefiles are a special case of course. Hard tabs cause several problems, e.g. messed up indentation in patches, even if the file uses hard tabs consistently. Personally, I'd like to work toward a future with no soft tabs, even if it excerbates the messed up indentation problem in the short term. What do you think? Mark PS: I recommend setting 'indent-tabs-mode' to nil in Emacs.
Re: [PATCH] Preserve keyword in 'syntax-rules' and 'define-syntax-rule'
Unfortunately, preserving the macro keyword breaks one of Oleg Kiselyov's macros, namely 'ppat' in system/base/pmatch.scm: --8---cut here---start-8--- (define-syntax ppat (syntax-rules (_ quote unquote) ((_ v _ kt kf) kt) ((_ v () kt kf) (if (null? v) kt kf)) ((_ v (quote lit) kt kf) (if (equal? v (quote lit)) kt kf)) ((_ v (unquote var) kt kf) (let ((var v)) kt)) ((_ v (x . y) kt kf) (if (pair? v) (let ((vx (car v)) (vy (cdr v))) (ppat vx x (ppat vy y kt kf) kf)) kf)) ((_ v lit kt kf) (if (eq? v (quote lit)) kt kf --8---cut here---end---8--- Oleg's macro uses '_' in the keyword position of the pattern, even though '_' is in the literals list. Therefore, it fails to match because 'ppat' does not match that literal. Among other things, this broke our build, which is why Hydra has been failing to build Guile recently. I reverted the change. Mark
Re: [PATCH] Implement ‘hash’ for structs
Hello!
Mark H Weaver m...@netris.org skribis:
l...@gnu.org (Ludovic Courtès) writes:
As incredible as it may seem, ‘hash’ until now always returned 263 % n
for structs, leading to interesting experiences when using structs as
hash table keys.
Yes, do you remember us talking about this long ago on IRC? I wanted to
fix this, but asked whether changing the hash function was okay for 2.0,
and you never gave me an answer :)
I don’t remember, but I’m glad we agree that something must be done.
It’s also a sign that email is better than IRC for these things, as far
as I’m concerned. ;-)
Andy said that he improved the hash function on the master branch.
You might want to look at what he did.
Thanks for the reminder. I just looked, it’s much nicer, but it doesn’t
address this particular problem, so we could port it there afterward.
I guess this 'if' is to avoid an infinite loop if the struct points back
to itself. However, it apparently fails to detect cycles in the general
case.
Yes, indeed.
Here’s an updated patch that uses the ‘depth’ argument of ‘scm_hasher’
for that, as is done for pairs.
Thanks for the review!
Ludo’.
diff --git a/libguile/hash.c b/libguile/hash.c
index a79f03d..8b00a0c 100644
--- a/libguile/hash.c
+++ b/libguile/hash.c
@@ -1,5 +1,6 @@
-/* Copyright (C) 1995,1996,1997, 2000, 2001, 2003, 2004, 2006, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
- *
+/* Copyright (C) 1995, 1996, 1997, 2000, 2001, 2003, 2004, 2006, 2008,
+ * 2009, 2010, 2011, 2012 Free Software Foundation, Inc.
+ *
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public License
* as published by the Free Software Foundation; either version 3 of
@@ -223,6 +224,8 @@ scm_hasher(SCM obj, unsigned long n, size_t d)
significant_bits = (scm_t_uintptr) SCM_POINTER_VALUE (obj) 4UL;
return (size_t) significant_bits % n;
}
+case scm_tcs_struct:
+ return scm_i_struct_hash (obj, n, d);
case scm_tc7_wvect:
case scm_tc7_vector:
{
diff --git a/libguile/struct.c b/libguile/struct.c
index 5837b7c..7e8f68c 100644
--- a/libguile/struct.c
+++ b/libguile/struct.c
@@ -922,6 +922,53 @@ scm_struct_ihashq (SCM obj, unsigned long n, void *closure)
return SCM_UNPACK (obj) % n;
}
+unsigned long
+scm_i_struct_hash (SCM obj, unsigned long n, size_t depth)
+#define FUNC_NAME hash
+{
+ SCM layout;
+ scm_t_bits *data;
+ size_t struct_size, field_num;
+ unsigned long hash;
+
+ SCM_VALIDATE_STRUCT (1, obj);
+
+ layout = SCM_STRUCT_LAYOUT (obj);
+ struct_size = scm_i_symbol_length (layout) / 2;
+ data = SCM_STRUCT_DATA (obj);
+
+ hash = SCM_UNPACK (SCM_STRUCT_VTABLE (obj)) % n;
+ if (depth 0)
+for (field_num = 0; field_num struct_size; field_num++)
+ {
+ int protection;
+
+ protection = scm_i_symbol_ref (layout, field_num * 2 + 1);
+ if (protection != 'h' protection != 'o')
+ {
+ int type;
+ type = scm_i_symbol_ref (layout, field_num * 2);
+ switch (type)
+ {
+ case 'p':
+ hash ^= scm_hasher (SCM_PACK (data[field_num]), n,
+depth / 2);
+ break;
+ case 'u':
+ hash ^= data[field_num] % n;
+ break;
+ default:
+ /* Ignore 's' fields. */;
+ }
+ }
+ }
+
+ /* FIXME: Tail elements should be taken into account. */
+
+ return hash % n;
+}
+#undef FUNC_NAME
+
SCM_DEFINE (scm_struct_vtable_name, struct-vtable-name, 1, 0, 0,
(SCM vtable),
Return the name of the vtable @var{vtable}.)
diff --git a/libguile/struct.h b/libguile/struct.h
index 3072f24..643fd9d 100644
--- a/libguile/struct.h
+++ b/libguile/struct.h
@@ -193,6 +193,8 @@ SCM_API void scm_print_struct (SCM exp, SCM port, scm_print_state *);
SCM_INTERNAL SCM scm_i_struct_equalp (SCM s1, SCM s2);
SCM_INTERNAL unsigned long scm_struct_ihashq (SCM, unsigned long, void *);
+SCM_INTERNAL unsigned long scm_i_struct_hash (SCM s, unsigned long n,
+ size_t depth);
SCM_INTERNAL SCM scm_i_alloc_struct (scm_t_bits *vtable_data, int n_words);
SCM_INTERNAL void scm_i_struct_inherit_vtable_magic (SCM vtable, SCM obj);
SCM_INTERNAL void scm_init_struct (void);
diff --git a/test-suite/tests/structs.test b/test-suite/tests/structs.test
index 431a014..0e3b241 100644
--- a/test-suite/tests/structs.test
+++ b/test-suite/tests/structs.test
@@ -126,7 +126,49 @@
(not (or (equal? (make-ball red Bob) (make-ball green Bob))
(equal? (make-ball red Bob) (make-ball red Bill))
+
+(with-test-prefix hash
+
+ (pass-if simple structs
+(let* ((v (make-vtable pr))
+ (s1 (make-struct v 0 hello))
+ (s2 (make-struct v 0 hello)))
+ (= (hash s1 ) (hash s2
+
+ (pass-if different structs
+(let* ((v (make-vtable pr))
+ (s1 (make-struct v 0 hello))
+ (s2 (make-struct v 0 world)))
+ (or (not (= (hash s1 ) (hash s2 )))
+ (throw 'unresolved
+
+ (pass-if different struct
Re: [PATCH] Preserve keyword in 'syntax-rules' and 'define-syntax-rule'
Hi, Mark H Weaver m...@netris.org skribis: Unfortunately, preserving the macro keyword breaks one of Oleg Kiselyov's macros, namely 'ppat' in system/base/pmatch.scm: [...] Oleg's macro uses '_' in the keyword position of the pattern, even though '_' is in the literals list. Therefore, it fails to match because 'ppat' does not match that literal. I would call it a bug in ‘ppat’. However, the real question is how frequent that “bug” is. If people have come to rely on the current behavior, then it may be more reasonable to stick to it. Among other things, What were the other things? :-) Ludo’.
Re: [PATCH] In string-split, add support for character sets and predicates.
Hi! Mark H Weaver m...@netris.org skribis: I vaguely recall hearing somewhere that this autogeneration of docs was considered a failed experiment, but we'd have to ask Andy or Ludovic about that. Well, look at this section of the manual (especially on hard copy), and compare it to hand-written sections... Ludo’.
Re: [PATCH] In string-split, add support for character sets and predicates.
Hi, FWIW, I think tabs are OK in C files, but not in Scheme files, because in the latter case, we want to be able to copy/paste without triggering tab completion. Our .dir-locals.el follows that. Thanks, Ludo’.
Re: [PATCH] Preserve keyword in 'syntax-rules' and 'define-syntax-rule'
l...@gnu.org (Ludovic Courtès) writes: Mark H Weaver m...@netris.org skribis: Unfortunately, preserving the macro keyword breaks one of Oleg Kiselyov's macros, namely 'ppat' in system/base/pmatch.scm: [...] Oleg's macro uses '_' in the keyword position of the pattern, even though '_' is in the literals list. Therefore, it fails to match because 'ppat' does not match that literal. I would call it a bug in ‘ppat’. However, the real question is how frequent that “bug” is. If people have come to rely on the current behavior, then it may be more reasonable to stick to it. I tend to agree that it's arguably a bug in 'ppat'. I could go either way on this, so I'll leave it up to you and Andy. Among other things, What were the other things? :-) Well, anything that uses pmatch will fail. I'm not aware of any other problems. Mark
Re: [PATCH] Preserve keyword in 'syntax-rules' and 'define-syntax-rule'
On Thu, Oct 11, 2012 at 5:41 AM, Ludovic Courtès l...@gnu.org wrote: Hi, Mark H Weaver m...@netris.org skribis: Unfortunately, preserving the macro keyword breaks one of Oleg Kiselyov's macros, namely 'ppat' in system/base/pmatch.scm: [...] Oleg's macro uses '_' in the keyword position of the pattern, even though '_' is in the literals list. Therefore, it fails to match because 'ppat' does not match that literal. I would call it a bug in ‘ppat’. However, the real question is how frequent that “bug” is. If people have come to rely on the current behavior, then it may be more reasonable to stick to it. This is not a bug. R5RS states: The keyword at the beginning of the pattern in a syntax rule is not involved in the matching and is not considered a pattern variable or literal identifier. R6RS forbids _ as a literal. R7RS retains the R5RS ignoring of the initial keyword, adds _ as a wildcard, but allows it to be used as a literal. So this code would only break in R6RS. -- Alex
