[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: T1249 - Remove (define define-ly-syntax define-public). (issue231304
From: |
ianhulin44 |
Subject: |
Re: T1249 - Remove (define define-ly-syntax define-public). (issue2313044) |
Date: |
Sat, 09 Oct 2010 15:31:15 +0000 |
Reviewers: Patrick McCarty, Neil Puttock, hanwenn,
carl.d.sorensen_gmail.com,
Message:
On 2010/10/08 05:00:00, Patrick McCarty wrote:
http://codereview.appspot.com/2313044/diff/1/scm/ly-syntax-constructors.scm
File scm/ly-syntax-constructors.scm (left):
http://codereview.appspot.com/2313044/diff/1/scm/ly-syntax-constructors.scm#oldcode20
scm/ly-syntax-constructors.scm:20: (define define-ly-syntax
define-public)
Instead of removing this definition (like I did), would it be possible
to use a
macro here? Something like
(defmacro define-ly-syntax (args . body)
`(define-public ,args ,body))
This is untested.
It doesn't work with Guile 1.9.
It's precisely the macro parsing stuff that causes the problem. That's
why I originally thought we could use (define-syntax define-ly-syntax
(syntax-rules ()
((_ args ... ) (define-public ... ))))
This works on Guile 1.9 but fails on V1.8 when using (use-modules(ice-9
syncase))
So your 'slash and burn' solution looks the way to go.
http://codereview.appspot.com/2313044/diff/1/scm/ly-syntax-constructors.scm
File scm/ly-syntax-constructors.scm (right):
http://codereview.appspot.com/2313044/diff/1/scm/ly-syntax-constructors.scm#newcode27
scm/ly-syntax-constructors.scm:27: `(define-public ,args
I forgot why I changed this...
Is there a reason why "primitive-eval" was used here in the first
place?
I don't know, but removing the cruft works with both Guile versions, and
the regtests still run with Guile 1.8.7. Maybe it was a hangover from
backwards compatibility with V1.6? You'll have to ask a grown-up to get
the answer to this one.
http://codereview.appspot.com/2313044/diff/1/scm/ly-syntax-constructors.scm#newcode250
scm/ly-syntax-constructors.scm:250: (define-public (partial parser
location dur)
This part needs to be rebased against current master.
It was because I had re-based it thew up differences with your
prototype. However I can see there are some potential problems here.
Will investigate.
Cheers,
Ian
Description:
T1249 - Remove (define define-ly-syntax define-public).
This prevents a compilation error in Guile V1.9.
Replace calls to (primitive-val define-ly-syntax ...
with direct define-public declarations in the macros in this file.
Please review this at http://codereview.appspot.com/2313044/
Affected files:
M scm/ly-syntax-constructors.scm