Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(224)

Issue 6057044: Part_combine_iterator::derived_mark: don't abort marking prematurely. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 2 months ago by dak
Modified:
7 years, 2 months ago
CC:
lilypond-devel_gnu.org
Base URL:
http://git.savannah.gnu.org/gitweb/?p=lilypond.git/trunk/
Visibility:
Public.

Description

Part_combine_iterator::derived_mark: don't abort marking prematurely.

Patch Set 1 #

Patch Set 2 : Sc...ip the complexity. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+8 lines, -13 lines) Patch
M lily/part-combine-iterator.cc View 1 1 chunk +8 lines, -13 lines 0 comments Download

Messages

Total messages: 6
Graham Percival
LGTM and can be pushed right now. But for my general ignorance: why not just ...
7 years, 2 months ago (2012-04-17 20:21:14 UTC) #1
dak
On 2012/04/17 20:21:14, Graham Percival wrote: > LGTM and can be pushed right now. > ...
7 years, 2 months ago (2012-04-18 04:39:20 UTC) #2
dak
On 2012/04/18 04:39:20, dak wrote: > One could instead use a static array with pointers-to-member ...
7 years, 2 months ago (2012-04-18 05:49:01 UTC) #3
dak
On 2012/04/18 05:49:01, dak wrote: > On 2012/04/18 04:39:20, dak wrote: > > One could ...
7 years, 2 months ago (2012-04-18 05:51:23 UTC) #4
dak
Short of any protests, I think I'll be going with void Part_combine_iterator::derived_mark () const { ...
7 years, 2 months ago (2012-04-18 07:56:39 UTC) #5
hanwenn
7 years, 2 months ago (2012-04-22 11:32:05 UTC) #6
On Wed, Apr 18, 2012 at 4:56 AM,  <dak@gnu.org> wrote:
> Short of any protests, I think I'll be going with
>
> void
> Part_combine_iterator::derived_mark () const
> {
>  if (first_iter_)
>    scm_gc_mark (first_iter_->self_scm ());
>  if (second_iter_)
>    scm_gc_mark (second_iter_->self_scm ());
>  if (unisono_event_)
>    scm_gc_mark (unisono_event_->self_scm ());
>  if (mmrest_event_)
>    scm_gc_mark (mmrest_event_->self_scm ());
>  if (solo_one_event_)
>    scm_gc_mark (solo_one_event_->self_scm ());
>  if (solo_two_event_)
>    scm_gc_mark (solo_two_event_->self_scm ());
> }
>
> All the rest is too smart for its own good.

FYI,  my experience is that writing this type of code involves cut &
paste, and it is easy to make errors like

 if (some_new_event_)
   mark(the_event_i_copied_it_from_)

I agree that 4 is borderline small enough not to use a loop.

-- 
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b