Uploaded image for project: 'PyLucene'
  1. PyLucene
  2. PYLUCENE-37

Extended interfaces beyond first are ignored

    Details

    • Type: Bug
    • Status: Open
    • Priority: Major
    • Resolution: Unresolved
    • Labels:
      None

      Description

      When generating wrapper for a Java interface that extends more than one other interface, then only the first extended interface is used when generating the C++ class.

      In cpp.header(), the code snippets:

          if cls.isInterface():
              if interfaces:
                  superCls = interfaces.pop(0)
      

      and:

              line(out, indent, 'class %s%s : public %s {',
                   _dll_export, cppname(names[-1]), absname(cppnames(superNames)))
      

      are likely responsible.

      1. jcc.multiple.inheritance.patch
        12 kB
        Jesper Mattsson
      2. Test.zip
        7 kB
        Jesper Mattsson

        Activity

        Hide
        vajda Andi Vajda added a comment -

        There seems to be something off with that code indeed.
        However, it would be helpful if you could include a small trivial example java code
        that triggers the bug you found and an explanation of what you'd expect it
        to do instead. This helps me ensure there is no misunderstanding and also helps with reproducing the bug.
        Thanks !

        Show
        vajda Andi Vajda added a comment - There seems to be something off with that code indeed. However, it would be helpful if you could include a small trivial example java code that triggers the bug you found and an explanation of what you'd expect it to do instead. This helps me ensure there is no misunderstanding and also helps with reproducing the bug. Thanks !
        Hide
        jmattsson Jesper Mattsson added a comment - - edited

        Certainly, I should have done that when creating the issue - sorry.

        I attached all the files I used when verifying the bug, but here are the highlights:

        The Java interface C is defined as:

        public interface C extends B, A {
            void c();
        }
        

        The generated header file for it, C.h, contains (copying only the relevant lines):

        #include "testjcc/B.h"
        
        namespace testjcc {
          class A;
        }
        
        namespace testjcc {
          class C : public ::testjcc::B {
        

        As you can see, A has a forward declaration, but is not included or inherited. Changing the order of the interfaces in the Java file changes the generated file so that A is included and inherited instead.

        I'd expect both A.h & B.h to be included, and the first line of the class to be:

          class C : public ::testjcc::B, public ::testjcc::A {
        
        Show
        jmattsson Jesper Mattsson added a comment - - edited Certainly, I should have done that when creating the issue - sorry. I attached all the files I used when verifying the bug, but here are the highlights: The Java interface C is defined as: public interface C extends B, A { void c(); } The generated header file for it, C.h, contains (copying only the relevant lines): #include "testjcc/B.h" namespace testjcc { class A; } namespace testjcc { class C : public ::testjcc::B { As you can see, A has a forward declaration, but is not included or inherited. Changing the order of the interfaces in the Java file changes the generated file so that A is included and inherited instead. I'd expect both A.h & B.h to be included, and the first line of the class to be: class C : public ::testjcc::B, public ::testjcc::A {
        Hide
        jmattsson Jesper Mattsson added a comment -

        I made a first stab at a solution. It is only implemented in the Python 2 version, and the Python wrapper generation isn't done, but I'll attach it as a patch in the hope that it will be of use to you.

        Show
        jmattsson Jesper Mattsson added a comment - I made a first stab at a solution. It is only implemented in the Python 2 version, and the Python wrapper generation isn't done, but I'll attach it as a patch in the hope that it will be of use to you.
        Hide
        jmattsson Jesper Mattsson added a comment -

        First attempt at solution.

        Show
        jmattsson Jesper Mattsson added a comment - First attempt at solution.
        Hide
        jmattsson Jesper Mattsson added a comment -

        I am currently unable to work any more on this beyond the patch that I sent.

        Show
        jmattsson Jesper Mattsson added a comment - I am currently unable to work any more on this beyond the patch that I sent.
        Hide
        vajda Andi Vajda added a comment -

        I have gotten to the same point as you - the Python side needs to be reworked to accomodate
        multiple parents, the current static initializations don't work with multiple base classes.
        It's work in progress...

        Show
        vajda Andi Vajda added a comment - I have gotten to the same point as you - the Python side needs to be reworked to accomodate multiple parents, the current static initializations don't work with multiple base classes. It's work in progress...
        Hide
        jmattsson Jesper Mattsson added a comment -

        Thanks for the update. Did you think that the changes in the patch are reasonable?

        Show
        jmattsson Jesper Mattsson added a comment - Thanks for the update. Did you think that the changes in the patch are reasonable?
        Hide
        vajda Andi Vajda added a comment -

        I've been working on this bug for a while now and I'm wondering how far a fix can get and what it achieves. I am about to check-in a large body of changes that makes a fix possible by switching Python type construction to use PyType_FromSpecWithBases() which can create a type with multiple base types. This is Python 3 only and this is fine. Python 2 support is in maintenance mode only.
        What would fixing this bug achieve ?

        • wrappers would be correct with regards to their super types and side-casting interfaces would work
        • wrappers would inherit static fields and methods declared on interfaces beyond the first one)
          Other than that, all is already working. For a non abstract java class to implement multiple interfaces, it has to have local implementations of all their methods (or inherit some from a parent class with local implementations) - thus I don't see any functional inheritance losses at the moment (beyond the static ones).
          Also, Python has, to say the least, some strange layout requirements when constructing a type from multiple bases - they all have to have the same tp_basicsize. This is not the case with the JCC t_type, the types used with Python (their sizeof() is tp_basicsize), that wrap the C++/Java bridge types as they may have an array of type parameter of variable size (depending on the number of type parameters) when the java type being wrapped is generic. At the moment, I'm not even sure how much Python can support JCC types with multiple base types - maybe the t_type layout can be modified a bit to fix the size of type parameters by moving to an array pointer.
          All that being said, it would be very nice if multiple inheritance could be properly supported:
        • it would better model 'class foo extends bar implements baz, ..."
        • it would better model the interface tree you proposed in the test
          The new code now has a lot of stuff in place for supporting multiple inheritance but it is not enabled:
        • placeholder for virtual inheritance
        • support for multiple parents everywhere
        • Python type construction supports multiple bases
          The code doesn't have the change in logic yet to properly track 'extends foo implements bar' and the code still suffers from the bug reported here, only the first interface is used as parent for a sub-interface.
          I'm going to continue working on this on and off as it's an interesting problem...
        Show
        vajda Andi Vajda added a comment - I've been working on this bug for a while now and I'm wondering how far a fix can get and what it achieves. I am about to check-in a large body of changes that makes a fix possible by switching Python type construction to use PyType_FromSpecWithBases() which can create a type with multiple base types. This is Python 3 only and this is fine. Python 2 support is in maintenance mode only. What would fixing this bug achieve ? wrappers would be correct with regards to their super types and side-casting interfaces would work wrappers would inherit static fields and methods declared on interfaces beyond the first one) Other than that, all is already working. For a non abstract java class to implement multiple interfaces, it has to have local implementations of all their methods (or inherit some from a parent class with local implementations) - thus I don't see any functional inheritance losses at the moment (beyond the static ones). Also, Python has, to say the least, some strange layout requirements when constructing a type from multiple bases - they all have to have the same tp_basicsize. This is not the case with the JCC t_type, the types used with Python (their sizeof() is tp_basicsize), that wrap the C++/Java bridge types as they may have an array of type parameter of variable size (depending on the number of type parameters) when the java type being wrapped is generic. At the moment, I'm not even sure how much Python can support JCC types with multiple base types - maybe the t_type layout can be modified a bit to fix the size of type parameters by moving to an array pointer. All that being said, it would be very nice if multiple inheritance could be properly supported: it would better model 'class foo extends bar implements baz, ..." it would better model the interface tree you proposed in the test The new code now has a lot of stuff in place for supporting multiple inheritance but it is not enabled: placeholder for virtual inheritance support for multiple parents everywhere Python type construction supports multiple bases The code doesn't have the change in logic yet to properly track 'extends foo implements bar' and the code still suffers from the bug reported here, only the first interface is used as parent for a sub-interface. I'm going to continue working on this on and off as it's an interesting problem...
        Hide
        jmattsson Jesper Mattsson added a comment -

        We ran into it because we are developing an API where the public parts are interfaces, and the implementing classes are private. The problem was that the methods declared in the second inherited interface was not accessible.

        I guess an alternate approach would be to include the methods from the inherited interfaces (except the one the C++ class inherits from). That would solve the problems we've run into so far. It looks to me that you'd run into problems when passing objects as arguments from C++, though:

        Given the declarations below:

        class A {
          void f(B b) { /* ... */ }
        }
        interface B { /* ... */ }
        class C extends D implements B { /* ... */ }
        class D { /* ... */ }
        interface E { /* ... */ }
        interface F extends E, B { /* ... */ }
        

        When trying to use the C++ wrappers to call A.f, wouldn't you currently get compilation errors if you passed the b argument from a reference of type C or F? What about in Python? Exception at runtime?

        Show
        jmattsson Jesper Mattsson added a comment - We ran into it because we are developing an API where the public parts are interfaces, and the implementing classes are private. The problem was that the methods declared in the second inherited interface was not accessible. I guess an alternate approach would be to include the methods from the inherited interfaces (except the one the C++ class inherits from). That would solve the problems we've run into so far. It looks to me that you'd run into problems when passing objects as arguments from C++, though: Given the declarations below: class A { void f(B b) { /* ... */ } } interface B { /* ... */ } class C extends D implements B { /* ... */ } class D { /* ... */ } interface E { /* ... */ } interface F extends E, B { /* ... */ } When trying to use the C++ wrappers to call A.f, wouldn't you currently get compilation errors if you passed the b argument from a reference of type C or F? What about in Python? Exception at runtime?
        Hide
        vajda Andi Vajda added a comment -

        If you use the wrappers generated for interfaces, you're going to face the problem reported here.
        If you use the wrappers generated for classes implementing said interfaces, then you should be ok with all implemented methods anywhere as the implementing class has them all (or it is abstract).

        You can also force the wrapping of private classes by explicitly listing them on the command line.

        Show
        vajda Andi Vajda added a comment - If you use the wrappers generated for interfaces, you're going to face the problem reported here. If you use the wrappers generated for classes implementing said interfaces, then you should be ok with all implemented methods anywhere as the implementing class has them all (or it is abstract). You can also force the wrapping of private classes by explicitly listing them on the command line.

          People

          • Assignee:
            Unassigned
            Reporter:
            jmattsson Jesper Mattsson
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:

              Development