[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Creates pure closures (issue 4894052)
From: |
Mike Solomon |
Subject: |
Re: Creates pure closures (issue 4894052) |
Date: |
Sat, 20 Aug 2011 22:21:28 +0200 |
On Aug 20, 2011, at 12:39 AM, address@hidden wrote:
>
> http://codereview.appspot.com/4894052/diff/9001/input/regression/pure-closure.ly
> File input/regression/pure-closure.ly (right):
>
> http://codereview.appspot.com/4894052/diff/9001/input/regression/pure-closure.ly#newcode18
> input/regression/pure-closure.ly:18: #(ly:make-pure-closure
> ly:stem::height '(-3 . 10))
> I'm a bit worried this is too close to an extra-spacing-height override
> to make a good test example.
>
I'll try to think of something better...if you have any suggestions in the
meantime, they're certainly welcome!
> http://codereview.appspot.com/4894052/diff/9001/lily/pure-closure.cc
> File lily/pure-closure.cc (right):
>
> http://codereview.appspot.com/4894052/diff/9001/lily/pure-closure.cc#newcode36
> lily/pure-closure.cc:36: return (SCM) SCM_CELL_WORD_1 (smob);
> SCM_PACK (SCM_CELL_WORD_1 (smob));
>
> (if you want to be strict :)
I have no clue how these SCM functions work, but I'll take your word for it!
>
> http://codereview.appspot.com/4894052/diff/9001/lily/pure-closure.cc#newcode43
> lily/pure-closure.cc:43: return (SCM) SCM_CELL_WORD_2 (smob);
> SCM_PACK (SCM_CELL_WORD_2 (smob));
>
Ditto.
> http://codereview.appspot.com/4894052/diff/9001/lily/pure-closure.cc#newcode60
> lily/pure-closure.cc:60: SCM_NEWSMOB2 (z, pure_closure_tag, unpure,
> pure);
> SCM_UNPACK (unpure), SCM_UNPACK (pure)
>
Done.
> http://codereview.appspot.com/4894052/diff/9001/lily/pure-closure.cc#newcode68
> lily/pure-closure.cc:68: assert (is_pure_closure (pc));
> optimized builds will segfault on invalid args, so you should use
> LY_ASSERT_TYPE () here
>
Done.
> http://codereview.appspot.com/4894052/diff/9001/lily/pure-closure.cc#newcode76
> lily/pure-closure.cc:76: assert (is_pure_closure (pc));
> LY_ASSERT_TYPE ()
>
Done.
> http://codereview.appspot.com/4894052/diff/9001/scm/define-grobs.scm
> File scm/define-grobs.scm (left):
>
> http://codereview.appspot.com/4894052/diff/9001/scm/define-grobs.scm#oldcode2655
> scm/define-grobs.scm:2655: (if (not (procedure? unpure))
> I think you want this instead:
>
> (let ((pure (ly:pure-closure-pure-part unpure)))
> (if (not (procedure? pure))
> pure
> (apply pure
> (append (list (car args) start end) (cdr args)))))
>
I've added a hideous code dup to cover all cases. This will be attenuated and
removed if I can find the time to move all of LilyPond's pure code over to
unpure-pure-containers.
New patchset up.
Thanks Neil!
Cheers,
MS
- Creates pure closures (issue 4894052), mtsolo, 2011/08/18
- Re: Creates pure closures (issue 4894052), n . puttock, 2011/08/19
- Re: Creates pure closures (issue 4894052),
Mike Solomon <=
- Re: Creates pure closures (issue 4894052), pkx166h, 2011/08/27
- Re: Creates pure closures (issue 4894052), n . puttock, 2011/08/31
- Re: Creates pure closures (issue 4894052), n . puttock, 2011/08/31