Re: [pygccxml-development] Implicit virtual functions in derived classes

[Roman Y. <rom...@gm...>]

On Feb 16, 2008 9:56 AM, Roman Yakovenko <rom...@gm...> wrote:
>
> On Sat, Feb 16, 2008 at 8:58 AM, Julian Scheid <jul...@rs...> wrote:
> > Julian Scheid wrote:
> >  > Due to the way funcs.update works (?) this will put A.foo into "funcs"
> >  > instead of B.foo when generating the wrapper for C. This in turn will
> >  > lead to the following wrapper code for class C to be generated:
> >
> >  To elaborate, the order in which "A::foo" and "B::foo" are traversed in
> >  the "for f in funcs" loop depends on details of Python's Hashset
> >  implementation - they might or might not be traversed in the right order
> >  depending on how they are hashed. The right order being B::foo first and
> >  then A::foo second so that B::foo is added and A::foo is later rejected
> >  when tested with is_same_function.
> >
> >  I haven't actually checked how things turn out for my hypothetical
> >  example from the previous mail, if you can't reproduce it using that
> >  example try using different class names (swapping A and B so that A is
> >  derived from B and C is derived from A should do the trick).
> >
> >  I found that this problem can be avoided simply by using a list instead
> >  of a set for "funcs" and extend'ing it instead of update'ing it. While
> >  possibly a tad less efficient, this seems to work fine and guarantees
> >  that later on the right superclass is used.
> >
> >  There is another intricacy. In the previous example (with classes A, B
> >  and C) consider that mb.class_("B").member_functions("foo").exclude()
> >  has been called. Now, if you use "not ignored and exportable" in the
> >  query as I suggested originally, what happens is that only A.foo ends up
> >  in "funcs" and the generated code for C_wrapper will incorrectly
> >  dispatch to A::foo. What works better is either to disregard whether or
> >  not a function is marked as ignore (downside being that exclude won't
> >  affect derived classes) or, better, to filter the resulting list
> >  "functions" at the very end of "redefined_funcs" and remove all methods
> >  that are marked as ignore.
> >
> >  My final results are attached as a patch. It seems to solve all issues
> >  except for the overhead problem. (I was wrong about the static modifier
> >  - this turned out not to be necessary.)
> >
> >  Cheers,
> >
> >  Julian
> >
> > --- class_wrapper.py    2008-02-16 17:52:10.000000000 +1100
> >  +++ class_wrapper_derived_virtuals.py   2008-02-16 17:53:30.000000000 +1100
> >  @@ -442,19 +442,20 @@
> >          all_included = declarations.custom_matcher_t( lambda decl: decl.ignore == False and decl.exportable )
> >          all_protected = declarations.access_type_matcher_t( 'protected' ) & all_included
> >          all_pure_virtual = declarations.virtuality_type_matcher_t( VIRTUALITY_TYPES.PURE_VIRTUAL )
> >  -        all_not_pure_virtual = ~all_pure_virtual
> >  +        all_virtual = declarations.virtuality_type_matcher_t( VIRTUALITY_TYPES.VIRTUAL ) & (declarations.access_type_matcher_t( 'public' ) | declarations.access_type_matcher_t( 'protected' ))
> >  +        all_not_pure_virtual = ~(all_pure_virtual | all_virtual)
> >
> >          query = all_protected | all_pure_virtual
> >          relevant_opers = declarations.custom_matcher_t( lambda decl: decl.symbol in ('()', '[]') )
> >  -        funcs = set()
> >  +        funcs = []
> >          defined_funcs = set()
> >
> >          for base in self.recursive_bases:
> >              if base.access == ACCESS_TYPES.PRIVATE:
> >                  continue
> >              base_cls = base.related_class
> >  -            funcs.update( base_cls.member_functions( query, allow_empty=True ) )
> >  -            funcs.update( base_cls.member_operators( relevant_opers & query, allow_empty=True ) )
> >  +            funcs.extend( base_cls.member_functions( query | (all_virtual & declarations.custom_matcher_t( lambda decl: decl.parent.alias == base_cls.alias )), allow_empty=True ) )
> >  +            funcs.extend( base_cls.member_operators( relevant_opers & query, allow_empty=True ) )
> >
> >              defined_funcs.update( base_cls.member_functions( all_not_pure_virtual, allow_empty=True ) )
> >              defined_funcs.update( base_cls.member_operators( all_not_pure_virtual & relevant_opers, allow_empty=True ) )
> >  @@ -481,7 +482,7 @@
> >                                  break
> >                          else:
> >                              not_reimplemented_funcs.add( f )
> >  -        functions = list( not_reimplemented_funcs )
> >  +        functions = filter( lambda f: not f.ignore and f.exportable, list( not_reimplemented_funcs ) )
> >          functions.sort( cmp=lambda f1, f2: cmp( ( f1.name, f1.location.as_tuple() )
> >                                                  , ( f2.name, f2.location.as_tuple() ) ) )
> >          self._redefined_funcs = functions
> >
> >
>
> Thank you very much!!!
>
> I am learning your patch and run through all my tests. If everything
> is okey I will commit it today.

Can I ask you to submit some kind of unit test - something very
simple, that will map your class hierarchy. I would like to be sure
next time I touch the code the things will continue to work.

Thanks


-- 
Roman Yakovenko
C++ Python language binding
http://www.language-binding.net/