Re: [PATCH] Fix and-let*.

[Mark H Weaver]

Mark H Weaver <address@hidden> writes:

> address@hidden (Taylan Ulrich "Bayırlı/Kammer") writes:
>
>> Our and-let* wasn't entirely conformant to SRFI-2.  It would return #t
>> for a variety of forms where it should return the last evaluated
>> expression's value instead.
>
> Can you give some examples that demonstrate the problem?

Nevermind, I see the problem.  We don't properly handle the case with
clauses but no body.  The bug was introduced in commit
0bf4a5fc2ea456ed74d45f52e2f1dd08d5e1fb9e.

address@hidden (Taylan Ulrich "Bayırlı/Kammer") writes:

> From e08e9a7e1048c8e0ad58e09585e4b6a071906db3 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Taylan=20Ulrich=20Bay=C4=B1rl=C4=B1/Kammer?=
>  <address@hidden>
> Date: Fri, 2 Oct 2015 22:56:04 +0200
> Subject: [PATCH] Fix and-let*.
>
> ---
>  module/ice-9/and-let-star.scm | 50 
> ++++++++++++++++++++++++++++++++-----------
>  1 file changed, 38 insertions(+), 12 deletions(-)

This needs a commit log in accordance with our conventions.

> diff --git a/module/ice-9/and-let-star.scm b/module/ice-9/and-let-star.scm
> index ff15a7a..0c12f16 100644
> --- a/module/ice-9/and-let-star.scm
> +++ b/module/ice-9/and-let-star.scm
> @@ -1,6 +1,7 @@
>  ;;;; and-let-star.scm --- and-let* syntactic form (SRFI-2) for Guile
>  ;;;;
>  ;;;; Copyright (C) 1999, 2001, 2004, 2006, 2013 Free Software Foundation, 
> Inc.
> +;;;; Copyright (C) 2015 Taylan Ulrich Bayırlı/Kammer <address@hidden>
>  ;;;; 
>  ;;;; This library is free software; you can redistribute it and/or
>  ;;;; modify it under the terms of the GNU Lesser General Public
> @@ -22,20 +23,45 @@
>  (define-syntax %and-let*
>    (lambda (form)
>      (syntax-case form ()
> -      ((_ orig-form ())
> -       #'#t)
> -      ((_ orig-form () body bodies ...)
> -       #'(begin body bodies ...))
> -      ((_ orig-form ((var exp) c ...) body ...)
> +
> +      ;; Handle zero-clauses special-case.
> +      ((_ orig-form () . body)
> +       #'(begin #t . body))
> +
> +      ;; Reduce clauses down to one regardless of body.
> +      ((_ orig-form ((var expr) rest . rest*) . body)
> +       (identifier? #'var)
> +       #'(let ((var expr))
> +           (and var (%and-let* (rest . rest*) . body))))

For improved error reporting, the '%and-let*' auxiliary macro accepts
the entire original form 'orig-form' as its first operand.  Here, and in
several other places, you forgot to pass 'orig-form' down to the
recursive use of '%and-let*'.  As a result, I guess this rewritten macro
is broken for all cases with more than one clause.

Anyway, are you willing to assign copyright to the FSF for your
contributions to Guile?  If so, we can get that process started.

     Thanks,
       Mark