[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Issue 4365: non-member is_smob<>(), is_derived_smob<>(), etc. (issue
From: |
nine . fierce . ballads |
Subject: |
Re: Issue 4365: non-member is_smob<>(), is_derived_smob<>(), etc. (issue 231680043 by address@hidden) |
Date: |
Thu, 14 May 2015 01:43:49 +0000 |
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/
- Re: Issue 4365: non-member is_smob<>(), is_derived_smob<>(), etc. (issue 231680043 by address@hidden), dak, 2015/05/13
- Re: Issue 4365: non-member is_smob<>(), is_derived_smob<>(), etc. (issue 231680043 by address@hidden), nine . fierce . ballads, 2015/05/13
- Re: Issue 4365: non-member is_smob<>(), is_derived_smob<>(), etc. (issue 231680043 by address@hidden),
nine . fierce . ballads <=
- Re: Issue 4365: non-member is_smob<>(), is_derived_smob<>(), etc. (issue 231680043 by address@hidden), dak, 2015/05/14
- Re: Issue 4365: non-member is_smob<>(), is_derived_smob<>(), etc. (issue 231680043 by address@hidden), dak, 2015/05/16
- Re: Issue 4365: non-member is_smob<>(), is_derived_smob<>(), etc. (issue 231680043 by address@hidden), nine . fierce . ballads, 2015/05/16
- Re: Issue 4365: non-member is_smob<>(), is_derived_smob<>(), etc. (issue 231680043 by address@hidden), dak, 2015/05/17