|
From: | lilypond |
Subject: | Re: [Lilypond-auto] Issue 2990 in lilypond: \RemoveEmptyStaves in StaffGroup context crashes |
Date: | Sat, 22 Dec 2012 11:37:01 +0000 |
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.
[Prev in Thread] | Current Thread | [Next in Thread] |