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

[dak]

https://codereview.appspot.com/231680043/diff/1/lily/include/smobs.hh
File lily/include/smobs.hh (right):

https://codereview.appspot.com/231680043/diff/1/lily/include/smobs.hh#newcode102
lily/include/smobs.hh:102: - unsmob<T> (SCM x) and derived_unsmob<T>
(SCM x) - unpack X and
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).

https://codereview.appspot.com/231680043/diff/1/lily/include/smobs.hh#newcode163
lily/include/smobs.hh:163: typedef Super super_type;
That one's a nuisance.  Is it really necessary?

https://codereview.appspot.com/231680043/diff/1/lily/include/smobs.hh#newcode267
lily/include/smobs.hh:267: return is_smob<T> (s) ?
T::super_type::unchecked_unsmob (s) : 0;
This will fail to compile for

  return is_smob<T> (s) ? T::unchecked_unsmob (s) : 0;

anyway in case of derivation because of the missing upcast on the return
type, won't it?  No need for the super_type here, right?

https://codereview.appspot.com/231680043/diff/1/lily/include/smobs.hh#newcode280
lily/include/smobs.hh:280: return derived_unsmob<T> (s);
You can still put

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

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

https://codereview.appspot.com/231680043/diff/1/lily/include/smobs.tcc
File lily/include/smobs.tcc (right):

https://codereview.appspot.com/231680043/diff/1/lily/include/smobs.tcc#newcode159
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?

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