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

Ensure posn also works as pattern in BSL+ #16

Closed
wants to merge 1 commit into from

Conversation

Blaisorblade
Copy link
Contributor

Commit 78ecfc5 didn't alter BSL+ (Beginning Student with List Abbreviations). I tested this change locally on Racket 6.2.1, and the affected code is identical, but please retest before merging.

Failing example in BSL+:

(require 2htdp/abstraction)
(define (f x)
    (match x
      [7 8]
      ["hey" "joe"]
      [(list 1 y 3) y]
      [(cons a (list 5 6)) (add1 a)]
      [(posn 5 5) 42]
      [(posn y y) y]
      [(posn y z) (+ y z)]
      [(cons (posn 1 z) y) z]))

This leads to: match: syntax error in pattern in: (posn 5 5).

Commit 78ecfc5 didn't alter BSL+ (Beginning Student with List Abbreviations). I tested this change locally on Racket 6.2.1, and the affected code is identical, but please retest before merging.
@Blaisorblade
Copy link
Contributor Author

To clarify: this is about valid BSL code failing in BSL+.

@samth
Copy link
Sponsor Member

samth commented Dec 10, 2015

I think this is ok to merge, but I'm not the expert here. @rfindler @mfelleisen ?

@rfindler
Copy link
Member

This looks reasonable to me, but perhaps a test case is in order?

@Blaisorblade
Copy link
Contributor Author

perhaps a test case is in order?

Good idea. The original commit adds a test for ISL in abstraction-use.rkt; it seems we'd want a section of that file for both BSL and BSL+.
(To avoid misunderstandings: I expect you might be quicker to do this yourself, than to wait for me to figure out Racket's testsuite).

78ecfc5#diff-f29ae2fe358f9044277d9fd49eafdde5

BTW, I'd have wanted to reuse the import from BSL, as for the rest, through all-from beginner:, but wasn't sure how to do that.

@stamourv
Copy link
Contributor

Merged, with test. Thanks!

@stamourv stamourv closed this Dec 10, 2015
@Blaisorblade Blaisorblade deleted the patch-1 branch December 10, 2015 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants