[C++-sig] static method implementation

[David Abrahams]

Nikolay Mladenov <nickm at sitius.com> writes:

> I am posting the diffs of my implementation of class_.static method.
> I'd appreciate any comments that you may have.
>
> Thanks a lot to David Abrahams, for pointing me in the right direction and,
> of course, for the amazing library!
>
> Regards,
>
> Nikolay
>
> Index: src/object/class.cpp
> ===================================================================
> RCS file: /cvsroot/boost/boost/libs/python/src/object/class.cpp,v
> retrieving revision 1.48
> diff -r1.48 class.cpp
> 442a443,454
>>   extern PyTypeObject function_type;
>>   void class_base::make_method_static(const char * method_name)
>>   {
>>         dict d(handle<>(borrowed(downcast<PyTypeObject>(this->ptr())->tp_dict)));
>>
>>         object method = (object)(d[method_name]);
>>
>>         this->attr(method_name) = object(handle<>(
>>             PyStaticMethod_New(expect_non_null(pytype_check(&function_type, method.ptr())) )
>>             ));
>>   }


It's good; maybe a little stylistically awkward.  Is pytype_check
really needed?  I think it's a little too strict.  Maybe you should
just check that the thing is callable.  You could consider:

   extern PyTypeObject function_type;
   void class_base::make_method_static(const char * method_name)
   {
        dict d(detail::borrowed_reference(downcast<PyTypeObject(this-ptr())->tp_dict));

        object method(d[method_name]);

        this-attr(method_name) = object(
                detail::new_reference(
                    PyStaticMethod_New(expect_non_null(callable_check(method.ptr())) )
                ));
   }

Of course you'd have to write callable_check.  I'm not sure mine is
much better than yours anyway.

Note that detail::[borrowed|new]_reference are implementation
details, but you're working on the implementation so it's OK!

> Index: src/object/function.cpp
> ===================================================================
> RCS file: /cvsroot/boost/boost/libs/python/src/object/function.cpp,v
> retrieving revision 1.26
> diff -r1.26 function.cpp
> 264a265,273
>>             else if (existing->ob_type == &PyStaticMethod_Type){
>>                 //this can probably be made a lot better
>>                 ::PyErr_Format(
>>                     PyExc_RuntimeError
>>                     , "Boost.Python - All overloads of \"%s\" must be exported before calling
> \"class_.staticmethod\""
>>                     , name_
>>                     );
>>                 throw_error_already_set();

I'm not sure this is technically neccessary, but I don't know if I
could improve it.  The behavior without it might just work anyway.

> Index: class.hpp
> ===================================================================
> RCS file: /cvsroot/boost/boost/boost/python/class.hpp,v
> retrieving revision 1.64
> diff -r1.64 class.hpp
> 354a355,359
>>     self& staticmethod(char const* name)
>>     {
>>         make_method_static(name);
>>         return *this;
>>     }
>
> Index: object/class.hpp
> ===================================================================
> RCS file: /cvsroot/boost/boost/boost/python/object/class.hpp,v
> retrieving revision 1.31
> diff -r1.31 class.hpp
> 50a51,52
>>
>>     void make_method_static(const char *method_name);

Looks like a damned fine patch!
Tests and documentation are still needed, of course.

-- 
                       David Abrahams
   dave at boost-consulting.com * http://www.boost-consulting.com
Boost support, enhancements, training, and commercial distribution