[PATCH 0/6] A handful of post 3.0.10 fixups

[PATCH 0/6] A handful of post 3.0.10 fixups

[Rob Browning]

I generated this set of patches while trying to track down the Debian
buildd failures.  Four of them are bug fixes, the other two are an
optimization and a doc fix.

Proposed for main, and if they're deemed plausible, I'll add relevant
changelog entries and NEWS updates.

Thanks

Rob Browning (6):
  define-meta-command: mention effects of a missing category
  Ensure GUILE-VERSION changes propagate to .version and Makefiles
  Add guile-procedures.txt to BUILT_SOURCES
  test-hashing: support 32-bit
  scm_i_utf8_string_hash: don't overrun when len is zero
  scm_i_utf8_string_hash: optimize ASCII

 Makefile.am                          |  5 +--
 configure.ac                         |  1 +
 libguile/hash.c                      | 56 +++++++++++++++-------------
 module/system/repl/command.scm       |  2 +
 test-suite/standalone/test-hashing.c |  8 +++-
 5 files changed, 43 insertions(+), 29 deletions(-)

-- 
2.43.0

[PATCH 1/6] define-meta-command: mention effects of a missing category

[Rob Browning]

module/system/repl/command.scm: add comment.

Signed-off-by: Rob Browning <rlb@defaultvalue.org>
---
 module/system/repl/command.scm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/module/system/repl/command.scm b/module/system/repl/command.scm
index ca7450610..8b0422dbd 100644
--- a/module/system/repl/command.scm
+++ b/module/system/repl/command.scm
@@ -235,6 +235,8 @@
      (define-meta-command ((name category) repl () . datums)
        docstring b0 b1 ...))
 
+    ;; These cases (with category #f) will only produce functional
+    ;; commands if the name is already in the *command-table*.
     ((_ (name repl (expression0 ...) . datums) docstring b0 b1 ...)
      (define-meta-command ((name #f) repl (expression0 ...) . datums)
        docstring b0 b1 ...))
-- 
2.43.0

[PATCH 2/6] Ensure GUILE-VERSION changes propagate to .version and Makefiles

[Rob Browning]

Have .version depend on the Makefile, and move our
CONFIG_STATUS_DEPENDENCIES setting to an AC_SUBST, as recommended by the
automake info pages "Rebuilding Makefiles" section, so that changes to
GUILE-VERSION will update the VERSION, etc. in the generated Makefiles.

* Makefile.am (CONFIG_STATUS_DEPENDENCIES): drop.
($(top_srcdir/.version)): depend on Makefile.
* configure: add GUILE-VERSION to CONFIG_STATUS_DEPENDENCIES via
AC_SUBST.

Signed-off-by: Rob Browning <rlb@defaultvalue.org>
---
 Makefile.am  | 4 +---
 configure.ac | 1 +
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index b2ac5539e..c74761628 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -110,8 +110,6 @@ endif !HAVE_READLINE
 clean-local:
 	rm -rf cache/
 
-CONFIG_STATUS_DEPENDENCIES = GUILE-VERSION
-
 gen_start_rev = 61db429e251bfd2f75cb4632972e0238056eb24b
 .PHONY: gen-ChangeLog
 gen-ChangeLog:
@@ -133,7 +131,7 @@ assert-no-store-file-names:
 	fi
 
 BUILT_SOURCES += $(top_srcdir)/.version
-$(top_srcdir)/.version:
+$(top_srcdir)/.version: Makefile
 	echo $(VERSION) > $@-t && mv $@-t $@
 gen-tarball-version:
 	echo $(VERSION) > $(distdir)/.tarball-version
diff --git a/configure.ac b/configure.ac
index 0dcb71cce..262b171f1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -40,6 +40,7 @@ m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])], [AC_SUBST([AM_DEFAULT_VERB
 AC_COPYRIGHT(GUILE_CONFIGURE_COPYRIGHT)
 AC_CONFIG_SRCDIR([GUILE-VERSION])
 
+AC_SUBST([CONFIG_STATUS_DEPENDENCIES], ['$(top_srcdir)/GUILE-VERSION'])
 . $srcdir/GUILE-VERSION
 GUILE_VERSION="$PACKAGE_VERSION"
 
-- 
2.43.0

[PATCH 3/6] Add guile-procedures.txt to BUILT_SOURCES

[Rob Browning]

Add guile-procedures.txt to BUILT_SOURCES to ensure it's available to
tests.  Without this, a "make check" from a clean checkout the
"bit-extract documented?" test in bit-operations.test will fail.

* Makefile.am (BUILT_SOURCES): add guile-procedures.txt

Signed-off-by: Rob Browning <rlb@defaultvalue.org>
---
 Makefile.am | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile.am b/Makefile.am
index c74761628..0df074475 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -59,6 +59,7 @@ libguile/scmconfig.h:
 
 # Build it from here so that all the modules are compiled by the time we
 # build it.
+BUILT_SOURCES += libguile/guile-procedures.txt
 libguile/guile-procedures.txt: libguile/guile-procedures.texi
 	$(AM_V_GEN)						\
 	$(top_builddir)/meta/guile --no-auto-compile		\
-- 
2.43.0

Re: [PATCH 3/6] Add guile-procedures.txt to BUILT_SOURCES

[Rob Browning]

Rob Browning <rlb@defaultvalue.org> writes:

> Add guile-procedures.txt to BUILT_SOURCES to ensure it's available to
> tests.  Without this, a "make check" from a clean checkout the
> "bit-extract documented?" test in bit-operations.test will fail.
>
> * Makefile.am (BUILT_SOURCES): add guile-procedures.txt

Hmm, I just did a bit more testing, and this might not be quite
right/sufficient yet.

-- 
Rob Browning
rlb @defaultvalue.org and @debian.org
GPG as of 2011-07-10 E6A9 DA3C C9FD 1FF8 C676 D2C4 C0F0 39E9 ED1B 597A
GPG as of 2002-11-03 14DD 432F AE39 534D B592 F9A0 25C8 D377 8C7E 73A4

[PATCH 4/6] test-hashing: support 32-bit

[Rob Browning]

Signed-off-by: Rob Browning <rlb@defaultvalue.org>
---
 test-suite/standalone/test-hashing.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/test-suite/standalone/test-hashing.c b/test-suite/standalone/test-hashing.c
index 5982a0fdb..50e132989 100644
--- a/test-suite/standalone/test-hashing.c
+++ b/test-suite/standalone/test-hashing.c
@@ -38,9 +38,15 @@ test_hashing ()
 
   // Value determined by calling wide_string_hash on {0x3A0, 0x3B5,
   // 0x3C1, 0x3AF} via a temporary test program.
+#if SIZEOF_UNSIGNED_LONG == 8
   const unsigned long expect = 4029223418961680680;
-  const unsigned long actual = scm_to_ulong (scm_symbol_hash (sym));
+#elif SIZEOF_UNSIGNED_LONG == 4
+  const unsigned long expect = 938126682;
+#else
+#error "unsigned long not 4 or 8 bytes (need additonal test data)"
+#endif
 
+  const unsigned long actual = scm_to_ulong (scm_symbol_hash (sym));
   if (actual != expect)
     {
       fprintf (stderr, "fail: unexpected utf-8 symbol hash (%lu != %lu)\n",
-- 
2.43.0

[PATCH 5/6] scm_i_utf8_string_hash: don't overrun when len is zero

[Rob Browning]

When the length is zero, the previous code would include the byte after
the end of the string in the hash.  Fix that (the wide an narrow hashers
also guard against it via "case 0"), and while we're there, switch to
u8_mbtouc since the unsafe variant is now the same (see the info pages),
and don't bother mutating length for the trailing bytes, since we don't
need to.

Signed-off-by: Rob Browning <rlb@defaultvalue.org>
---
 libguile/hash.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/libguile/hash.c b/libguile/hash.c
index a038a11bf..d92f60df8 100644
--- a/libguile/hash.c
+++ b/libguile/hash.c
@@ -195,32 +195,34 @@ scm_i_utf8_string_hash (const char *str, size_t len)
   /* Handle most of the key.  */
   while (length > 3)
     {
-      ustr += u8_mbtouc_unsafe (&u32, ustr, end - ustr);
+      ustr += u8_mbtouc (&u32, ustr, end - ustr);
       a += u32;
-      ustr += u8_mbtouc_unsafe (&u32, ustr, end - ustr);
+      ustr += u8_mbtouc (&u32, ustr, end - ustr);
       b += u32;
-      ustr += u8_mbtouc_unsafe (&u32, ustr, end - ustr);
+      ustr += u8_mbtouc (&u32, ustr, end - ustr);
       c += u32;
       mix (a, b, c);
       length -= 3;
     }
 
   /* Handle the last 3 elements's.  */
-  ustr += u8_mbtouc_unsafe (&u32, ustr, end - ustr);
-  a += u32;
-  if (--length)
+  if (length)
     {
-      ustr += u8_mbtouc_unsafe (&u32, ustr, end - ustr);
-      b += u32;
-      if (--length)
+      ustr += u8_mbtouc (&u32, ustr, end - ustr);
+      a += u32;
+      if (length > 1)
         {
-          ustr += u8_mbtouc_unsafe (&u32, ustr, end - ustr);
-          c += u32;
+          ustr += u8_mbtouc (&u32, ustr, end - ustr);
+          b += u32;
+          if (length > 2)
+            {
+              ustr += u8_mbtouc (&u32, ustr, end - ustr);
+              c += u32;
+            }
         }
+      final (a, b, c);
     }
 
-  final (a, b, c);
-
   if (sizeof (unsigned long) == 8)
     ret = (((unsigned long) c) << 32) | b;
   else
-- 
2.43.0

[PATCH 6/6] scm_i_utf8_string_hash: optimize ASCII

[Rob Browning]

Since we already compute the char length, use that to detect all ASCII
strings and handle those the same way we handle latin-1.

Signed-off-by: Rob Browning <rlb@defaultvalue.org>
---
 libguile/hash.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/libguile/hash.c b/libguile/hash.c
index d92f60df8..bc65deb25 100644
--- a/libguile/hash.c
+++ b/libguile/hash.c
@@ -169,25 +169,29 @@ scm_i_latin1_string_hash (const char *str, size_t len)
 unsigned long 
 scm_i_utf8_string_hash (const char *str, size_t len)
 {
-  const uint8_t *end, *ustr = (const uint8_t *) str;
-  unsigned long ret;
-
-  /* The length of the string in characters.  This name corresponds to
-     Jenkins' original name.  */
-  size_t length;
-
-  uint32_t a, b, c, u32;
-
   if (len == (size_t) -1)
     len = strlen (str);
 
-  end = ustr + len;
-
+  const uint8_t *ustr = (const uint8_t *) str;
   if (u8_check (ustr, len) != NULL)
     /* Invalid UTF-8; punt.  */
     return scm_i_string_hash (scm_from_utf8_stringn (str, len));
 
-  length = u8_mbsnlen (ustr, len);
+  /* The length of the string in characters.  This name corresponds to
+     Jenkins' original name.  */
+  size_t length = u8_mbsnlen (ustr, len);
+
+  if (len == length) // ascii, same as narrow_string_hash above
+    {
+      unsigned long ret;
+      JENKINS_LOOKUP3_HASHWORD2 (str, len, ret);
+      ret >>= 2; /* Ensure that it fits in a fixnum.  */
+      return ret;
+    }
+
+  const uint8_t *end = ustr + len;
+  uint32_t a, b, c, u32;
+  unsigned long ret;
 
   /* Set up the internal state.  */
   a = b = c = 0xdeadbeef + ((uint32_t)(length<<2)) + 47;
-- 
2.43.0