Re: issue #5341: Urgent corrections to stencil-integral.cc (skylines) (issue 341350043 by address@hidden)

[dak]

https://codereview.appspot.com/341350043/diff/1/lily/stencil-integral.cc
File lily/stencil-integral.cc (right):

https://codereview.appspot.com/341350043/diff/1/lily/stencil-integral.cc#newcode1132
lily/stencil-integral.cc:1132: SCM new_s = s->smobbed_copy ();
This does not work: the compiler will optimize new_s away unless you
write scm_remember_upto_here_1 (new_s) at a proper later point of time
(after last use).

However, cough cough, all this is an unnecessary complication.  Sorry
for overlooking this.  Just write

Stencil q = *s;

instead of allocating the stencil in some gc-accessible area (of course
then also using q.rotate_degrees and q.expr ()).  It's a feature of
"simple smobs" that they contain all of their referenced SCM values in
the structure itself and so can be used as normal automatic variable.
You don't want a "smobbed copy" here, an ordinary (and unsmobbed) copy
will work fine and save you all the trouble of having to cater for
garbage collection: the stack is automatically scanned.

Of course this still suffers from the whole
"Transform_matrix_and_expression" thing being crap since it does not
protect the contained SCM expression, and creating a vector of it is
particularly bad mojo since then the data is outside of the stack and no
longer scanned for SCM values.  The old stencil expression likely got
away by being protected in the original data it came from.  Your rotated
stencil does not have that luxury.

So basically the old code tended to usually work by accident and your
code does not have the luxury to continue working by accident.

I'm afraid I'll have to take a much more thorough look at this shit show
and rework its data structures.  I suggest you remove this change (which
would not even help due to the lack of scm_remember_upto_here_1) from
this patch set, retaining the cosmetic parts, and I'll try coming up
with something that has a chance of working regarding the GC parts.

https://codereview.appspot.com/341350043/