Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2022)

Issue 42190043: grob-property: if callback is independent of layout, call just once

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 4 months ago by Keith
Modified:
10 years, 4 months ago
Reviewers:
MikeSol, dak, mike7
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

grob-property: if callback is independent of layout, call just once

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+22 lines, -1 line) Patch
M lily/grob-property.cc View 1 chunk +10 lines, -1 line 0 comments Download
M lily/include/unpure-pure-container.hh View 1 chunk +1 line, -0 lines 0 comments Download
M lily/unpure-pure-container.cc View 1 chunk +11 lines, -0 lines 2 comments Download

Messages

Total messages: 3
MikeSol
https://codereview.appspot.com/42190043/diff/1/lily/unpure-pure-container.cc File lily/unpure-pure-container.cc (right): https://codereview.appspot.com/42190043/diff/1/lily/unpure-pure-container.cc#newcode45 lily/unpure-pure-container.cc:45: Looking good. The only thing is - how do ...
10 years, 4 months ago (2013-12-14 09:47:33 UTC) #1
dak
https://codereview.appspot.com/42190043/diff/1/lily/unpure-pure-container.cc File lily/unpure-pure-container.cc (right): https://codereview.appspot.com/42190043/diff/1/lily/unpure-pure-container.cc#newcode45 lily/unpure-pure-container.cc:45: On 2013/12/14 09:47:33, MikeSol wrote: > Looking good. > ...
10 years, 4 months ago (2013-12-14 10:00:43 UTC) #2
mike7
10 years, 4 months ago (2013-12-14 10:09:21 UTC) #3
On Dec 14, 2013, at 12:00 PM, dak@gnu.org wrote:

> 
> https://codereview.appspot.com/42190043/diff/1/lily/unpure-pure-container.cc
> File lily/unpure-pure-container.cc (right):
> 
>
https://codereview.appspot.com/42190043/diff/1/lily/unpure-pure-container.cc#...
> lily/unpure-pure-container.cc:45:
> On 2013/12/14 09:47:33, MikeSol wrote:
>> Looking good.
> 
>> The only thing is - how do we verify that this actually ignores start
> and end?
> 
> The function does not get passed start and end, so it has little choice
> but to ignore them.
> 
>> For example, let's say that there is a scheme function with optional
> arguments
>> for start and end that is used as the pure and unpure function.
> 
> You are confused.  If the second container element is _unbound_, which
> is what is being checked here, the function in the first container will
> _always_ be called without begin and end arguments.

Ah, ok, didn’t know that.

> 
>> It'll evaluate
>> just fine for unpure cuz begin and end are optional.  But begin and
> end may be
>> useful when it is pure.  If this is the only function passed to the
> constructor,
>> it will return true for the function above and yet perhaps be an
> operation that
>> we don't want to cache.
> 
>> Conversely, it is possible to do:
> 
>> (ly:make-unpure-pure-container foo foo)
> 
>> that should be cached but it would return false for this function.
> 
> Well, but in this case the function foo will be called without begin/end
> for the pure case and _with_ begin/end for the unpure case.  Different
> thing.
> 
>> So, my proposal would be:
> 
>> 1) if there is a single function, verify that it takes only one
>> argument
> 
> Doesn't matter.  It will always get only one argument.

Ok.

> 
>> 2) if there are two functions, verify that the second takes only one
> argument
> 
>> All of this is of course predicated, like I said above, on being able
> to check
>> the number of arguments a scheme function takes, which may or may not
> be
>> possible...
> 
> I would try to avoid this: the fine details here differ significantly
> between GUILEv1 and GUILEv2, and it does not appear that you can rely on
> this information being available at all or at least with the same
> mechanism for functions defined in C in GUILEv2.  So there is a large
> danger that we would be painting us into a corner here.
> 
> Once can, of course, call the function with three arguments and trap an
> error (if one occurs).  But error trapping is not likely efficient.
> 

Sounds like more trouble than it’s worth.
LGTM then.

Cheers,
MS
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b