Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discrepancy between match behaviour and documentation #3403

Open
JustinHuPrime opened this issue Sep 20, 2020 · 21 comments
Open

Discrepancy between match behaviour and documentation #3403

JustinHuPrime opened this issue Sep 20, 2020 · 21 comments
Labels
documentation Issues related to README and documentation (typos, rewording, new docs, etc)

Comments

@JustinHuPrime
Copy link

What version of Racket are you using?
e.g., 7.2 [3m]

What program did you run?

(match '(+ 1 2 3)
  [`(+ ,@a) #:when ((listof number?) a) (list "match" a)]
  [_ "no match"])

What should have happened?
I expect a syntax error - it is not permissible to use unquote-splicing followed by just an identifier, according to the documentation for match patterns. Instead, I get '("match" (1 2 3))

Please include any other relevant details
e.g., the operating system used or how you are running the code.

Using DrRacket's definitions window, on Ubuntu Linux (20.04)

@jackfirth
Copy link
Sponsor Contributor

I think this is an error in the docs. None of the documented rules for quasipatterns cover that case:

,pat - match pat
,@(list lvp ...) - match lvps, spliced
,@(list-rest lvp ... pat) - match lvps plus pat, spliced
,@'qp - match list-matching qp, spliced

What happens if you try ,@'a?

@jackfirth jackfirth added the documentation Issues related to README and documentation (typos, rewording, new docs, etc) label Sep 30, 2020
@JustinHuPrime
Copy link
Author

JustinHuPrime commented Sep 30, 2020

I think this is an error in the docs. None of the documented rules for quasipatterns cover that case:

,pat - match pat
,@(list lvp ...) - match lvps, spliced
,@(list-rest lvp ... pat) - match lvps plus pat, spliced
,@'qp - match list-matching qp, spliced

What happens if you try ,@'a?

The #:when clause has an error: a: unbound identifier in: a

@jackfirth
Copy link
Sponsor Contributor

That does make sense, since it's trying to match a list whose tail matches the literal symbol 'a instead of whose tail is bound to a pattern variable. I think that quote before qp in the docs is a typo and should be removed.

@JustinHuPrime
Copy link
Author

I think the typo is that the quote should be a quasiquote - the following match works as expected if the single quote in the docs were a backtick:

(match '(+ 1 2 3)
  [`(+ ,@`(,a ,b ,c)) (list "match" a b c)]
  [_ "no match"])

@jackfirth
Copy link
Sponsor Contributor

Oh that would make sense. Then I think fixing that clause and adding another clause for the bare identifier case would be appropriate.

@JustinHuPrime
Copy link
Author

I'm not sure the bare identifier case should exist at all - this snippet fails on a syntax error:

#lang racket
(match '(+ 1 2 3)
  [`(+ ,@a ,@b) (list "match" a)]
  [_ "no match"])

The error is match: non-list pattern inside unquote-splicing in: a

I'm not sure how Racket would decide how to split up the list '(1 2 3) between a and b, especially if there's #:when clauses involved.

@sorawee
Copy link
Collaborator

sorawee commented Sep 30, 2020

I didn't totally follow the discussion, but on Slack I believe that @pavpanchekha (and maybe @samth?) mentioned that they are planning to improve the bare identifier case. So I would consider the failure in the bare identifier case a bug that should be fixed, rather than something that shouldn't be supported.

@sorawee
Copy link
Collaborator

sorawee commented Sep 30, 2020

FWIW, here's a message from @pavpanchekha:

Why does the following not work?
(match-define `(0 ,@v 9) (range 10))
I expect it to do the same as this:
(match-define `(0 ,@(list v ...) 9) (range 10))

(match-define `(0 ,@v 9) (range 10)) results in the very same error that @JustinHuPrime mentioned above.

@pavpanchekha
Copy link
Contributor

Yep, that's the case I ran into. It's true that (+ ,@a ,@b) would be ambiguous but (+ ,a ... ,b ...) has the same ambiguity so that horse is out of the barn.

@JustinHuPrime
Copy link
Author

Yep, that's the case I ran into. It's true that (+ ,@a ,@b) would be ambiguous but (+ ,a ... ,b ...) has the same ambiguity so that horse is out of the barn.

I don't think that (quasiquote + ,a ... ,b ...) is a valid pattern - ... isn't allowed in a quasipattern, and the pat production doesn't allow a ... to follow a bare identifier.

@pavpanchekha
Copy link
Contributor

pavpanchekha commented Sep 30, 2020

I meant those two examples with a quasiquote in front:

> (match-define `(+ ,a ... ,b ...) `(+ 1 2 3 4 5 5 6))
> a
'(1 2 3 4 5 5 6)
> b
'()

@sorawee
Copy link
Collaborator

sorawee commented Sep 30, 2020

I don't think that (quasiquote + ,a ... ,b ...) is a valid pattern - ... isn't allowed in a quasipattern, and the pat production doesn't allow a ... to follow a bare identifier.

If you meant (quasiquote (+ ,a ... ,b ...)), the answer is that it does. See:

qp ::= ........
     | (qp ooo . qp)
     | ........

on https://docs.racket-lang.org/reference/match.html?q=match#%28form._%28%28lib._racket%2Fmatch..rkt%29._match%29%29.

@JustinHuPrime
Copy link
Author

I don't think that (quasiquote + ,a ... ,b ...) is a valid pattern - ... isn't allowed in a quasipattern, and the pat production doesn't allow a ... to follow a bare identifier.

If you meant (quasiquite (+ ,a ... ,b ...)), the answer is that it does. See:

qp ::= ........
     | (qp ooo . qp)
     | ........

on https://docs.racket-lang.org/reference/match.html?q=match#%28form._%28%28lib._racket%2Fmatch..rkt%29._match%29%29.

Ah, my bad on both counts.

@sorawee
Copy link
Collaborator

sorawee commented Sep 30, 2020

I'm not sure how Racket would decide how to split up the list '(1 2 3) between a and b, especially if there's #:when clauses involved.

#:when clause is performed after pattern matching is done. And for pattern matching itself, when there's an ambiguity, it splits things up nondeterministically.

It's the same situation as:

(match-define (list a ... b ...) (list 1 2 3 4))

@pavpanchekha
Copy link
Contributor

The pull request mentioned above adjusts the behavior for quasiquote so it now always works with bare variables. Does the documentation still need to be adjusted?

@sorawee
Copy link
Collaborator

sorawee commented Jan 12, 2021

There's a weird ' in ,@'qp. That should be ,@qp IIUC.

@sorawee
Copy link
Collaborator

sorawee commented Jan 12, 2021

Scratch that. ,@'qp seems correct already, though weirdly constrained (e.g., what about ,@(quasiquote qp)?). The new ,@id doesn't match the current grammar, so it should be added.

From a quick test,

(match (vector 1 2)
  [`(,@(vector 1 2)) 'a])

seems to work. Would it make more sense to replace

| ,@(list lvp ...) |   | match lvps, spliced
| ,@(list-rest lvp ... pat) |   | match lvps plus pat, spliced
| ,@'qp |   | match list-matching qp, spliced

with one general clause, allowing arbitrary pat? I.e., ,@pat.

@shhyou
Copy link
Collaborator

shhyou commented Jan 12, 2021

I think ,@pat still doesn't work in general, but perhaps unquote splicing at the end of (qp ...) is a special case that the implementation doesn't check for grammar errors. e.g.

;; okay
(match '(1 2 3) [`(,@'(1 2) 3) 'okay])
(match '(1 2 3) [`(,@(list 1 2) 3) 'okay])

;; works after the above fix
(match '(1 2 3) [`(,@x 3) 'okay])

;; doesn't work
(match '(1 2 3) [`(,@(and x) 3) x])

@samth
Copy link
Sponsor Member

samth commented Jan 13, 2021

I'm confused about the current state here. Is there still a problem with the docs?

@shhyou
Copy link
Collaborator

shhyou commented Jan 13, 2021

@samth Yes. In the docs, ,@id is missing in the grammar of qp. PR #3597 makes ,@id a valid pattern.

A separate issue is that (qp ... ,@pat) is allowed by the implementation for arbitrary (valid or invalid) pattern pat. I don't know what is the status of this.

[((unquote-splicing p))
(let ([pat (parameterize ([in-splicing? #t]) (parse #'p))])
pat)]
[((unquote-splicing p) . rest)
(let ([pat (parameterize ([in-splicing? #t]) (parse #'p))]
[rpat (pq #'rest)])
(if (null-terminated? pat)
(append-pats pat rpat)
(raise-syntax-error 'match "non-list pattern inside unquote-splicing"
stx #'p)))]

@samth
Copy link
Sponsor Member

samth commented Jan 13, 2021

Ok, then we should fix the docs. I still don't understand the second comment you're making.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Issues related to README and documentation (typos, rewording, new docs, etc)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants