Thread: [pygccxml-development] USE_CALLDEF_ORGANIZER
Brought to you by:
mbaas,
roman_yakovenko
Menu
▾
▴
pygccxml-development
|
From: Berserker <ber...@ho...> - 2010-01-27 13:51:57
|
As described in the documentation I have registration order problems :)
USE_CALLDEF_ORGANIZER seems the ideal solution to me but as far as
I understard it works only for functions with one param right? I have methods
like these:
class Foo
{
public:
void setAttribute(const std::string &name, bool value);
void setAttribute(const std::string &name, int value);
void setAttribute(const std::string &name, const std::string &value);
};
Would be easy to support situations like this?
_________________________________________________________________
25 Gygabite gratis online. Archivia e condividi i tuoi file!
http://www.windowslive.it/skyDrive.aspx |
|
From: Roman Y. <rom...@gm...> - 2010-01-27 14:59:22
|
On Wed, Jan 27, 2010 at 3:51 PM, Berserker <ber...@ho...> wrote:
> As described in the documentation I have registration order problems :)
> USE_CALLDEF_ORGANIZER seems the ideal solution to me but as far as
> I understard it works only for functions with one param right? I have
> methods
> like these:
> class Foo
> {
> public:
> void setAttribute(const std::string &name, bool value);
> void setAttribute(const std::string &name, int value);
> void setAttribute(const std::string &name, const std::string &value);
> };
>
> Would be easy to support situations like this?
It is not going to be too difficult. But, If I were you, I would not
go this way. I would drop "overloading" and gave the functions
different name.
The interface exposed to the user is fragile and could be easily
broken with addition/deletion of a function.
Let me know if you still want the change.
--
Roman Yakovenko
C++ Python language binding
http://www.language-binding.net/
|
|
From: Berserker <ber...@ho...> - 2010-01-27 15:19:21
|
> It is not going to be too difficult. But, If I were you, I would not
> go this way. I would drop "overloading" and gave the functions
> different name.
>
> The interface exposed to the user is fragile and could be easily
> broken with addition/deletion of a function.
>
> Let me know if you still want the change.
In my option one or more params doesn't make any difference when
someone choose to use USE_CALLDEF_ORGANIZER, but it's my opinion :)
I managed to write a working solution in ugly python code :D
class my_calldef_organizer_t(object):
def __init__( self ):
object.__init__( self )
#preserve order in which functions where defined
self.__cmp_unrelated = lambda d1, d2: cmp( d1.location.line, d2.location.line )
def __build_groups( self, decls ):
groups = { None: [] }
for d in decls:
# First patch here
if not isinstance( d, declarations.calldef_t ) or len( d.required_args ) == 0:
groups[ None ].append( d )
else:
if not groups.has_key( d.name ):
groups[ d.name ] = []
groups[ d.name ].append( d )
return groups
def __cmp_types( self, t1, t2 ):
return decl_wrappers.algorithm.registration_order.is_related( t1, t2 )
def __cmp( self, f1, f2 ):
# Second patch here
result = None
args = len(f1.arguments)
if args == len(f2.arguments):
i = 0
while i < args and result is None:
if not declarations.is_same(f1.arguments[i].type, f2.arguments[i].type):
result = self.__cmp_types(f1.arguments[i].type, f2.arguments[i].type)
i = i + 1
if result is None:
result = self.__cmp_unrelated( f1, f2 )
return result
def __sort_groups( self, groups ):
for group in groups.keys():
if None is group:
continue
groups[ group ].sort( self.__cmp )
def __join_groups( self, groups ):
decls = []
sorted_keys = groups.keys()
sorted_keys.sort()
for group in sorted_keys:
decls.extend( groups[group] )
return decls
def sort( self, decls ):
groups = self.__build_groups( decls )
self.__sort_groups(groups)
result = self.__join_groups(groups)
return result
def sort_calldefs_wrapper( decls ):
return my_calldef_organizer_t().sort(decls)
creators_factory.sort_algorithms.sort_calldefs = sort_calldefs_wrapper
I think it would make sense to apply this patch in svn.
Thank you as usual ;)
Bye
_________________________________________________________________
Sei bravo con le parole? Gioca su Typectionary
http://typectionary.it.msn.com/ |
|
From: Berserker <ber...@ho...> - 2010-01-27 15:49:13
|
There was a mistake in my previous post, Foo class must be:
class Foo
{
public:
void set(bool value);
void set(int value);
void set(const std::string &value);
void set(const Bar &value);
};
Sorry
_________________________________________________________________
Non sei a casa? Prova il nuovo Web Messenger
http://www.messenger.it/web/default.aspx |
|
From: Berserker <ber...@ho...> - 2010-01-27 15:45:14
|
> It is not going to be too difficult. But, If I were you, I would not
> go this way. I would drop "overloading" and gave the functions
> different name.
> The interface exposed to the user is fragile and could be easily
> broken with addition/deletion of a function.
> Let me know if you still want the change.
I have another issue related to this topic :)
Please consider this case:
C++ code
class Bar
{
public:
Bar(const std::string &str);
};
class Foo
{
public:
void set(const std::string &name, bool value);
void set(const std::string &name, int value);
void set(const std::string &name, const std::string &value);
void set(const std::string &name, const Bar &value);
};
Python code
f = Foo()
f.setAttribute("alfa")
The problem here is that boost::python calls the overload with
"Bar" as argument because is the last (available) registered.
What's your opinion about this? Shouldn't the registration order
be:
-> Foo::set(const std::string &name, const Bar &value);
-> Foo::set(const std::string &name, const std::string &value);
-> Foo::set(const std::string &name, int value);
-> Foo::set(const std::string &name, bool value);
As "general rule" Bar is dependent/related from std::string because
of it's implicit convertibility?
Maybe decl_wrappers.algorithm.registration_order.is_related must
be updated according to this rule.
_________________________________________________________________
Un mondo di personalizzazioni per Messenger, PC e cellulare
http://www.pimpit.it/ |
|
From: Roman Y. <rom...@gm...> - 2010-01-27 16:26:37
|
On Wed, Jan 27, 2010 at 5:45 PM, Berserker <ber...@ho...> wrote:
>> It is not going to be too difficult. But, If I were you, I would not
>> go this way. I would drop "overloading" and gave the functions
>> different name.
>
>> The interface exposed to the user is fragile and could be easily
>> broken with addition/deletion of a function.
>
>> Let me know if you still want the change.
>
> I have another issue related to this topic :)
>
> Please consider this case:
>
> C++ code
>
> class Bar
> {
> public:
> Bar(const std::string &str);
> };
>
> class Foo
> {
> public:
> void set(const std::string &name, bool value);
> void set(const std::string &name, int value);
> void set(const std::string &name, const std::string &value);
> void set(const std::string &name, const Bar &value);
> };
>
>
> Python code
>
> f = Foo()
> f.setAttribute("alfa")
>
>
> The problem here is that boost::python calls the overload with
> "Bar" as argument because is the last (available) registered.
> What's your opinion about this? Shouldn't the registration order
> be:
>
> -> Foo::set(const std::string &name, const Bar &value);
> -> Foo::set(const std::string &name, const std::string &value);
> -> Foo::set(const std::string &name, int value);
> -> Foo::set(const std::string &name, bool value);
>
> As "general rule" Bar is dependent/related from std::string because
> of it's implicit convertibility?
> Maybe decl_wrappers.algorithm.registration_order.is_related must
> be updated according to this rule.
No. I can't change the algorithm, because it will break backward compatibility.
As I said, if you have such problems you'd better avoid them by
changing the alias of the exposed functions. As you can see, a
developer can add a new constructor and as the result other function
will be called. This is hard to find problem.
Don't you think so?
--
Roman Yakovenko
C++ Python language binding
http://www.language-binding.net/
|
|
From: Berserker <ber...@ho...> - 2010-01-28 09:12:11
|
> No. I can't change the algorithm, because it will break backward compatibility. > > As I said, if you have such problems you'd better avoid them by > changing the alias of the exposed functions. As you can see, a > developer can add a new constructor and as the result other function > will be called. This is hard to find problem. > > Don't you think so? I understand the "backward compatibility" problem, but why can't you simply implement a new method in calldef_organizer_t (to avoid backward compatibility issues) instead of using the actual "decl_wrappers.algorithm.registration_order.is_related"? In my opinion I would remove the "one param only" limit in every case, but I think that the second problem that I have reported is what users "expect" when USE_CALLDEF_ORGANIZER is enabled. If you decide to not implement this change, can you help me in writing that? _________________________________________________________________ 25 Gygabite gratis online. Archivia e condividi i tuoi file! http://www.windowslive.it/skyDrive.aspx |
|
From: Roman Y. <rom...@gm...> - 2010-01-28 09:16:28
|
On Thu, Jan 28, 2010 at 11:10 AM, Berserker <ber...@ho...> wrote: >> No. I can't change the algorithm, because it will break backward >> compatibility. >> >> As I said, if you have such problems you'd better avoid them by >> changing the alias of the exposed functions. As you can see, a >> developer can add a new constructor and as the result other function >> will be called. This is hard to find problem. >> >> Don't you think so? > > I understand the "backward compatibility" problem, but why can't you > simply implement a new method in calldef_organizer_t > (to avoid backward compatibility issues) instead of using the actual > "decl_wrappers.algorithm.registration_order.is_related"? > In my opinion I would remove the "one param only" limit in every case, I can and will do this. > but I think that the second problem that I have reported is what users > "expect" when USE_CALLDEF_ORGANIZER is enabled. > If you decide to not implement this change, can you help me in writing that? definitely :-). -- Roman Yakovenko C++ Python language binding http://www.language-binding.net/ |
|
From: Berserker <ber...@ho...> - 2010-01-28 09:18:39
|
I forgot to report another important problem about calldef_organizer_t.
I have a case like this:
class Foo
{
public:
void setValue(bool v);
void setValue(int v);
void setValue(double v);
};
Actually, enabling USE_CALLDEF_ORGANIZER, the "bool" override is registered
after the "int" one but before "double" and so the python code:
f = Foo()
f.setValue(True)
invokes the "double" override.
The problem is that "is_related" method invokes is_integral and not is_arithmetic
as I expected: I think that this is another reason to implement a custom "is_related"
method in calldef_organizer_t.
_________________________________________________________________
25 Gygabite gratis online. Archivia e condividi i tuoi file!
http://www.windowslive.it/skyDrive.aspx |
|
From: Roman Y. <rom...@gm...> - 2010-01-28 09:37:15
|
On Thu, Jan 28, 2010 at 11:18 AM, Berserker <ber...@ho...> wrote:
> I forgot to report another important problem about calldef_organizer_t.
> I have a case like this:
>
> class Foo
> {
> public:
> void setValue(bool v);
> void setValue(int v);
> void setValue(double v);
> };
>
> Actually, enabling USE_CALLDEF_ORGANIZER, the "bool" override is registered
> after the "int" one but before "double" and so the python code:
>
> f = Foo()
> f.setValue(True)
>
> invokes the "double" override.
> The problem is that "is_related" method invokes is_integral and not
> is_arithmetic
> as I expected: I think that this is another reason to implement a custom
> "is_related"
> method in calldef_organizer_t.
That's okey. You will be able to define your own sort criteria.
--
Roman Yakovenko
C++ Python language binding
http://www.language-binding.net/
|
|
From: Berserker <ber...@ho...> - 2010-01-28 09:45:55
|
> That's okey. You will be able to define your own sort criteria. Thanks, what about the "implict conversion" problem? I'm looking at the Py++ sources and maybe I think that "is_convertible" method among t1, t2 should do the job when organizer_t_wrapper compares the two types. O.T.: this is ours "preliminary" Python documentation http://osiris.kodeware.net/python/0.13/ Thanks again Roman for your great job with Py++! _________________________________________________________________ Un mondo di personalizzazioni per Messenger, PC e cellulare http://www.pimpit.it/ |
|
From: Roman Y. <rom...@gm...> - 2010-01-28 09:53:51
|
On Thu, Jan 28, 2010 at 11:45 AM, Berserker <ber...@ho...> wrote:
>> That's okey. You will be able to define your own sort criteria.
>
> Thanks, what about the "implict conversion" problem?
> I'm looking at the Py++ sources and maybe I think that "is_convertible"
> method among t1, t2 should do the job when organizer_t_wrapper
> compares the two types.
May be. It takes into account the conversion operators too. I am not
sure whether you want it or not.
struct A{}
struct B{
operator A() const;
}
I suggest you to start prepare a ton of test cases, so when you will
done with algorithm, you will be able to check the result.
> O.T.: this is ours "preliminary" Python documentation
> http://osiris.kodeware.net/python/0.13/
> Thanks again Roman for your great job with Py++!
You are welcome.
--
Roman Yakovenko
C++ Python language binding
http://www.language-binding.net/
|
|
From: Roman Y. <rom...@gm...> - 2010-01-28 20:53:49
|
On Thu, Jan 28, 2010 at 11:53 AM, Roman Yakovenko
<rom...@gm...> wrote:
> On Thu, Jan 28, 2010 at 11:45 AM, Berserker <ber...@ho...> wrote:
>>> That's okey. You will be able to define your own sort criteria.
>>
>> Thanks, what about the "implict conversion" problem?
>> I'm looking at the Py++ sources and maybe I think that "is_convertible"
>> method among t1, t2 should do the job when organizer_t_wrapper
>> compares the two types.
>
> May be. It takes into account the conversion operators too. I am not
> sure whether you want it or not.
>
> struct A{}
>
> struct B{
> operator A() const;
> }
>
> I suggest you to start prepare a ton of test cases, so when you will
> done with algorithm, you will be able to check the result.
http://pygccxml.svn.sourceforge.net/pygccxml/?rev=1821&view=rev
I committed a change, which extends the algorithms to work with
functions, that has any number of arguments. It doesn't take into
account implicit conversion, cause it could break backward
compatibility.
I also changed the methods names, so they will not be private. All you
need is to redefine "cmp_calldefs" and to introduce your logic. Both
functions have same number of required arguments. You can take a look
on pygccxml.declarations.type_traits module, which contains many
useful functions.
Enjoy.
--
Roman Yakovenko
C++ Python language binding
http://www.language-binding.net/
|
|
From: Berserker <ber...@ho...> - 2010-01-29 08:43:39
|
> http://pygccxml.svn.sourceforge.net/pygccxml/?rev=1821&view=rev > > I committed a change, which extends the algorithms to work with > functions, that has any number of arguments. It doesn't take into > account implicit conversion, cause it could break backward > compatibility. > > I also changed the methods names, so they will not be private. All you > need is to redefine "cmp_calldefs" and to introduce your logic. Both > functions have same number of required arguments. You can take a look > on pygccxml.declarations.type_traits module, which contains many > useful functions. > > Enjoy. Thanks I'll play with it ;) _________________________________________________________________ Non sei a casa? Prova il nuovo Web Messenger http://www.messenger.it/web/default.aspx |
|
From: Berserker <ber...@ho...> - 2010-01-29 09:25:28
|
> I also changed the methods names, so they will not be private. All you > need is to redefine "cmp_calldefs" and to introduce your logic. Both > functions have same number of required arguments. You can take a look > on pygccxml.declarations.type_traits module, which contains many > useful functions. > > Enjoy. Shouldn't algorithm.registration_order.select_problematics be changed according to this patch?Actually it's: def select_problematics( calldef ): """Return list of problematic functions for function "calldef" """ if 1 != len( calldef.required_args ): return [] ... _________________________________________________________________ Velocità, sicurezza e...tanto spazio! Scopri le novità di Hotmail http://www.windowslive.it/hotmail/Home_novita.aspx |
|
From: Roman Y. <rom...@gm...> - 2010-01-29 20:28:21
|
On Fri, Jan 29, 2010 at 11:25 AM, Berserker <ber...@ho...> wrote: >> I also changed the methods names, so they will not be private. All you >> need is to redefine "cmp_calldefs" and to introduce your logic. Both >> functions have same number of required arguments. You can take a look >> on pygccxml.declarations.type_traits module, which contains many >> useful functions. >> >> Enjoy. > > Shouldn't algorithm.registration_order.select_problematics be changed > according to this patch?Actually it's: > > def select_problematics( calldef ): > """Return list of problematic functions for function "calldef" """ > if 1 != len( calldef.required_args ): > return [] > ... Yes, it should. I will fix it in a few days.Thanks for noting this. -- Roman Yakovenko C++ Python language binding http://www.language-binding.net/ |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X