racket / htdp

Other
91 stars 69 forks source link

2htdp/abstraction's `match` accidentally re-uses parts `racket/match`s pattern language. #171

Closed jasonhemann closed 2 years ago

jasonhemann commented 2 years ago

Using ASL and the abstractions teachpack we define a couple of structs and try to match against instances. These look like they ought to be perfectly valid patterns, and we'd expect 3 as the result. However, we get 1. It looks like var must be being understood as part of matchs pattern language, rather than the struct we're intending to match. The error is more obvious if you try, e.g., pregexp struct.

(require 2htdp/abstraction)

(define-struct var (x))
(define-struct a (x))
(define-struct pregexp (z))
(define-struct b (x))

(define (ee expr)
  (match expr
    [(var x) 1]
    [(a x) 2]
    [(b x) 3]))

(ee (make-b 'cat))
mfelleisen commented 2 years ago

This program exhibits the same behavior:

#lang racket 

(match 1
  [(var x) 2])

Why?

From the documentation of match:

pat

::=

id

match anything, bind identifier

|

(var id)

match anything, bind identifier

I guess you could argue that a “pedagogic” version of match should not implement this feature but why should plain match do so. @samth ?

samth commented 2 years ago

The point of var is to allow you to write patterns like (list x (var ...)) which matches two element lists and blinds the second element to ....

It is not a really great feature and is not worth it in pedagogic versions.

mfelleisen commented 2 years ago

So the implementation of var is in the parser:

    [(var v)
     (identifier? #'v)
     (Var (rearm #'v))]

which suggests I can't eliminate from match easily -- without reimplementing a parser.

Am I missing something here?

sorawee commented 2 years ago

It's not just var, but also list, vector, etc. I think we need a custom parser anyway if we want to disallow programs like:

(require 2htdp/abstraction)

(match '(1)
  [(list 1) 2])

To avoid (name pattern [...]) to be incorrectly recognized, we can expand it to (struct name pattern [...]).

These must be done recursively for all pattern positions, though.

mfelleisen commented 2 years ago

That's the conclusion I have come to. @samth : should I reassign this to racket/match for now?

samth commented 2 years ago

What's wrong with (list 1) as a pattern?

sorawee commented 2 years ago

It doesn't follow the grammar in https://docs.racket-lang.org/teachpack/2htdpabstraction.html#%28part._x._matching%29

mfelleisen commented 2 years ago

I think Oak meant this:

#lang racket

(struct list [])

(match '(1)
  [(list 1) 2])
mfelleisen commented 2 years ago

There are two separate issues here:

  1. racket/match is, well, confusing with its parser (see list example).
  2. 2htdp/abstraction is perhaps not pedagogical enough. @sorawee (list 1) is an abbreviation of (cons 1 empty) in HtDP/2e.
samth commented 2 years ago

That match doesn't use the bindings of its keywords for matching is long standing mistake, but not something that's going to change at this point -- it would break too much code. Writing a syntax class to enforce the grammar in the documentation for 2htdp/abstraction should be fairly straightforward.

mfelleisen commented 2 years ago

OK, I'll just focus on my match. (I now wonder whether @michaelballantyne 's hosted DSLs wouldn't be better for something like match.)

rfindler commented 2 years ago

Another approach would be to refactor matchs implementation to parameterize over the "is it a keyword?" predicate, providing some (awkwardly named) version of match that Matthias could use to implement the teaching language version?

samth commented 2 years ago

I think even that version would require preprocessing. For example, cons has a different binding in the teaching languages, so it wouldn't be recognized by match, similarly for list.

mfelleisen commented 2 years ago

Enforcing the grammar isn't going to help, if I then trampoline into your match. Your parser would still deal with (var x:id) and impose its now interpretation. The best I can do is write my own match -- as if it were 1984 again. If so, reusability of PL design (for LOP) is questioned here. (This may point to a larger problem than I thought at first.)

sorawee commented 2 years ago

I submitted a draft PR that fixes this issue: #172.