Re: Design flaw in Rest_collision

[Keith OHara]

On Mon, 05 Nov 2012 21:52:15 -0800, address@hidden <address@hidden> wrote:

> On 6 nov. 2012, at 04:51, Keith OHara <address@hidden> wrote:
>
> > Just to be clear, rest-collision.cc breaks the circular dependency by setting
> > positioning-done := true,
>
> I'm still not sure how this breaks the dependency.  If I look up the extent of 
> a rest, this will set 'Y-extent to 'calculation-in-progress for the rest.  
> Then, when 'Y-extent is read again in F, it will still be 
> 'calculation-in-progress.  Setting 'positioning-done to #t only prevents 
> calc_positioning_done by being called multiple times (say by other rests) - it 
> doesn't fill in the 'calculation-in-progress for 'Y-extent.  Or am I missing 
> something?

Setting 'positioning-done to #t prevents calc_positioning_done() being called 
repeatedly for the same rest in an endless cycle; that is, it breaks the cycle 
of dependency.

If the process begins with a lookup of Y-extent, then I follow what you say on 
how the chain of dependencies could lookup the same Y-extent once again, before 
reaching the end of the data-dependency chain.  The first lookup of Y-extent 
sets a trap intended to detect and stop cyclic dependencies, while the second 
lookup activates the trap.

In this case there would be no further lookups of Y-extent on the same rest, so 
we could consider this a false-positive activation of the trap.  It is probably 
wisest to keep the simple trap, even if it can have false-positives. (I argued 
before that the calculation-in-progress message should be a warning, not error.)

Do you have input that generates the "calculation-in-progress encountered..." 
message for rests?

Maybe you can arrange to have Rest_collision::force_shift_callback_rest() 
called before looking up the Y-extent of a rest.

Or, maybe we should change calc_positioning_done() to call directly 
Rest::generic_extent_callback() rather than going through the symbol lookup.  
This way, the C code would assume responsibility for preventing endless data 
dependency chains.  Your earlier suggestion about having 
Rest::generic_extent_callback() consistently ignore ledgers should work fine.
> line 249 of rest.cc [...]
>   SCM m = brew_internal_stencil (me, false);
> and then run make check.