Comment #17 on issue 2990 by [email protected]: \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.
