Re: crash moving DotColumn

[David Kastrup]

Thomas Morley <address@hidden> writes:

> 2016-05-12 15:51 GMT+02:00 David Kastrup <address@hidden>:
>> Thomas Morley <address@hidden> writes:
>>
>>> To get more space between NoteColumn and DotColumn the following used
>>> to work with 2.18.2
>>>
>>> foo =
>>> \override Score.DotColumn.before-line-breaking =
>>>   #(lambda (dot-column)
>>>     (ly:grob-set-property! dot-column 'positioning-done
>>>       (lambda (grob)
>>>         (ly:dot-column::calc-positioning-done grob)
>>>         (ly:grob-translate-axis! grob 15 X))))
>>>
>>>
>>> {
>>>   \foo
>>>   <gis' a' c''>4.
>>> }
>
> Thanks a lot for your analysis and findings.
>
> \version "2.19.42"
>
> foo =
> \override Score.DotColumn.before-line-breaking =
>   #(lambda (dot-column)
>     (ly:grob-set-property! dot-column 'positioning-done
>       (lambda (grob)
>         (ly:dot-column::calc-positioning-done grob)
>         (ly:grob-translate-axis! grob 15 X)
>         #t)))
>
> {
>   \foo
>   <gis' a' c''>4.
> }
>
> Works indeed.
> I could have sworn I tested this already...
>
> Can't think about further implications right now, too tired.

This is all sort-of shaky.

ly:dot-column::calc-positioning-done does


  /*
    Trigger note collision resolution first, since that may kill off
    dots when merging.
  */
  if (Grob *collision = unsmob<Grob> (me->get_object ("note-collision")))
    (void) collision->get_property ("positioning-done");

  me->set_property ("positioning-done", SCM_BOOL_T);
  [...]

so it rather explicitly sets positioning-done to #t early on instead of
waiting to return #t regularly (which it also does eventually),
presumably to avoid mutual recursion.

Now _your_ routine returns *unspecified* even _after_ positioning-done
has been explicitly set to #t in the callback.

This is quite explicitly the case that the assertion is supposed to
complain about.  Since obviously this is triggerable by user error (in
this case yours), maybe the assertion should be demoted to a programming
error.  I don't see that this can trigger except by a callback directly
(or indirectly) setting a value and then returning *unspecified*.  In
which case there does not seem the danger of recursion unless the
callback directly or indirectly reinstated itself.

So there does not seem to be much of a point in an assertion failure.
It does seem to constitute a programming error in general, however: a
callback should likely always return a specific value here in order to
let the caching mechanisms work properly.

So maybe always complain about value == SCM_UNSPECIFIED here but only on
programming_error level?

-- 
David Kastrup