lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Issue 5732: Use unique_ptr in layout code (issue 573500043 by addres


From: nine . fierce . ballads
Subject: Re: Issue 5732: Use unique_ptr in layout code (issue 573500043 by address@hidden)
Date: Mon, 03 Feb 2020 10:53:45 -0800

Thanks for the reviews, g


https://codereview.appspot.com/573500043/diff/561420043/lily/beam-quanting.cc
File lily/beam-quanting.cc (right):

https://codereview.appspot.com/573500043/diff/561420043/lily/beam-quanting.cc#newcode1049
lily/beam-quanting.cc:1049: configs.clear ();
On 2020/02/03 17:57:52, hahnjo wrote:
> I think you don't need this: If a vector goes out of scope, it will
call the
> destructor of all elements still present.

I didn't want to investigate the consequences of deferring deletion to
the end of the scope.

https://codereview.appspot.com/573500043/diff/561420043/lily/system-start-delimiter-engraver.cc
File lily/system-start-delimiter-engraver.cc (right):

https://codereview.appspot.com/573500043/diff/561420043/lily/system-start-delimiter-engraver.cc#newcode149
lily/system-start-delimiter-engraver.cc:149: (new Bracket_nesting_staff
(0)));
On 2020/02/03 17:57:52, hahnjo wrote:
> Can you check if
> children_.emplace_back (new Bracket_nesting_staff (0));
> works? This would be much neater

It compiles, but it raises its own flag to reviewers because it does not
show that the new object is managed by a smart pointer.

If I had C++14, I would have used
std::make_unique<Bracket_nesting_staff> (0) to avoid repeating the class
name.  We can define our own make_unique if we want to, but I don't
think it should be done in this patch.

https://codereview.appspot.com/573500043/



reply via email to

[Prev in Thread] Current Thread [Next in Thread]