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

[David Kastrup]

Dan Eble <address@hidden> writes:

> On May 14, 2015, at 01:07 , address@hidden wrote:
>
>> The disadvantage with that is that we are going to sacrifice the
>> performance advantage of is_smob over unsmob for "base smobs".  The
>> difference is not tied to the redundant dynamic_cast but rather to
>> having to fetch the pointer at all (going through unsmob checks _both_
>> the type of the smob as well as its pointer being non-zero.  The latter
>> is always the case but the compiler does not know that).
>
> Some template trickery could be used to select the more efficient
> implementation for the type.  I’ll leave a summary comment in the new
> unsmob<>.

Template trickery tends to be a maintenance pig.

>> I also seem to remember using is_smob+unsmob (typical for use of
>> LY_ASSERT_SMOB) only fetched the type once whereas unsmob+unsmob fetched
>> type, pointer, type, pointer.  Maybe we should create some version of
>> LY_ASSERT_SMOB leaving the fetched pointer in a variable.
>
> Something like this?
>
> template <class T>
> inline T* noisy_unsmob (la la la)
> {
>   // test here; return smob
> }
>
> #define LY_ASSERT_SMOB(klass, var, number) noisy_unsmob<klass> (var,
> number, __FUNCTION__)

I did not think of an actual return (and noisy_unsmob seems like an
absurd name, it's more like an assertive_unsmob or so, but it's not like
the name would be overly visible anyway) but that might actually be a
useful way of implementing it.  So one would do something like

Engraver *e = LY_ASSERT_SMOB(Engraver, eng, 1);

My idea would have been something like

LY_ASSERT_SMOB(Engraver, e, eng, 1)

but then it's not really clear whether you want to create e or only
assign to it.  Your proposal is probably quite cleaner and clearer and
it would seem backward-compatible (unless we get obnoxious warnings
about discarded return values).

>> After all, that's the most frequent use, and then this consideration
>> of avoiding a double unsmob would be moot, again making maintaining a
>> separate is_smob in addition to a generic unsmob less attractive.
>
> So in conclusion,
> * use just unsmob<T> -- no is_smob<T> or derived_...<T>
> * treat LY_ASSERT_SMOB specially for performance (test it)
> * create a ticket suggesting a return value for LY_ASSERT_SMOB

Unless GCC breaks out into warnings, one can probably just add the
return value right away.  And then make it a big ticket to actually make
use of it everywhere.  That will likely not work just with sed patterns.

> Let me know if I got that wrong.  Thanks.

Looks ok.

-- 
David Kastrup