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

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

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.

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