Re: Checks for recursive element behavior (issue 6943072)

[address@hidden]

On 22 déc. 2012, at 09:46, address@hidden wrote:

> On 2012/12/22 08:29:59, mike7 wrote:
>> Where in the new code are things being allocated and thrown away
> temporarily on
>> the heap?
> 
> What do you think extract_grob_set does?

Got it.

> 
>> If there is a more efficient way to implement this patch, let me know.
> It
>> seems, though, that the only way to prevent grobs being elements of
> other grobs
>> is to prevent grobs from being added to elements lists à la this
> patch.
> 
> That is rubbish.  We can't add checks for any hypothetical programming
> error that can't come about as user error.

True.

> Here we have one situation,
> namely axis groups, where there is a possibility of user error.  So I
> proposed setting a context property when having an axis group engraver.

It sounds like you're mixing together "axis-groups" and "Axis group engraver." 
The axis group engraver creates VerticalAxisGroups, which implement the 
axis-group-interface.  There are, however, numerous grobs that implement the 
axis-group-interface that are not created in the Axis_group_engraver. To avoid 
future confusion, it may be worth renaming the engraver with a convert-ly rule.

My patch treats all grobs with an element array (meaning that they implement 
axis-group-interface).

> 
> If you think that adding something to an existing set twice is a
> fundamental problem, you can easily enough create a hashtable for each
> set and check before adding to it that the element is not a member of
> this set.

This is one manifestation of the problem.  The deeper problem is that if we 
have grobs A, B, C, and D and the following happens :

A adds B as an element.
B adds C
C adds D
D adds A

A hash structure cannot avoid this.

> 
> In this particular case, however, the problem is adding an axis group to
> an axis group.  _Any_ old axis group to _any_ old axis group.  The check
> for that is trivial: in the code adding a grob to an axis group, check
> that what you add is not an axis group (assuming that axis groups are
> top-level containers).

It seems like you're mixing up axis-group-interface with Axis_group_engraver.  
When you say _any_ old axis group, note that NoteColumn, Ambitus, and 
DynamicLineSpanner are all axis groups with element lists.  A user can make an 
Ambitus the element of a NoteColumn which is the element of a 
DynamicLineSpanner which is then the element of the Ambitus if she so chooses.  
This will result in a segfault when the elements lists are recursively iterated 
through.  My patch is intended to avoid this segfault by not allowing recursive 
nesting of elements in the elements list.

I am positive that your proposal would work in this specific case, but I also 
know that you advocate general, holistic, simple and "boring" solutions that 
cover many things.  I feel that my solution is that.  It seems that the only 
issue, as expressed before, is the efficiency one.  I will do benchmarks later 
today.

Cheers,
MS