LGTM and can be pushed right now. But for my general ignorance: why not just ...
11 years, 11 months ago
(2012-04-17 20:21:14 UTC)
#1
LGTM and can be pushed right now.
But for my general ignorance: why not just hard-code 4? I suppose that using
the sizeof() trick makes it a bit safer in case somebody adds something to ptrs,
but is there any other reason?
On 2012/04/17 20:21:14, Graham Percival wrote: > LGTM and can be pushed right now. > ...
11 years, 11 months ago
(2012-04-18 04:39:20 UTC)
#2
On 2012/04/17 20:21:14, Graham Percival wrote:
> LGTM and can be pushed right now.
>
> But for my general ignorance: why not just hard-code 4? I suppose that using
> the sizeof() trick makes it a bit safer in case somebody adds something to
ptrs,
> but is there any other reason?
No. It is just a common C idiom. Personally, I think it would make more sense
to just unfold the loop. Takes fewer lines and is clearer.
I just kept with the original style.
For an array-based style, an automatic array makes little sense over explicit
code. One could instead use a static array with pointers-to-member (basically
offsets), and then dereference them. In that case, a 0 entry would indeed work
as an ending mark.
But for 4 entries?
On 2012/04/18 04:39:20, dak wrote: > One could instead use a static array with pointers-to-member ...
11 years, 11 months ago
(2012-04-18 05:49:01 UTC)
#3
On 2012/04/18 04:39:20, dak wrote:
> One could instead use a static array with pointers-to-member (basically
> offsets), and then dereference them. In that case, a 0 entry would indeed work
> as an ending mark.
>
> But for 4 entries?
To illustrate:
static Stream_event * Part_combine_iterator::* ptrs[]
=
{
&Part_combine_iterator::unisono_event_,
&Part_combine_iterator::mmrest_event_,
&Part_combine_iterator::solo_two_event_,
&Part_combine_iterator::solo_one_event_,
0
};
for (int i = 0; ptrs[i]; i++)
if (ptrs[i])
scm_gc_mark ((this->*ptrs[i])->self_scm ());
On 2012/04/18 05:49:01, dak wrote: > On 2012/04/18 04:39:20, dak wrote: > > One could ...
11 years, 11 months ago
(2012-04-18 05:51:23 UTC)
#4
On 2012/04/18 05:49:01, dak wrote:
> On 2012/04/18 04:39:20, dak wrote:
> > One could instead use a static array with pointers-to-member (basically
> > offsets), and then dereference them. In that case, a 0 entry would indeed
work
> > as an ending mark.
> >
> > But for 4 entries?
>
> To illustrate:
>
> static Stream_event * Part_combine_iterator::* ptrs[]
> =
> {
> &Part_combine_iterator::unisono_event_,
> &Part_combine_iterator::mmrest_event_,
> &Part_combine_iterator::solo_two_event_,
> &Part_combine_iterator::solo_one_event_,
> 0
> };
> for (int i = 0; ptrs[i]; i++)
> if (ptrs[i])
Uh, if (this->*ptrs[i]) of course
> scm_gc_mark ((this->*ptrs[i])->self_scm ());
Short of any protests, I think I'll be going with void Part_combine_iterator::derived_mark () const { ...
11 years, 11 months ago
(2012-04-18 07:56:39 UTC)
#5
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.
On Wed, Apr 18, 2012 at 4:56 AM, <dak@gnu.org> wrote: > Short of any protests, ...
11 years, 11 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
Issue 6057044: Part_combine_iterator::derived_mark: don't abort marking prematurely.
(Closed)
Created 11 years, 11 months ago by dak
Modified 11 years, 11 months ago
Reviewers: Graham Percival, hanwenn
Base URL: http://git.savannah.gnu.org/gitweb/?p=lilypond.git/trunk/
Comments: 0