Changes necessary to get pmatch to work #47
Conversation
This is the equivalent of Racket's make-syntax-introducer and generate-temporaries.
This makes debugging much easier.
david-christiansen
changed the title
|
There's now a |
Some examples relied on this bug - it's now fixed. Fixes #48.
|
This exposed a strange bug that I had to fix as well :-) |
|
Do you think this is ready for a merge? |
If you want, but I haven't reviewed much of it yet. I multi-task, so the lack of new comments doesn't mean I'm done reviewing, it just means I got distracted by something else. I use GitHub's "Approve / Request Changes" button when I'm done. |
|
Sounds good! Just wasn't sure :-)
…On Sun, Apr 26, 2020, 20:53 Samuel Gélineau ***@***.***> wrote:
Do you think this is ready for a merge?
If you want, but I haven't reviewed much of it yet. I multi-task, so the
lack of new comments doesn't mean I'm done reviewing, it just means I got
distracted by something else. I use GitHub's "Approve / Request Changes"
button when I'm done.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#47 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA4FARLYG3AY2WOHGWHRY3ROT6R3ANCNFSM4MN2ECKA>
.
|
|
|
||
| (example (pmatch (zero) (else 't))) | ||
| (example (pmatch (zero) ((else x) 't))) | ||
| (example (pmatch (zero) (x 't))) |
There was a problem hiding this comment.
(else x) and x both match anything? why do we need both?
There was a problem hiding this comment.
I bet I'm going to accidentally write the pattern zero instead of (zero) all the time. only having ,x bind variables would be a lot less error-prone, I think (and would match the original algorithm)
There was a problem hiding this comment.
else unambiguously binds the variable x, while x alone may be a macro that expands to a pattern case using the normal case machinery.
As far as the comma goes, I am not a fan of the aesthetics of using an antiquotation operator for something that really isn't antiquotation...
There was a problem hiding this comment.
Just to be clear - x does not presently get to be a macro invocation, but this syntax at least does not rule that out!
| [ppat | ||
| (lambda (stx) | ||
| (syntax-case stx | ||
| [(list (_ tgt pat ks kf)) |
There was a problem hiding this comment.
earlier the continuation function was called fk and now it's called kf. which is it?
There was a problem hiding this comment.
To be pedantic, the earlier thing is a function, while this is a local name for a piece of code. But I'll call them all "kf".
examples/pmatch.kl
Outdated
| (syntax-case stxs | ||
| [() (none)] | ||
| [(cons e es) | ||
| (bind 'internal-x e (abstract es))]))) |
There was a problem hiding this comment.
Abstracted, none, and bind aren't used anywhere, I'm guessing this is a leftover from a previous implementation?
There was a problem hiding this comment.
Yep! Will delete.
| [(cons c cs) | ||
| (syntax-case c | ||
| [(list (pat rhs)) | ||
| (>>= (else-case pat) |
There was a problem hiding this comment.
Why is the else cases treated separately? I would have expected them to be handled by ppat, just like every other pattern.
There was a problem hiding this comment.
Else cases are only valid at the top of a given pattern, while ppat is invoked recursively.
examples/pmatch.kl
Outdated
| [(:: id-and-stx rest) | ||
| `(ppat ,(fst id-and-stx) ,(snd id-and-stx) ,(combine rest) ,kf)])) | ||
| (pure `(case ,tgt | ||
| [,(cons-list-syntax what (the Syntax (list->syntax temp-names pat)) pat) |
There was a problem hiding this comment.
What is the purpose of the the Syntax type annotation? The name list->syntax already communicates quite clearly that it is returning a Syntax. I am guessing that this is leftover from a previous debugging session?
There was a problem hiding this comment.
On that note, I had completely forgotten that Klister was a typed language while reading this code. How about defining versions of define, let and flet which encourages including a type signature?
| deriving (Eq, Show) | ||
|
|
||
| instance Show MacroAction where | ||
| show _ = "MacroAction..." |
There was a problem hiding this comment.
Meh, if these Show instances aren't going to give any information, should we even bother giving them a Show instance at all?
There was a problem hiding this comment.
Because other things that include a MacroAction do have useful Show instances that are derived.
This is the equivalent of Racket/Scheme's make-syntax-introducer and generate-temporaries.