Re: Copy alist instead of deep copy on Grob clone (issue 561640045 by address@hidden)

[Han-Wen Nienhuys]

On Tue, Apr 14, 2020 at 9:28 AM <address@hidden> wrote:
>
> On 2020/04/14 07:24:10, hanwenn wrote:
> > On 2020/04/14 07:00:56, hahnjo wrote:
> > > On 2020/04/13 21:01:11, hanwenn wrote:
> > > > it's hard to say if this makes a measurable difference:
> > > >
> > > > [...]
> > >
> > > So then ... why do it at all? From a code perspective, a deep copy
> looks saner
> > > in terms of "encapsulation" (one principle of object-oriented
> programming). If
> > > there is no provable gain, I'm for leaving the current code alone
> until there
> > is
> > > good reason to change - "premature optimization is the root of all
> evil"
> >
> > As I described in the commit message, this code doesn't protect
> anything,
> > because get_property can also return values from the immutable list.
>
> get_property uses data backed by mutable_property_alist_, right?

Use the Source, Jonas!

https://github.com/lilypond/lilypond/blob/0c00cd98e81b27325bed5891b950fe7f0f0ebe5d/lily/grob-property.cc#L140

it reads from the immutable_list_ if there is no override in the
mutable property list.

If anything modifies an indirect value they get from get_property,
they break the entire formatting process in a grotesque way, because
they'd potentially be modifying shared state that is kept in
immutable_property_alist_.

-- 
Han-Wen Nienhuys - address@hidden - http://www.xs4all.nl/~hanwen