Re: T1249 - Remove (define define-ly-syntax define-public). (issue2313044)

[ianhulin44]

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