bug-guile
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#16726: Possible bug?


From: Mark H Weaver
Subject: bug#16726: Possible bug?
Date: Tue, 11 Feb 2014 22:40:32 -0500
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux)

tags 16726 notabug
thanks

Hi,

Nigel Warner <address@hidden> writes:
>   This file is an extract of a documentation and code assembly process
>   which works from a list of elements and attributes. Perhaps I'm
>   being particularly stupid but I can't see why the function
>   emit-groff-bug? does not work as expected.

In the future, please show us what output the program produced,
and what output you expected.

There are several mistakes in your program.

> (define-syntax say
>   (syntax-rules ()
>    ((say format-string ...)
>     (format #t format-string ...))
>    ((say format-string)
>     (format #t format-string))))

The second rule will never be used, because the first rule covers that
case.  In macros, "x ..." matches zero or more items.

> (define-syntax emit
>   (syntax-rules ()
>     ((emit a-string)
>      (say (string-append a-string "\n")))
>     ((emit a-string ...)
>      (say (string-append (format #f a-string ...) "\n")))))

This is a bad idea, because you are conflating the format string with
the strings you want to output.  If the string you want to output
includes "~", then it will misbehave.  Did you notice the compiler
warnings about non-literal format strings?

> (define (process-attributes-for-groff lst)
>   (for-each
>     (lambda (element)
>      (emit ".attribute ~a ~a" (list-ref element 0)
>                               (list-ref element 1))
>      (emit ".start-hint")
>      (emit "~a" (list-ref element 2))
>      (emit ".end-hint"))
>    lst))

I suggest that you use the (ice-9 match) module to destructure 'element'
into its components.  The code will look a lot nicer, and it will also
be more efficient.

> (define (emit-groff lst)
>   (for-each
>    (lambda (element)
>      (cond ((and (pair? element)
>                  (eq? #:object-name (car element)))
>             (emit ".object-name ~a" (cdr element)))
>            ((and (pair? element)
>                  (eq? #:description (car element)))
>             (emit ".start-description")
>             (emit "~a" (cdr element))
>             (emit ".end-description"))
>            ((and (list? element)
>                  (eq? #:attributes (car element)))
>             (process-attributes-for-groff (cdr element)))
>            (else
>             (emit "emit-groff | unknown list type")
>             (throw 'snafu))))
>    lst))
>
> (define (emit-groff-bug? lst)
>   (for-each
>    (lambda (element)
>      (cond ((pair? element)
>             (cond ((eq? #:object-name (car element))
>                    (emit ".object-name ~a" (cdr element)))
>                   ((eq? #:description (car element))
>                    (emit ".start-description")
>                    (emit "~a" (cdr element))
>                    (emit ".end-description"))))
>            ((list? element)
>             (cond ((eq? #:attributes (car element)))
>                   (process-attributes-for-groff (cdr element)))
>             (else
>              (emit "emit-groff | unknown list type")
>              (throw 'snafu)))))
>    lst))

There are several mistakes in this procedure:

* Every list is also a pair, so the (list? element) case is never taken.
  The (pair? element) case is taken for the #:attributes, and then no
  matching case is found in the inner 'cond' there.

There are two parenthesis errors:

* You have the (process-attributes-for-groff (cdr element)) in a
  position where its treated as a clause of the inner 'cond', instead of
  as an expression within the (eq? #:attributes (car element)) case.

* You've put the 'else' within the "(list? element)" case, instead of as
  its own clause.  Did you see the compiler warning about "possibly
  unbound variable `else'"?

     Mark





reply via email to

[Prev in Thread] Current Thread [Next in Thread]