Re: crash moving DotColumn

[David Kastrup]

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.
> }

Stock 2.18.2 from Ubuntu:

-*- mode: compilation; default-directory: "/tmp/" -*-
Compilation started at Thu May 12 15:02:35

/usr/bin/lilypond crash.ly 
GNU LilyPond 2.18.2
Processing `crash.ly'
Parsing...
crash.ly:1: warning: no \version statement found, please add

\version "2.18.2"

for future compatibility
Interpreting music...
Preprocessing graphical objects...lilypond: grob-property.cc:248: 
scm_unused_struct* Grob::try_callback_on_alist(scm_unused_struct**, SCM, SCM): 
Assertion `value == ((SCM) ((((4)) << 8) + scm_tc8_flag)) || value == marker' 
failed.

Compilation aborted (core dumped) at Thu May 12 15:03:04

Wow, they compiled with options enabling assertions.  In principle
that's good, but I fear it might have been done using
--disable-optimising.

> With a build from latest master I get:
>
> lilypond: /home/hermann/lilypond-git/lily/grob-property.cc:242:
> scm_unused_struct* Grob::try_callback_on_alist(scm_unused_struct**,
> SCM, SCM): Assertion `scm_is_null (value) || scm_is_eq (value,
> marker)' failed.
> Aborted (core dumped)

Ok, I might focus on the wrong things here...  But do you notice that
the 2.18.2 assertion message at the top is much less readable because of
being macro-expanded?

That means that you are compiling with a rather current libc version.
One that has already has my fix for this assertion message expansion
issue.

> A bug?

It's not new, anyway.

> How to do it different?

Good question.  Wait.  When the call returned by a callback passed to
try_callback_on_alist is *unspecified*, then the assertion checks that
the original value is unset (or blocked by the current call).
Otherwise, it _caches_ the callback value.

The condition is rather obscure.  But the point appears to be that a
callback should _never_ return *unspecified* since that messes with the
caching.

So probably your callback is missing a final #t.

The assertion has been added with

commit 635bff94c5a920d5866be4ff16751d771a35b7da
Author: Han-Wen Nienhuys <address@hidden>
Date:   Thu Jan 4 21:39:34 2007 +0100

    allow callbacks on object values too. Init via meta field.

However, the value checked by the assertion is fetched via
internal_get_property rather than get_property_data (as it is now) in
that commit.  This has been changed in

commit a7646e7a00e1fe86e4f52123049f20fd28b48739
Author: Han-Wen Nienhuys <address@hidden>
Date:   Fri Jan 26 01:34:03 2007 +0100

    check for calculation-in-progress marker in get_property().

The problem we have after this commit is that get_property_data will
return a procedure if the original entry was a callback (which it will
be most of the time) and thus the assertion triggers.

I am surprised we are not seeing this more often.  So at any rate, that
assertion seems to be rather flaky.  We only reach it when a callback
returns *unspecified*, though.

I'm not sure what the expected behavior should be.

-- 
David Kastrup