Re: Issue 4365: non-member is_smob<>(), is_derived_smob<>(), etc. (issue 231680043 by address@hidden)

[nine . fierce . ballads]

On 2015/05/13 11:20:54, dak wrote:
> My gut feeling is that it does not make sense to maintain separate
unsmob<T> and
> derived_unsmob<T> functions:  derived_unsmob<T> made sense in contrast
to
> T::unsmob (which could have produced a base unsmob in contrast).

I'll revert it to Base::unsmob and derived_unsmob<Derived>.

> lily/include/smobs.hh:163: typedef Super super_type;
> That one's a nuisance.  Is it really necessary?

Well, there is a difference in gcc's errors when is_smob<Derived> is
tried.

When is_smob<T> is implemented as
    return Smob_base<T>::is_base_smob (s);
then the (ridiculous, voluminous) errors consist of reasons that
Smob_base<T> can not be instantiated.

When is_smob<T> is implemented as
    return Smob_base<typename T::super_type>::is_base_smob (s);
then the errors refer to this line and say that is_base_smob is private.
Since this line is introduced by a "this intentionally fails" comment,
I prefer this way, but not enough to fight for it.

I'm planning to leave this as it is until you confirm that you want me
to
remove the typedef.

>    return T::is_smob (s) && derived_unsmob<T> (s);

> here (assuming proper friendship) and bypass the dynamic_cast when the
first
> condition is false.

dynamic_cast<T>(0) seems to be pretty fast so I think we should just
stick with
the simpler implementation.  I timed both implementations, twice
compiling a
single large score, and twice compiling a bunch of smaller scores, and I
could
draw no conclusion from the results.

> lily/include/smobs.tcc:159: ly_add_type_predicate ((void *)
is_base_smob,
> smob_name_.c_str ());
> Why use is_base_smob here instead of is_smob<Super> ?  The whole point
of
> ly_add_type_predicate is to be able to identify type check functions
by address,
> and the address used in code elsewhere will be that of is_smob<Super>,
won't it?

I'll change it.  Thanks for the review.

https://codereview.appspot.com/231680043/