bug#9567: curious match bug (?)

bug#9567: curious match bug (?)

[Andy Wingo]

Hi,

Try this:

  (use-modules (language tree-il) (ice-9 match))
  (define foo (parse-tree-il '(let-values (apply (lambda () (lambda-case ((() #f #f #f () ()) (apply (primitive values) (const 1) (const 2)))))) (lambda-case (((a b) #f #f #f () (#{a 134390}# #{b 134391}#)) (apply (primitive list) (lexical a #{a 134390}#) (lexical b #{b 134391}#)))))))
  (match foo
    (($ <let-values> src exp
        ($ <lambda-case> src2 req #f #f #f () gensyms body #f))
     #t)
    (_
     #f))
  => #t
                         
  (match foo
    (($ <let-values> src foo ;; <- rename "exp" to "foo"
        ($ <lambda-case> src2 req #f #f #f () gensyms body #f))
     #t)
    (_
     #f))
  => #f

I tried to reduce this case a bit, but didn't succeed directly, and I
need to move on.  But what is the deal here?

Andy
-- 
http://wingolog.org/

bug#9567: curious match bug (?)

[Stefan Israelsson Tampe]

[-- Attachment #1.1: Type: text/plain, Size: 1226 bytes --]

This is an old bug I spotted before, it can be solved by inserting an extra
let
in the expansion aka

(macroexpand '(match a (par code  ...))
-> '(let ((arg a)) ....)

This is missing from atoms in match so I added that there and the
missbehavior dissapears. see the git diffed patch in this post.


On Wed, Sep 21, 2011 at 5:34 AM, Andy Wingo <wingo@pobox.com> wrote:

> Hi,
>
> Try this:
>
>  (use-modules (language tree-il) (ice-9 match))
>  (define foo (parse-tree-il '(let-values (apply (lambda () (lambda-case
> ((() #f #f #f () ()) (apply (primitive values) (const 1) (const 2))))))
> (lambda-case (((a b) #f #f #f () (#{a 134390}# #{b 134391}#)) (apply
> (primitive list) (lexical a #{a 134390}#) (lexical b #{b 134391}#)))))))
>  (match foo
>    (($ <let-values> src exp
>        ($ <lambda-case> src2 req #f #f #f () gensyms body #f))
>     #t)
>    (_
>     #f))
>  => #t
>
>  (match foo
>    (($ <let-values> src foo ;; <- rename "exp" to "foo"
>        ($ <lambda-case> src2 req #f #f #f () gensyms body #f))
>     #t)
>    (_
>     #f))
>  => #f
>
> I tried to reduce this case a bit, but didn't succeed directly, and I
> need to move on.  But what is the deal here?
>
> Andy
> --
> http://wingolog.org/
>
>
>
>

[-- Attachment #1.2: Type: text/html, Size: 1741 bytes --]

[-- Attachment #2: match-bug.patch --]
[-- Type: text/x-patch, Size: 869 bytes --]

diff --git a/module/ice-9/match.scm b/module/ice-9/match.scm
index 686539b..0384f69 100644
--- a/module/ice-9/match.scm
+++ b/module/ice-9/match.scm
@@ -57,3 +57,21 @@
 ;; Note: Make sure to update `match.test.upstream' when updating this
 ;; file.
 (include-from-path "ice-9/match.upstream.scm")
+
+(define-syntax match
+  (syntax-rules ()
+    ((match)
+     (match-syntax-error "missing match expression"))
+    ((match atom)
+     (match-syntax-error "no match clauses"))
+    ((match (app ...) (pat . body) ...)
+     (let ((v (app ...)))
+       (match-next v ((app ...) (set! (app ...))) (pat . body) ...)))
+    ((match #(vec ...) (pat . body) ...)
+     (let ((v #(vec ...)))
+       (match-next v (v (set! v)) (pat . body) ...)))
+    ((match atom (pat . body) ...)
+     (let ((v atom))
+       (match-next v (atom (set! atom)) (pat . body) ...)))
+    ))
+

bug#9567: curious match bug (?)

[Ludovic Courtès]

Could it be a feature?

  (let ((pat '('a _ ...)))
    (match '(a b c) (pat #t)))
  => #t

Ludo'.

bug#9567: curious match bug (?)

[Stefan Israelsson Tampe]

[-- Attachment #1: Type: text/plain, Size: 776 bytes --]

On the other hand

(let ((pat '(a _ ...)))
   (match '(x y z) (pat #t)))
$1 = #t

as well,

a more canonical example of this problem (where I saw it first)
are when using 'and' e.g.

(define a '(1 2))

(match a ((and (a 2) (1 b)) (+ a b)) (_ #f))
-> #f

But with my fix you wil get the correct 3.

Now the reson are that for 'and' as well as for '$' another location a is
under the command
of an implicit (let ((a car)) ...). One could perhaps argue that this is a
bug in syntax handling but I would not think so. Anyway maybe we should
consider moving this fix upstream.

/Stefan
2011/9/23 Ludovic Courtès <ludo@gnu.org>

> Could it be a feature?
>
>  (let ((pat '('a _ ...)))
>    (match '(a b c) (pat #t)))
>  => #t
>
> Ludo'.
>
>
>
>

[-- Attachment #2: Type: text/html, Size: 1149 bytes --]

bug#9567: curious match bug (?)

[Andy Wingo]

On Fri 23 Sep 2011 16:45, ludo@gnu.org (Ludovic Courtès) writes:

> Could it be a feature?
>
>   (let ((pat '('a _ ...)))
>     (match '(a b c) (pat #t)))
>   => #t

`match' compiles patterns to Scheme code at macro expansion time.  It
doesn't do runtime matching.  Are you thinking that `pat' is being used
as a pattern?  It is not:

  (let ((pat '('b _ ...)))
    (match '(a b c) (pat #t)))
  => #t
  
If it were a pattern, it would not match.  It is a bug IMO.

Andy
-- 
http://wingolog.org/

bug#9567: `match' bug ?

[Andy Wingo]

Hi Alex,

We are getting the following bug in Guile:

    > (use-modules (ice-9 match))

This imports your `match' library.

    > (define a '(1))
    > (match a (((and a 1)) a) (_ #f))
    1

Here we destructured the first element from '(1).

    > (define a '(1 2))
    > (match a ((and (a 2) (1 b)) (+ a b)) (_ #f))
    #f

Now we are trying to destructure the first two elements from '(1 2).
But it doesn't work!  OTOH it does work if we rename the pattern vars:

    > (match a ((and (x 2) (1 y)) (+ x y)) (_ #f))
    3

Can you reproduce this bug on other Schemes?  Could it be that the code
to extract vars to be bound is erroneously propagating the var bound to
the input value?  Or is it a bug in Guile?

Stefan Israelsson Tampe reports that the following redefinition of
`match' fixes the problem for him:

  (define-syntax match
    (syntax-rules ()
      ((match)
       (match-syntax-error "missing match expression"))
      ((match atom)
       (match-syntax-error "no match clauses"))
      ((match (app ...) (pat . body) ...)
       (let ((v (app ...)))
         (match-next v ((app ...) (set! (app ...))) (pat . body) ...)))
      ((match #(vec ...) (pat . body) ...)
       (let ((v #(vec ...)))
         (match-next v (v (set! v)) (pat . body) ...)))
      ((match atom (pat . body) ...)
       (let ((v atom))
         (match-next v (atom (set! atom)) (pat . body) ...)))))

As you see the last case introduces a `let'.

Regards,

Andy
-- 
http://wingolog.org/

bug#9567: `match' bug ?

[Alex Shinn]

Hi Andy,

On Sun, Sep 25, 2011 at 12:01 AM, Andy Wingo <wingo@pobox.com> wrote:
>
> We are getting the following bug in Guile:
> [...]
>
>    > (define a '(1 2))
>    > (match a ((and (a 2) (1 b)) (+ a b)) (_ #f))
>    #f

Yes, that's a bug - confirmed in Chibi.  The
diagnosis is also correct.  I've applied the
patch and updated the synthcode match.scm
and the Chibi repo.

Thanks Stefan!

-- 
Alex

bug#9567: `match' bug ?

[Ludovic Courtès]

Hi Alex,

Alex Shinn <alexshinn@gmail.com> skribis:

> On Sun, Sep 25, 2011 at 12:01 AM, Andy Wingo <wingo@pobox.com> wrote:
>>
>> We are getting the following bug in Guile:
>> [...]
>>
>>    > (define a '(1 2))
>>    > (match a ((and (a 2) (1 b)) (+ a b)) (_ #f))
>>    #f
>
> Yes, that's a bug - confirmed in Chibi.  The
> diagnosis is also correct.  I've applied the
> patch and updated the synthcode match.scm
> and the Chibi repo.

I’m trying to update Guile’s copy from Chibi but changeset
876:528cdab3f818 in the default branch doesn’t seem to contain the fix.

What am I missing?

Thanks,
Ludo’.

bug#9567: `match' bug ?

[Alex Shinn]

Hi Ludovic,

2011/10/15 Ludovic Courtès <ludo@gnu.org>:
>
> I’m trying to update Guile’s copy from Chibi but changeset
> 876:528cdab3f818 in the default branch doesn’t seem to contain the fix.
>
> What am I missing?

"hg export 851" will show you the changes, including
the new test case.  Does it not work in Guile?

On the off chance you're unfamiliar with mercurial,
did you remember to "hg update" after fetching the
changes?

-- 
Alex

bug#9567: `match' bug ?

[Ludovic Courtès]

Hi Alex,

Alex Shinn <alexshinn@gmail.com> skribis:

> 2011/10/15 Ludovic Courtès <ludo@gnu.org>:
>>
>> I’m trying to update Guile’s copy from Chibi but changeset
>> 876:528cdab3f818 in the default branch doesn’t seem to contain the fix.
>>
>> What am I missing?
>
> "hg export 851" will show you the changes, including
> the new test case.  Does it not work in Guile?

Yes.

> On the off chance you're unfamiliar with mercurial,
> did you remember to "hg update" after fetching the
> changes?

Oh indeed, I had run ‘hg pull’ (I think) but not ‘hg update’.

Thanks!

Ludo’.