Merge branch 'devel' of https://github.com/ewmailing/swig-v8 into devel

Lib/javascript/jsc/javascriptcode.swg

@@ -113,6 +113,25 @@ void $jswrapper(JSObjectRef thisObject)
 }
 %}
 
+/* -----------------------------------------------------------------------------
+ * js_dtor:  template for a destructor wrapper
+ *   - $jsmangledname:  mangled class name
+ *   - $jstype:         class type
+ *   - ${destructor_action}: The custom destructor action to invoke.
+ * ----------------------------------------------------------------------------- */
+%fragment ("js_dtoroverride", "templates")
+%{
+void $jswrapper(JSObjectRef thisObject)
+{
+  SWIG_PRV_DATA* t = (SWIG_PRV_DATA*) JSObjectGetPrivate(thisObject);
+  if(t && t->swigCMemOwn) {
+    $jstype* arg1 = ($jstype*)t->swigCObject;
+    ${destructor_action}
+  }
+  if(t) free(t);
+}
+%}
+
 /* -----------------------------------------------------------------------------
  * js_getter:  template for getter function wrappers
  *   - $jswrapper:  wrapper function name

Lib/javascript/v8/javascriptcode.swg

@@ -112,14 +112,14 @@ fail:
 %fragment ("js_dtor", "templates")
 %{
 
-#ifdef BUILDING_NODE_EXTENSION
+#if defined(BUILDING_NODE_EXTENSION) || defined(V8_3_14)
 void $jswrapper(v8::Persistent< v8::Value > object, void *parameter) 
 #else
 void $jswrapper(v8::Isolate *isolate, v8::Persistent< v8::Object > * object, SWIGV8_Proxy *proxy) 
 #endif
 {
 
-#ifdef BUILDING_NODE_EXTENSION
+#if defined(BUILDING_NODE_EXTENSION) || defined(V8_3_14)
   SWIGV8_Proxy *proxy = static_cast<SWIGV8_Proxy *>(parameter); 
 #endif
 
@@ -132,7 +132,7 @@ void $jswrapper(v8::Isolate *isolate, v8::Persistent< v8::Object > * object, SWI
   delete proxy;
 
 
-#ifdef BUILDING_NODE_EXTENSION
+#if defined(BUILDING_NODE_EXTENSION) || defined(V8_3_14)
   object.Clear();
   object.Dispose();
 #else
@@ -142,6 +142,27 @@ void $jswrapper(v8::Isolate *isolate, v8::Persistent< v8::Object > * object, SWI
 }
 %}
 
+/* -----------------------------------------------------------------------------
+ * js_dtoroverride:  template for a destructor wrapper
+ *   - $jsmangledname:  mangled class name
+ *   - $jstype:         class type
+ *   - ${destructor_action}: The custom destructor action to invoke. 
+ * ----------------------------------------------------------------------------- */
+%fragment ("js_dtoroverride", "templates")
+%{
+void $jswrapper(v8::Persistent< v8::Value > object, void *parameter) {
+  SWIGV8_Proxy* proxy = (SWIGV8_Proxy*) parameter;
+  if(proxy->swigCMemOwn && proxy->swigCObject) {
+    $jstype arg1 = ($jstype)proxy->swigCObject;
+    ${destructor_action}
+  }
+  delete proxy;
+
+  object.Clear();
+  object.Dispose();
+}
+%}
+
 /* -----------------------------------------------------------------------------
  * js_getter:  template for getter function wrappers
  *   - $jswrapper:  wrapper function name
@@ -281,7 +302,7 @@ fail:
 %fragment("jsv8_define_class_template", "templates") 
 %{
   v8::Handle<v8::FunctionTemplate> $jsmangledname_class = SWIGV8_CreateClassTemplate("$jsmangledname");
-#ifdef BUILDING_NODE_EXTENSION
+#if defined(BUILDING_NODE_EXTENSION) || defined(V8_3_14)
   $jsmangledname_clientData.class_templ = v8::Persistent<v8::FunctionTemplate>::New($jsmangledname_class);
 #else
   $jsmangledname_clientData.class_templ.Reset(v8::Isolate::GetCurrent(), $jsmangledname_class);
@@ -300,7 +321,7 @@ fail:
 %{
   if (SWIGTYPE_p$jsbaseclass->clientdata && !(static_cast<SWIGV8_ClientData *>(SWIGTYPE_p$jsbaseclass->clientdata)->class_templ.IsEmpty()))
   {
-#ifdef BUILDING_NODE_EXTENSION
+#if defined(BUILDING_NODE_EXTENSION) || defined(V8_3_14)
     $jsmangledname_class->Inherit(static_cast<SWIGV8_ClientData *>(SWIGTYPE_p$jsbaseclass->clientdata)->class_templ);
 #else
     $jsmangledname_class->Inherit(
@@ -429,9 +450,10 @@ fail:
  * ----------------------------------------------------------------------------- */
 %fragment("js_initializer", "templates") 
 %{
-extern "C" {
+// The extern "C" makes little sense here because the paramater is using C++ objects and templates.
+// extern "C" {
 
-#ifdef BUILDING_NODE_EXTENSION
+#if defined(BUILDING_NODE_EXTENSION)
 void $jsname_initialize(v8::Handle<v8::Object> global_obj, v8::Handle<v8::Object> /*module*/)
 #else
 void $jsname_initialize(v8::Handle<v8::Object> global_obj)
@@ -443,7 +465,7 @@ void $jsname_initialize(v8::Handle<v8::Object> global_obj)
 
   // a class template for creating proxies of undefined types
 
-#ifdef BUILDING_NODE_EXTENSION
+#if defined(BUILDING_NODE_EXTENSION) || defined(V8_3_14)
   SWIGV8_SWIGTYPE_Proxy_class_templ = v8::Persistent<v8::FunctionTemplate>::New(SWIGV8_CreateClassTemplate("SwigProxy"));
 #else
   SWIGV8_SWIGTYPE_Proxy_class_templ.Reset(v8::Isolate::GetCurrent(), SWIGV8_CreateClassTemplate("SwigProxy"));
@@ -475,11 +497,11 @@ void $jsname_initialize(v8::Handle<v8::Object> global_obj)
   $jsv8registernspaces
 }
 
-#ifdef BUILDING_NODE_EXTENSION
+#if defined(BUILDING_NODE_EXTENSION)
 NODE_MODULE($jsname, $jsname_initialize);
 #endif
 
 
 
-} // extern "C"
+// } // extern "C"
 %}

Lib/javascript/v8/javascriptinit.swg

@@ -1,5 +1,6 @@
 %insert(init) %{
 
+#include <assert.h>
 
 SWIGRUNTIME void
 SWIG_V8_SetModule(void *, swig_module_info *swig_module) {

Lib/javascript/v8/javascriptruntime.swg

@@ -40,6 +40,7 @@ void SWIG_V8_Raise(const char* msg) {
 */
 class V8ErrorHandler {
 public:
+  virtual ~V8ErrorHandler() {}
   virtual void error(int code, const char* msg) {
     SWIG_V8_Raise(msg);
   }
@@ -73,7 +74,7 @@ public:
   };
   
   ~SWIGV8_Proxy() { 
-#ifdef BUILDING_NODE_EXTENSION
+#if defined(BUILDING_NODE_EXTENSION) || defined(V8_3_14)
     handle.ClearWeak();
 #else
     handle.ClearWeak(v8::Isolate::GetCurrent());
@@ -81,7 +82,7 @@ public:
 
     //handle->SetInternalField(0, v8::Undefined());
 
-#ifdef BUILDING_NODE_EXTENSION
+#if defined(BUILDING_NODE_EXTENSION) || defined(V8_3_14)
     handle.Dispose();
 #else
     handle.Dispose(v8::Isolate::GetCurrent());
@@ -100,7 +101,7 @@ class SWIGV8_ClientData {
 public:
   v8::Persistent<v8::FunctionTemplate> class_templ;
 
-#ifdef BUILDING_NODE_EXTENSION
+#if defined(BUILDING_NODE_EXTENSION) || defined(V8_3_14)
   void (*dtor) (v8::Persistent< v8::Value> object, void *parameter);
 #else
   void (*dtor) (v8::Isolate *isolate, v8::Persistent< v8::Object > *object, SWIGV8_Proxy *proxy);
@@ -114,7 +115,7 @@ int SWIG_V8_ConvertInstancePtr(v8::Handle<v8::Object> objRef, void** ptr, swig_t
   
   if(objRef->InternalFieldCount() < 1) return SWIG_ERROR;
 
-#ifdef BUILDING_NODE_EXTENSION
+#if defined(BUILDING_NODE_EXTENSION) || defined(V8_3_14)
   v8::Handle<v8::Value> cdataRef = objRef->GetInternalField(0);
   SWIGV8_Proxy *cdata = static_cast<SWIGV8_Proxy *>(v8::External::Unwrap(cdataRef));
 #else
@@ -141,13 +142,13 @@ int SWIG_V8_ConvertInstancePtr(v8::Handle<v8::Object> objRef, void** ptr, swig_t
   return SWIG_OK;
 }
 
-#ifdef BUILDING_NODE_EXTENSION
+#if defined(BUILDING_NODE_EXTENSION) || defined(V8_3_14)
 void SWIGV8_Proxy_DefaultDtor(v8::Persistent< v8::Value > object, void *parameter)
 #else
 void SWIGV8_Proxy_DefaultDtor(v8::Isolate *, v8::Persistent< v8::Object > *object, SWIGV8_Proxy *proxy)
 #endif
 {
-#ifdef BUILDING_NODE_EXTENSION
+#if defined(BUILDING_NODE_EXTENSION) || defined(V8_3_14)
   SWIGV8_Proxy *proxy = static_cast<SWIGV8_Proxy *>(parameter);
 #endif
 
@@ -160,7 +161,7 @@ void SWIGV8_SetPrivateData(v8::Handle<v8::Object> obj, void* ptr, swig_type_info
   cdata->swigCMemOwn = (flags & SWIG_POINTER_OWN) ? 1 : 0;
   cdata->info = info;
 
-#ifdef BUILDING_NODE_EXTENSION
+#if defined(BUILDING_NODE_EXTENSION) || defined(V8_3_14)
   obj->SetPointerInInternalField(0, cdata);
   cdata->handle = v8::Persistent<v8::Object>::New(obj);
 #else
@@ -168,16 +169,16 @@ void SWIGV8_SetPrivateData(v8::Handle<v8::Object> obj, void* ptr, swig_type_info
   cdata->handle.Reset(v8::Isolate::GetCurrent(), obj);
 #endif
 
-#ifdef BUILDING_NODE_EXTENSION
+#if defined(BUILDING_NODE_EXTENSION) || defined(V8_3_14)
   // clientdata must be set for owned data as we need to register the dtor
-  if(cdata->swigCMemOwn) {
+  if(cdata->swigCMemOwn && (SWIGV8_ClientData*)info->clientdata) {
     cdata->handle.MakeWeak(cdata, ((SWIGV8_ClientData*)info->clientdata)->dtor);
   } else {
     cdata->handle.MakeWeak(cdata, SWIGV8_Proxy_DefaultDtor);
   }
   cdata->handle.MarkIndependent();
 #else
-  if(cdata->swigCMemOwn) {
+  if(cdata->swigCMemOwn && (SWIGV8_ClientData*)info->clientdata) {
     cdata->handle.MakeWeak(v8::Isolate::GetCurrent(), cdata, ((SWIGV8_ClientData*)info->clientdata)->dtor);
   } else {
     cdata->handle.MakeWeak(v8::Isolate::GetCurrent(), cdata, SWIGV8_Proxy_DefaultDtor);
@@ -201,7 +202,7 @@ v8::Handle<v8::Object> SWIG_V8_NewPointerObj(void *ptr, swig_type_info *info, in
   v8::Handle<v8::FunctionTemplate> class_templ;
 
 
-#ifdef BUILDING_NODE_EXTENSION
+#if defined(BUILDING_NODE_EXTENSION) || defined(V8_3_14)
   if(info->clientdata != 0) {
     class_templ = ((SWIGV8_ClientData*) info->clientdata)->class_templ;
   } else {

Source/Modules/javascript.cxx

@@ -880,7 +880,6 @@ int JSEmitter::emitCtor(Node *n) {
 
 int JSEmitter::emitDtor(Node *n) {
 
-  Template t_dtor = getTemplate("js_dtor");
   String *wrap_name = Swig_name_wrapper(Getattr(n, "sym:name"));
 
   SwigType *type = state.clazz(TYPE);
@@ -888,20 +887,116 @@ int JSEmitter::emitDtor(Node *n) {
   String *ctype = SwigType_lstr(p_classtype, "");
   String *free = NewString("");
 
+   // (Taken from JSCore implementation.)
+  /* The if (Extend) block was taken from the Ruby implementation.
+   * The problem is that in the case of an %extend to create a destructor for a struct to coordinate automatic memory cleanup with the Javascript collector,
+   * the swig function was not being generated. More specifically:
+     struct MyData {
+       %extend {
+         ~MyData() {
+           FreeData($self);
+         }
+       }
+     };
+     %newobject CreateData;	 
+     struct MyData* CreateData(void);
+     %delobject FreeData;
+     void FreeData(struct MyData* the_data);
+	 
+	 where the use case is something like:
+	   var my_data = example.CreateData();
+	   my_data = null;
+     
+	 This function was not being generated:
+	 SWIGINTERN void delete_MyData(struct MyData *self){
+       FreeData(self);
+     }
+
+     I don't understand fully why it wasn't being generated. It just seems to happen in the Lua generator.
+	 There is a comment about staticmemberfunctionHandler having an inconsistency and I tracked down dome of the SWIGINTERN void delete_*
+	 code to that function in the Language base class.
+	 The Ruby implementation seems to have an explicit check for if(Extend) and explicitly generates the code, so that's what I'm doing here.
+	 The Ruby implementation does other stuff which I omit.
+  */
+  if (Extend) {
+    String *wrap = Getattr(n, "wrap:code");
+    if (wrap) {
+      Printv(f_wrappers, wrap, NIL);
+    }
+  }
+
+
   // HACK: this is only for the v8 emitter. maybe set an attribute wrap:action of node
   // TODO: generate dtors more similar to other wrappers
+  // EW: I think this is wrong. delete should only be used when new was used to create. If malloc was used, free needs to be used.
   if(SwigType_isarray(type)) {
     Printf(free, "delete [] (%s)", ctype);
   } else {
     Printf(free, "delete (%s)", ctype);
   }
 
-  state.clazz(DTOR, wrap_name);
+  String* destructor_action = Getattr(n, "wrap:action");
-  t_dtor.replace(T_NAME_MANGLED, state.clazz(NAME_MANGLED))
+  // Adapted from the JSCore implementation.
+  /* The next challenge is to generate the correct finalize function for JavaScriptCore to call.
+     Originally, it would use this fragment from javascriptcode.swg
+	 %fragment ("JS_destructordefn", "templates")
+     %{
+     void _wrap_${classname_mangled}_finalize(JSObjectRef thisObject)
+     {
+        SWIG_PRV_DATA* t = (SWIG_PRV_DATA*)JSObjectGetPrivate(thisObject);
+        if(t && t->swigCMemOwn) free ((${type}*)t->swigCObject);
+        if(t) free(t);
+     }
+     %}
+
+     But for the above example case of %extend to define a destructor on a struct, we need to override the system to not call
+     free ((${type}*)t->swigCObject);
+	 and substitute it with what the user has provided.
+     To solve this, I created a variation fragment called JS_destructoroverridedefn:
+     SWIG_PRV_DATA* t = (SWIG_PRV_DATA*)JSObjectGetPrivate(thisObject);
+     if(t && t->swigCMemOwn) {
+       ${type}* arg1 = (${type}*)t->swigCObject;
+       ${destructor_action}
+     }
+     if(t) free(t);
+
+	 Based on what I saw in the Lua and Ruby modules, I use Getattr(n, "wrap:action")
+	 to decide if the user has a preferred destructor action.
+	 Based on that, I decide which fragment to use.
+	 And in the case of the custom action, I substitute that action in.
+	 I noticed that destructor_action has the form 
+       delete_MyData(arg1);
+     The explicit arg1 is a little funny, so I structured the fragment to create a temporary variable called arg1 to make the generation easier.
+     This might suggest this solution misunderstands a more complex case.
+
+	 Also, there is a problem where destructor_action is always true for me, even when not requesting %extend as above.
+	 So this code doesn't actually quite work as I expect. The end result is that the code still works because 
+	 destructor_action calls free like the original template. The one caveat is the string in destructor_action casts to char* which is wierd.
+	 I think there is a deeper underlying SWIG issue because I don't think it should be char*. However, it doesn't really matter for free.
+
+	 Maybe the fix for the destructor_action always true problem is that this is supposed to be embedded in the if(Extend) block above.
+	 But I don't fully understand the conditions of any of these things, and since it works for the moment, I don't want to break more stuff.
+  */
+  if(destructor_action) {
+    Template t_dtor = getTemplate("js_dtoroverride");
+    state.clazz(DTOR, wrap_name);
+    t_dtor.replace("${classname_mangled}", state.clazz(NAME_MANGLED))
+      .replace(T_WRAPPER, wrap_name)
+      .replace(T_FREE, free)
+      .replace(T_TYPE, ctype);
+
+    t_dtor.replace("${destructor_action}", destructor_action);
+    Wrapper_pretty_print(t_dtor.str(), f_wrappers);
+  }
+  else {
+    Template t_dtor = getTemplate("js_dtor");
+    state.clazz(DTOR, wrap_name);
+    t_dtor.replace(T_NAME_MANGLED, state.clazz(NAME_MANGLED))
       .replace(T_WRAPPER, wrap_name)
       .replace(T_FREE, free)
       .replace(T_TYPE, ctype)
       .pretty_print(f_wrappers);
+  }
 
   Delete(p_classtype);
   Delete(ctype);
@@ -1190,6 +1285,16 @@ void JSEmitter::emitCleanupCode(Node *n, Wrapper *wrapper, ParmList *params) {
   Parm *p;
   String *tm;
 
+  for (p = params; p;) {
+    if ((tm = Getattr(p, "tmap:argout"))) {
+      Replaceall(tm, "$input", Getattr(p, "emit:input"));
+      Printv(wrapper->code, tm, "\n", NIL);
+      p = Getattr(p, "tmap:argout:next");
+    } else {
+      p = nextSibling(p);
+    }
+  }
+
   for (p = params; p;) {
     if ((tm = Getattr(p, "tmap:freearg"))) {
       //addThrows(n, "tmap:freearg", p);