Re: [Lilypond-auto] Issue 2990 in lilypond: \RemoveEmptyStaves in StaffGroup context crashes

[lilypond]

Comment #17 on issue 2990 by address@hidden: \RemoveEmptyStaves in  
StaffGroup context crashes
http://code.google.com/p/lilypond/issues/detail?id=2990

You are right.

There are other places in the code base that element lists are recursed  
through (Axis_group_interface::generic_bound_extent, for example).  One  
could make the code segfault there as well.  I don't think it is a good  
idea to write checks for cyclical dependencies in the elements list every  
time they are recursed through.  That would be like writing checks for  
cyclical dependencies every time a property is looked up.  I think it is  
important to have one central place where this is nipped in the bud.  Here,  
in my proposed patch, it is pointer-group-interface.cc.

With grob dependencies, when a circular dependency is found in _any_  
situation, we issue a programming error and return a dummy value.  This  
takes place in one function instead of in several places where cyclic  
dependcies could occur.  That is _exactly_ what I am trying to do here. I  
want to flag cyclical element structures in one function instead of in  
several.

I believe we should also check for cyclical parenting.

This patch is the opposite of "scattering everywhere."  Coming up with  
local solutions to a deeper problem, like what is currently being done in  
Axis_group_engraver, is a bandaid.  This patch addresses the fundamental  
issue - that grobs should not be elements of themselves on any level.

Because my patch issues a programming error at the moment the cyclical  
dependency tries to be created, I think it makes it very clear where what  
is happening where.  It takes the instance protected against in  
Axis_group_engraver and generalizes it to all situations.

I'm getting a 0.08% slow down on average of LilyPond from this patch when  
run on large scores.