director protected methods

[William S Fulton]

I've reprotected the director methods for Java. I see that the wrappers for
protected methods call SWIG_exception after trying a downcast. What is the logic
here? From a Java perspective, I don't see this cast and exception being
necessary as the method will be protected from the Java side. Am I missing
something? If it is redundant, I'd like to get rid of the exception being thrown
as it is not implemented correctly. The method must return as soon as
SWIG_exception is called otherwise unpredictable results will occur on calling
any further JNI methods.

A suggestion for the future for wrapping of director protected methods. The
director class is generated into the xxxx_wrap.h file. I'm not quite sure why
these need to be put into a wrapper file, maybe someone can enlighten me? Anyway
the problem is that the director classes break the encapsulation of the
protected methods should anyone use the header file. We could maintain/enforce
the encapsulation by modifying the generated code to use friends. Currently we
have when wrapping a protected method Bar::pang() :

class SwigDirector_Bar : public Bar, public Swig::Director {
...
public:
virtual std::string pang();
...
};

We could instead generate this:

class SwigDirector_Bar : public Bar, public Swig::Director {
...
protected:
virtual std::string pang();
// Python
friend PyObject * ::_wrap_Bar_pang(PyObject *self, PyObject *args);
// Java
friend JNIEXPORT jstring JNICALL ::Java_exampleJNI_Bar_1pang(JNIEnv *jenv,
jclass jcls, jlong jarg1);
...
};

Thoughts?

William

[Marcelo Matus]

Post by William S Fulton
I've reprotected the director methods for Java. I see that the
wrappers for protected methods call SWIG_exception after trying a
downcast. What is the logic here? From a Java perspective, I don't see
this cast and exception being necessary as the method will be
protected from the Java side.
Am I missing something? If it is redundant, I'd like to get rid of the
exception being thrown as it is not implemented correctly. The method
must return as soon as SWIG_exception is called otherwise
unpredictable results will occur on calling any further JNI methods.

I'm not sure either, the SWIG_exception is catching the case where the
dynamic cast fails, maybe in java it could never never fails.

The idea behind this was that you need a dynamic cast since
you only get a Foo* value, and not a SwigDirector_Foo*, so,
you need to retreive the SwigDirector_Foo* value before doing
the real call.

And if already you get a right Foo* value, but the dynamic cast fails,
is that for some strange combinations of casting, you didn't get the right
pointer value.

This was specially valid in python, before the runtime protection was
implemented, now it seems it doesn't matter because python is catching
the bad accessing before it happens:

/* here now it catches the protected member called */
if (director && !(director->swig_get_inner("ping")))
SWIG_exception(SWIG_RuntimeError,"accessing protected member ping");

if (director && (director->swig_get_self()==obj0)) director->swig_set_up();
darg = dynamic_cast<SwigDirector_Foo *>(arg1);

if (!darg) SWIG_exception(SWIG_RuntimeError,"accesing protected member
ping");


so, if the SWIG_RuntimeError() is not working fine in Java, I guess we
could safely
get rid of it. I tried to replace with return $nil, but it doesn't work
in python,
so. You can safetly delete the entire line.

In, fact, I just tested and it works fine. I just need to check director
!= 0
in the python side. And if for any strange reason we found
python would need to check the casting, I could add that code
just in the python side.

In ruby is worging too, just in ocalm it will fail, but it will get
fixed when
the protection mechanism is implemented.

Post by William S Fulton
A suggestion for the future for wrapping of director protected
methods. The director class is generated into the xxxx_wrap.h file.
I'm not quite sure why these need to be put into a wrapper file, maybe
someone can enlighten me?

You mean, why the director class goes into the xxx_wrap.h file?, I don't
know,
maybe some historical reason behind it (?).

But just for the record, I remember that someone has problems with this,
ie, the xxx_wrap.cc file was generated in some directory, but the
xxx_wrap.h was
generated in another place.

This was related to running swig in a different directory, like

swig -noruntime main/main.i -o main/main_wrap.cxx

Maybe the class declaration could also goes into the wrap.cxx file?,
hmmmm.... thinking about it, maybe is something related to the %import
directive?, I don't know.... but I will not touch it!!!.

Post by William S Fulton
Anyway the problem is that the director classes break the
encapsulation of the protected methods should anyone use the header
file. We could maintain/enforce the encapsulation by modifying the
generated code to use friends. Currently we have when wrapping a
class SwigDirector_Bar : public Bar, public Swig::Director {
...
virtual std::string pang();
...
};
class SwigDirector_Bar : public Bar, public Swig::Director {
...
virtual std::string pang();
// Python
friend PyObject * ::_wrap_Bar_pang(PyObject *self, PyObject *args);
// Java
friend JNIEXPORT jstring JNICALL ::Java_exampleJNI_Bar_1pang(JNIEnv
*jenv, jclass jcls, jlong jarg1);
...
};
Thoughts?

That looks nice, but it will require more changes in the
python.cxx, java.cxx, ruby.cxx and ocaml.cxx files.

And Scott will finally kill me if we do that right now.

Post by William S Fulton
William
_______________________________________________
http://mailman.cs.uchicago.edu/mailman/listinfo/swig-dev

[William S Fulton]

Post by Marcelo Matus

Post by William S Fulton
I've reprotected the director methods for Java. I see that the
wrappers for protected methods call SWIG_exception after trying a
downcast. What is the logic here? From a Java perspective, I don't see
this cast and exception being necessary as the method will be
protected from the Java side.
Am I missing something? If it is redundant, I'd like to get rid of the
exception being thrown as it is not implemented correctly. The method
must return as soon as SWIG_exception is called otherwise
unpredictable results will occur on calling any further JNI methods.

I'm not sure either, the SWIG_exception is catching the case where the
dynamic cast fails, maybe in java it could never never fails.
The idea behind this was that you need a dynamic cast since
you only get a Foo* value, and not a SwigDirector_Foo*, so,
you need to retreive the SwigDirector_Foo* value before doing
the real call.
And if already you get a right Foo* value, but the dynamic cast fails,
is that for some strange combinations of casting, you didn't get the right
pointer value.

I'm trying to think of how a Foo* would get passed to this function, but can't
think of one if using proxy/shadow classes. So I've removed the SWIG_exception.

In taking a closer look at SWIG_exception, which is used in other general SWIG
libraries, I've noticed that it doesn't work as it should for Java and probably
some other languages because it fails to implement an immediate function return.
I can't figure out how to solve this without implementing a uniform error
handling approach across all languages, so it isn't going to happen for this
release.

Post by Marcelo Matus
This was specially valid in python, before the runtime protection was
implemented, now it seems it doesn't matter because python is catching
/* here now it catches the protected member called */
if (director && !(director->swig_get_inner("ping")))
SWIG_exception(SWIG_RuntimeError,"accessing protected member ping");
if (director && (director->swig_get_self()==obj0)) director->swig_set_up();
darg = dynamic_cast<SwigDirector_Foo *>(arg1);
if (!darg) SWIG_exception(SWIG_RuntimeError,"accesing protected member
ping");
so, if the SWIG_RuntimeError() is not working fine in Java, I guess we
could safely
get rid of it. I tried to replace with return $nil, but it doesn't work
in python,

It's actually $null, but it will only work in Java and C#.

Post by Marcelo Matus
so. You can safetly delete the entire line.
In, fact, I just tested and it works fine. I just need to check director
!= 0
in the python side. And if for any strange reason we found
python would need to check the casting, I could add that code
just in the python side.
In ruby is worging too, just in ocalm it will fail, but it will get
fixed when
the protection mechanism is implemented.

Post by William S Fulton
A suggestion for the future for wrapping of director protected
methods. The director class is generated into the xxxx_wrap.h file.
I'm not quite sure why these need to be put into a wrapper file, maybe
someone can enlighten me?

You mean, why the director class goes into the xxx_wrap.h file?, I don't
know,
maybe some historical reason behind it (?).
But just for the record, I remember that someone has problems with this,
ie, the xxx_wrap.cc file was generated in some directory, but the
xxx_wrap.h was
generated in another place.
This was related to running swig in a different directory, like
swig -noruntime main/main.i -o main/main_wrap.cxx
Maybe the class declaration could also goes into the wrap.cxx file?,
hmmmm.... thinking about it, maybe is something related to the %import
directive?, I don't know.... but I will not touch it!!!.

I fixed that recently. Now the _wrap.h file goes into the same directory as the
xxx_wrap.cxx file. There was also a bug where it was emitting
#include "main/xxx_wrap.h"
instead of
#include "xxx_wrap.h"
But it is all sorted now.

Post by Marcelo Matus

Post by William S Fulton
Anyway the problem is that the director classes break the
encapsulation of the protected methods should anyone use the header
file. We could maintain/enforce the encapsulation by modifying the
generated code to use friends. Currently we have when wrapping a
class SwigDirector_Bar : public Bar, public Swig::Director {
...
virtual std::string pang();
...
};
class SwigDirector_Bar : public Bar, public Swig::Director {
...
virtual std::string pang();
// Python
friend PyObject * ::_wrap_Bar_pang(PyObject *self, PyObject *args);
// Java
friend JNIEXPORT jstring JNICALL ::Java_exampleJNI_Bar_1pang(JNIEnv
*jenv, jclass jcls, jlong jarg1);
...
};
Thoughts?

That looks nice, but it will require more changes in the
python.cxx, java.cxx, ruby.cxx and ocaml.cxx files.
And Scott will finally kill me if we do that right now.

In my head I had the definition of the 'future' as after the release :) Let's
tackle it then.

William

[Mark Rose]

Post by Marcelo Matus

Post by William S Fulton
A suggestion for the future for wrapping of director protected
methods. The director class is generated into the xxxx_wrap.h file.
I'm not quite sure why these need to be put into a wrapper file, maybe
someone can enlighten me?

You mean, why the director class goes into the xxx_wrap.h file?, I don't
know,
maybe some historical reason behind it (?).
But just for the record, I remember that someone has problems with this,
ie, the xxx_wrap.cc file was generated in some directory, but the
xxx_wrap.h was
generated in another place.
This was related to running swig in a different directory, like
swig -noruntime main/main.i -o main/main_wrap.cxx
Maybe the class declaration could also goes into the wrap.cxx file?,
hmmmm.... thinking about it, maybe is something related to the %import
directive?, I don't know.... but I will not touch it!!!.

Mostly historical reasons. I couldn't decide if the class declarations
might be useful outside of swig, so to be safe I broke them out into
headers. If noone else can think of a compelling reason to do this
then they can be inlined in the cxx file.

Sorry I haven't had time to follow all the director-related events of
late. I should have a chance after the holidays to catch up though.

Cheers,
Mark

[Scott Michel]

Post by Marcelo Matus

Post by William S Fulton
I've reprotected the director methods for Java. I see that the
wrappers for protected methods call SWIG_exception after trying a
downcast. What is the logic here? From a Java perspective, I don't see
this cast and exception being necessary as the method will be
protected from the Java side.
Am I missing something? If it is redundant, I'd like to get rid of the
exception being thrown as it is not implemented correctly. The method
must return as soon as SWIG_exception is called otherwise
unpredictable results will occur on calling any further JNI methods.

I'm not sure either, the SWIG_exception is catching the case where the
dynamic cast fails, maybe in java it could never never fails.

I don't believe that what William is saying. With Java, you want to throw
the exception and leave the method as quickly as possible. You don't want to
execute any other code. The code should have a couple extra braces, like so:

if (director_var != NULL) {
// Do what we're supposed to do
} else {
// Throw the exception
}

Post by Marcelo Matus

Post by William S Fulton
A suggestion for the future for wrapping of director protected
methods. The director class is generated into the xxxx_wrap.h file.
I'm not quite sure why these need to be put into a wrapper file, maybe
someone can enlighten me?

You mean, why the director class goes into the xxx_wrap.h file?, I don't
know,
maybe some historical reason behind it (?).

Putting the director classes in the extra wrapper file was something I
continued from kiping the Python code. But I had a reason.

The wrapper header is really handy when one is wrapping a lot of classes and
using multiple interface files. I'm not entirely sure in which context the
director classes from other interface files show up in the JNI code at this
point, but the feature didn't do any harm in the past and I'm really sure
that taking the wrapper header files away will do irrevokable harm to my sanity.



-scooter

[Marcelo Matus]

Post by William S Fulton
Am I missing something? If it is redundant, I'd like to get rid of
the exception being thrown as it is not implemented correctly. The
method must return as soon as SWIG_exception is called otherwise
unpredictable results will occur on calling any further JNI methods.

This is now not related to the dirprot mode, since we don't need to call
the SWIG_exception anymore (the "reprotection" mechanism is taking care
of the bad cases), but, I though that SWIG_exception was returning
just after been called, which is the case in python, but it seems
is not the case in Java.

In python, it "throws" the exception, ie, tells python
there was an error, and then it jumps to the end of the routine:

#define SWIG_fail goto fail
#define SWIG_exception(a,b) { SWIG_exception_(a,b); SWIG_fail; }

where the auxiliar "SWIG_exception_" is in python the equivalent to
"SWIG_JavaException" in java, and the typical routine end looks like:

.....
return resultobj;
fail:
<some clean-up if needed, not always code here>
return NULL;
}


maybe java and the other languages that don' have it,
just need to define the SWIG_fail action, ie:

in java:

#define SWIG_fail return 0
#define SWIG_exception(code, msg) { SWIG_JavaException(jenv, code, msg);
SWIG_fail;}

and in ruby:

#define SWIG_fail return Qnil
#define SWIG_exception(code, msg) { SWIG_exception_((a), (b)); SWIG_fail;}

where, of course, 'Qnil' is not 0, but '4' :).


Again, this is not a problem for directors or the dirprot mode right
now, but it seems
there are several libraries (std_vector, std_pair, etc), that are using
the SWIG_exception
macro, and we now know that at least in python and java the macro
has different meanings.

And if that already was problematic with the old dirprot implementation,
the same
could be for the libraries where SWIG_exception is also used.


Other languages:

Tcl (behaves like python):
===============
#define SWIG_fail goto fail
#define SWIG_exception(a,b) { Tcl_SetResult(interp,b,TCL_VOLATILE);
SWIG_fail; }


Perl (like python):
===========
#define SWIG_fail goto fail
#define SWIG_croak(x) { SWIG_SetError(x); goto fail; }
#define SWIG_exception(a,b) SWIG_croak(b)

(but the last #define is only emitted if you include exception.i, which
is clearly
wrong since for all the other languages you always has the
SWIG_exception available).

Ruby (like java):
==========
#define SWIG_exception(code, msg) { SWIG_exception_((a), (b)); }

Chicken (like java):
============
#define SWIG_exception(a,b) SWIG_exception_((a),(b))

etc...

So, in any case, it seems that the SWIG_exception() macro needs a
cross-language revision.

Marcelo