Implement and use "derived_unsmob"

Changed commit message containing the fixed sed script:

    Issue 4377: Run a script for using derived_unsmob where obvious
    
    Script is:
    
    matched="[^()]*"
    for i in 1 2 3 4 5
    do
        matched="\\(($matched)\\|[^()]\\)*"
    done
    filelist="$(git grep -l '\<dynamic_cast[^>]*> ([_a-zA-Z]*::unsmob\>')"
    typelist="$(sed -n 's/^.*\<dynamic_cast\s*<\([_a-zA-Z]\+\)\s*\*> (\([_a-zA-Z]*\)::unsmob\>.*$/\1/p' $filelist | sort -u )"
    for typ in $typelist
    do
        # crazy: we have unsmob definitions that are not even used
        if git grep -q "$typ::unsmob" || git grep -q "\\<$typ"'\s*\*\s*unsmob\s*(SCM'
        then
        echo "There already is $typ::unsmob"
        sed -i '/unsmob (SCM/,/;/!s/\<dynamic_cast\s*<\('"$typ"'\)\s*\*> (\([_a-zA-Z]*\)::unsmob\s*(\('"$matched"'\))\s*)/\1::unsmob (\3)/g' $filelist
        else
        sed -i 's/\<dynamic_cast\s*<\('"$typ"'\)\s*\*> (\([_a-zA-Z]*\)::unsmob\s*(\('"$matched"'\))\s*)/derived_unsmob<\1> (\3)/g' $filelist
        fi
    done
    
    The awkward bit at the start of the script is for matching matched
    parentheses.  This replaces the construct
    
        dynamic_cast<T *>(xxx::unsmob (yyy))
    
    with
    
        derived_unsmob<T> (yyy)
    
    where appropriate.  If T::unsmob already exists, it is used
    instead (apart from inside of its own definition, of course).

[dak, 2015-05-08 14:15:24]

Script around recursive and preexisting definitions

[Dan Eble, 2015-05-08 22:50:32]

LGTM. This is indeed good to commit before issue 4365. Thanks.

[Dan Eble, 2015-05-08 23:10:24]

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

https://codereview.appspot.com/238020043/diff/20001/lily/include/smobs.hh#new...
lily/include/smobs.hh:240: inline T *derived_unsmob (SCM arg)
But don't you need "static" here?

[dak, 2015-05-09 05:29:29]

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

https://codereview.appspot.com/238020043/diff/20001/lily/include/smobs.hh#new...
lily/include/smobs.hh:240: inline T *derived_unsmob (SCM arg)
On 2015/05/08 23:10:24, Dan Eble wrote:
> But don't you need "static" here?

Uh, why?  If someone takes the address of the function, presumably for use in
function-documentation, why would I want this address to be different in every
compilation unit?

And it isn't a member function of anything, either: the other unsmobs have
"static" not because of static linkage but because they are member functions
without a this-pointer.

[dak, 2015-05-09 06:56:54]

On 2015/05/09 05:29:29, dak wrote:
> https://codereview.appspot.com/238020043/diff/20001/lily/include/smobs.hh
> File lily/include/smobs.hh (right):
> 
>
https://codereview.appspot.com/238020043/diff/20001/lily/include/smobs.hh#new...
> lily/include/smobs.hh:240: inline T *derived_unsmob (SCM arg)
> On 2015/05/08 23:10:24, Dan Eble wrote:
> > But don't you need "static" here?
> 
> Uh, why?  If someone takes the address of the function, presumably for use in
> function-documentation, why would I want this address to be different in every
> compilation unit?
> 
> And it isn't a member function of anything, either: the other unsmobs have
> "static" not because of static linkage but because they are member functions
> without a this-pointer.

I think I might understand the problem you have with this line: the "functions
may only be defined in one compilation unit" decree.  But these days, pretty
much the sole meaning of "inline" (which is implied for functions defined inside
of a class definition but here we are outside) is "tell the linker to put this
into a named common block so that identical definitions in separate compilation
units are merged" (well, linkers are smarter than that these days, but that's
what it meant when the BLOCK DATA SUBROUTINE requirement for named common block
initializations went away).

So "inline" is not as much an optimization instruction (GCC makes its own
decisions about that) but rather "may be defined in more than one compilation
unit, please merge".

It may well be that you go now "come on dude, I knew *that*".  But I really have
difficulties seeing what else you might have meant.

[Dan Eble, 2015-05-09 13:25:11]

On 2015/05/09 06:56:54, dak wrote:
> I think I might understand the problem you have with this line: the "functions
> may only be defined in one compilation unit" decree.  But these days, pretty

That's it.

> So "inline" is not as much an optimization instruction (GCC makes its own
> decisions about that) but rather "may be defined in more than one compilation
> unit, please merge".

Yes, I knew that "inline" is just a request, but I vaguely recall that leaving
off "static" has caused problems in my professional work (in the past few
years?).  Could be my memory is faulty.   If you're not worried about it in this
context, then I won't either.

[dak, 2015-05-09 20:38:11]

On 2015/05/09 13:25:11, Dan Eble wrote:
> On 2015/05/09 06:56:54, dak wrote:
> > I think I might understand the problem you have with this line: the
"functions
> > may only be defined in one compilation unit" decree.  But these days, pretty
> 
> That's it.
> 
> > So "inline" is not as much an optimization instruction (GCC makes its own
> > decisions about that) but rather "may be defined in more than one
compilation
> > unit, please merge".
> 
> Yes, I knew that "inline" is just a request, but I vaguely recall that leaving
> off "static" has caused problems in my professional work (in the past few
> years?).  Could be my memory is faulty.   If you're not worried about it in
this
> context, then I won't either.

Without "inline", you will indeed get compilation errors precisely because of
the "one definition only" rule.  In that respect, "inline" is _not_ just a
request.  The difference between static inline and inline is that the former
will, if you take the address of the function, generate one callable function
per compilation unit while the latter will only generate one callable function
across the whole compilation.  In either case (and actually even when inline is
left off) the compiler may feel free to not actually call that function but
rather substitute it inline: only any explicitly taken function pointer is
guaranteed to be associated with a callable version of the function.

Removing the "inline" here will get you the errors at link time you probably
were expecting.

So in short: the declaration is very much intentional.  Adding "static" here
would work also but cause problems in case we start adding those kind of
function pointers to function-documentation.cc.