Issue 5344: Avoid repeated calculation of accepted contexts (issue 346080043 by address@hidden)

[dak]

https://codereview.appspot.com/346080043/diff/1/lily/acceptance-set.cc
File lily/acceptance-set.cc (right):

https://codereview.appspot.com/346080043/diff/1/lily/acceptance-set.cc#newcode34
lily/acceptance-set.cc:34: if (!scm_is_null (default_))
Why do you prioritize the default child?

The only purpose of the defaultchild is to provide a default path for
reaching a Bottom context via the creation of implicit contexts.  You
are mashing up several semantics here in something that feels like a
handwavy "do-what-I-mean" scheme rather than a well-confined notion of
defaultchild.  I actually invested quite a bit of work to get rid of
that so I'd like to know what advantage you see in adding additional
semantics to defaultchild.

https://codereview.appspot.com/346080043/diff/1/lily/acceptance-set.cc#newcode50
lily/acceptance-set.cc:50: accepted_ = scm_cons(item, scm_delete_x
(item, accepted_));
I'm not overly happy with this kind of O(n^2) behavior.  Most of the
context definitions will be done in engraver-init.ly where duplication
is not an issue because it has been taken care of manually.

https://codereview.appspot.com/346080043/diff/1/lily/context-def.cc
File lily/context-def.cc (right):

https://codereview.appspot.com/346080043/diff/1/lily/context-def.cc#newcode330
lily/context-def.cc:330: Acceptance_set& acc = context->acceptance_;
This is a bloody mess.  You take the original list without copying and
call acc.accept and acc.deny and similar on it, operations which use
scm_delete_x and similar for _destructively_ changing the lists in
question.  Previously, \accept/\denies/\defaultchild for better or worse
were not interpreted in \with blocks (which is where ops comes from if I
remember correctly).

So you do several changes in semantics in a combination that wreaks
havoc and lets \with blocks destructively change global context defs.

Very much a showstopper.

https://codereview.appspot.com/346080043/