Re: Creates pure closures (issue 4894052)

[Mike Solomon]

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