From: William S F. <ws...@fu...> - 2008-09-06 10:31:19
|
zhiyang jiang wrote: > > > 2008/7/30 William S Fulton <ws...@fu... > <mailto:ws...@fu...>> > > zhiyang jiang wrote: > > > > 2008/7/14 William S Fulton <ws...@fu... > <mailto:ws...@fu...> <mailto:ws...@fu... > <mailto:ws...@fu...>>>: > > > zhiyang jiang wrote: > > > Hi William > Through a few days' work, i make some changes in SWIG > source for > bettering the multi inheritance .Now the multi inheritance > likes this: > > > Hi, > > Hope you are well. Thanks for this. I'm a bit overloaded at the > moment, but will have a look in the next few days. > > William > > > this is a new patch! > > -- > > I am sorry for taking so long to look at this, there is too much > going on at the moment. > > I think you have left out the .i test and runtime test for > multiple_inheritance_abstract in your second patch, so I looked for > these in your first patch... not a problem unless you changed them. > > Having looked at the patch, the following change is a big concern: > > - super($imclassname.SWIG$javaclassnameUpcast(cPtr), cMemoryOwn); > + //super($imclassname.SWIG$javaclassnameUpcast(cPtr), cMemoryOwn); > > This upcast is absolutely necessary as the pointer to the base class > might not be the same value as the pointer to a derived class, > therefore the original pointer must be stored in the derived class > as well as the pointer value up cast to the base class. > > The SWIG$javaclassnameUpcast(cPtr) just return the pointer of derived > classes' first base class,so i don't think it is useful when up cast to > the case class.But it will get undefined behavior when calling a c++ > function that it has a Base classes as parameter ,see testcase's > function foo6(),foo7(),foo8() in java. i think it may be caused by the > conversion of pointer from java to c++,when the address of a class is > converted from java to c++,some type info will lost,i am not sure about it. > > > The following also won't work when a null pointer is being used: > > -%typemap(javain) SWIGTYPE *& "$*javaclassname.getCPtr($javainput)" > +/*%typemap(javain) SWIGTYPE *& "$*javaclassname.getCPtr($javainput)"*/ > +%typemap(javain) SWIGTYPE *& "$javainput.getCPtr()" > > The new patch will check the $javainput use a condition expression now! > > > I suggest you add another test method to your testcase, one which > takes ABase1* as a parameter. Also CBase1*, CBase2* as both return > types and input parameters. > > I add some new functions into testcase, they test the base classes as > parameters, but unfortunately, it will get unexpected results when use > these functions, > > > For any future patches, can I suggest you post them publicly. One of > the lessons to be learnt in open source programming, which is > encouraged for all Google summer of code students, is to develop in > the open. > > William > > This patch works well when there is no c++ function that has a base > class as its parameter,or it will get unexpected results ,and it will be > ok when there are functions that has base class parameters in java, see > the testcase about that.I am now finding what's wrong with it. > > Hi Jiang Sorry, it has taken a while, but I have now tried out your patch and can see your multiple_inheritance_abstract testcase you added which is quite comprehensive for this feature. That is good and it runs fine. Before looking at the patch any further though, the regressions that it introduces need to be sorted out. If you run the entire test-suite you will see plenty of problems. Any proposed solution must not break existing code. The changes you made to the typemaps in java.swg are the main reasons for the breakages and I'm afraid that these cannot result in changing behaviour. Instead what you might consider is a library of typemaps that can be applied for any types that want to use %feature("interface"). Even then, a really good case would need to be made for changing the definition of getCPtr. William |