Make PHP directors work more like other languages

A PHP exception now gets translated to a C++ exception to skips over C++
code to get back to PHP, avoiding the need to gate every directorout
typemap on EG(exception).

8 files changed, 49 insertions, 87 deletions

diff --git a/Doc/Manual/Php.html b/Doc/Manual/Php.html
index 6a53b3189..ad9773cdf 100644
--- a/Doc/Manual/Php.html
+++ b/Doc/Manual/Php.html
@@ -1159,7 +1159,12 @@ should suffice in most cases:
 <div class="code">
 <pre>
 %feature("director:except") {
-  if ($error == FAILURE) {
+#if SWIG_VERSION &gt;= 0x040100
+  if ($error != NULL)
+#else
+  if ($error == FAILURE)
+#endif
+  {
     throw Swig::DirectorMethodException();
   }
 }
@@ -1167,6 +1172,20 @@ should suffice in most cases:
 </div>
 
 <p>
+If you only need to support SWIG >= 4.1.0, you can just use the
+<tt>($error != NULL)</tt> condition.
+</p>
+
+<p>
+In SWIG 4.1.0, <tt>$error</tt> was changed in the SWIG/PHP director
+implementation to make it work more like how it does for other languages.
+Previously, <tt>$error</tt> didn't actually indicate an exception, but instead
+was only set to <tt>FAILURE</tt> if there was a problem calling the PHP method.
+Now <tt>$error</tt> indicates if the PHP method threw a PHP exception, and
+directorout typemaps for PHP no longer need to be gated by <tt>if (EG(exception))</tt>.
+</p>
+
+<p>
 This code will check the PHP error state after each method call from a
 director into PHP, and throw a C++ exception if an error occurred.  This
 exception can be caught in C++ to implement an error handler.
diff --git a/Examples/test-suite/director_exception.i b/Examples/test-suite/director_exception.i
index 71366bef0..9ff7f3842 100644
--- a/Examples/test-suite/director_exception.i
+++ b/Examples/test-suite/director_exception.i
@@ -18,22 +18,7 @@ namespace Swig {
 
 %include "std_string.i"
 
-#ifdef SWIGPHP
-
-%feature("director:except") {
-  if ($error == FAILURE) {
-    Swig::DirectorMethodException::raise("$symname");
-  }
-}
-
-%exception {
-  try { $action }
-  catch (Swig::DirectorException &) { SWIG_fail; }
-}
-
-#endif
-
-#ifdef SWIGPYTHON
+#if defined SWIGPHP || defined SWIGPYTHON
 
 %feature("director:except") {
   if ($error != NULL) {
diff --git a/Examples/test-suite/director_stl.i b/Examples/test-suite/director_stl.i
index 46946e513..cbcb4ba85 100644
--- a/Examples/test-suite/director_stl.i
+++ b/Examples/test-suite/director_stl.i
@@ -17,11 +17,7 @@
 %feature("director") Foo;
 
 %feature("director:except") {
-#ifndef SWIGPHP
   if ($error != NULL) {
-#else
-  if ($error == FAILURE) {
-#endif
     throw Swig::DirectorMethodException();
   }
 }
diff --git a/Lib/php/php.swg b/Lib/php/php.swg
index db47f49a4..496e1029e 100644
--- a/Lib/php/php.swg
+++ b/Lib/php/php.swg
@@ -93,18 +93,15 @@
 
 %typemap(directorout) SWIGTYPE ($&1_ltype tmp)
 %{
-  /* If exit was via exception, PHP NULL is returned so skip the conversion. */
-  if (!EG(exception)) {
-    if ($needNewFlow) {
-      tmp = ($&1_ltype) &SWIG_Z_FETCH_OBJ_P($1)->ptr;
-      SWIG_Z_FETCH_OBJ_P($1)->newobject = 0;
-    } else {
-      if (SWIG_ConvertPtr($input, (void **) &tmp, $&1_descriptor, 0) < 0 || tmp == NULL) {
-	SWIG_PHP_Error(E_ERROR, "Type error in argument $argnum of $symname. Expected $&1_descriptor");
-      }
+  if ($needNewFlow) {
+    tmp = ($&1_ltype) &SWIG_Z_FETCH_OBJ_P($1)->ptr;
+    SWIG_Z_FETCH_OBJ_P($1)->newobject = 0;
+  } else {
+    if (SWIG_ConvertPtr($input, (void **) &tmp, $&1_descriptor, 0) < 0 || tmp == NULL) {
+      SWIG_PHP_Error(E_ERROR, "Type error in argument $argnum of $symname. Expected $&1_descriptor");
     }
-    $result = *tmp;
   }
+  $result = *tmp;
 %}
 
 %typemap(in) SWIGTYPE *,
diff --git a/Lib/php/phprun.swg b/Lib/php/phprun.swg
index cad0f840c..2011229e4 100644
--- a/Lib/php/phprun.swg
+++ b/Lib/php/phprun.swg
@@ -55,6 +55,9 @@ static zend_always_inline void *zend_object_alloc(size_t obj_size, zend_class_en
 
 #define SWIG_fail goto fail
 
+// If there's an active PHP exception, just return so it can propagate.
+#define SWIG_FAIL() do { if (!EG(exception)) zend_error_noreturn(SWIG_ErrorCode(), "%s", SWIG_ErrorMsg()); goto thrown; } while (0)
+
 static const char *default_error_msg = "Unknown error occurred";
 static int default_error_code = E_ERROR;
 
diff --git a/Lib/php/std_string.i b/Lib/php/std_string.i
index b55751f07..082a32ce1 100644
--- a/Lib/php/std_string.i
+++ b/Lib/php/std_string.i
@@ -33,10 +33,8 @@ namespace std {
     %}
 
     %typemap(directorout) string %{
-      if (!EG(exception)) {
         convert_to_string($input);
         $result.assign(Z_STRVAL_P($input), Z_STRLEN_P($input));
-      }
     %}
 
     %typemap(out) string %{
@@ -74,12 +72,10 @@ namespace std {
     %}
 
     %typemap(directorout) string & ($*1_ltype *temp) %{
-      if (!EG(exception)) {
         convert_to_string($input);
         temp = new $*1_ltype(Z_STRVAL_P($input), Z_STRLEN_P($input));
         swig_acquire_ownership(temp);
         $result = temp;
-      }
     %}
 
     %typemap(argout) string & %{
diff --git a/Lib/php/utils.i b/Lib/php/utils.i
index d1930bf15..b8fd9091d 100644
--- a/Lib/php/utils.i
+++ b/Lib/php/utils.i
@@ -75,19 +75,15 @@
 %}
 %typemap(directorout) TYPE
 %{
-  if (!EG(exception)) {
-    CONVERT_IN($result, $1_ltype, *$input);
-  }
+  CONVERT_IN($result, $1_ltype, *$input);
 %}
 %typemap(directorout) const TYPE &
 %{
   $*1_ltype swig_val;
-  if (!EG(exception)) {
-    CONVERT_IN(swig_val, $*1_ltype, *$input);
-    $1_ltype temp = new $*1_ltype(($*1_ltype)swig_val);
-    swig_acquire_ownership(temp);
-    $result = temp;
-  }
+  CONVERT_IN(swig_val, $*1_ltype, *$input);
+  $1_ltype temp = new $*1_ltype(($*1_ltype)swig_val);
+  swig_acquire_ownership(temp);
+  $result = temp;
 %}
 %typemap(directorfree) const TYPE &
 %{
diff --git a/Source/Modules/php.cxx b/Source/Modules/php.cxx
index d96895556..ede9471f6 100644
--- a/Source/Modules/php.cxx
+++ b/Source/Modules/php.cxx
@@ -400,20 +400,6 @@ public:
     Printf(s_header, "#define SWIG_ErrorMsg() ZEND_MODULE_GLOBALS_ACCESSOR(%s, error_msg)\n", module);
     Printf(s_header, "#define SWIG_ErrorCode() ZEND_MODULE_GLOBALS_ACCESSOR(%s, error_code)\n", module);
 
-    /* The following can't go in Lib/php/phprun.swg as it uses SWIG_ErrorMsg(), etc
-     * which has to be dynamically generated as it depends on the module name.
-     */
-    Append(s_header, "#ifdef __GNUC__\n");
-    Append(s_header, "static void SWIG_FAIL(void) __attribute__ ((__noreturn__));\n");
-    Append(s_header, "#endif\n\n");
-    Append(s_header, "static void SWIG_FAIL(void) {\n");
-    Append(s_header, "    zend_error(SWIG_ErrorCode(), \"%s\", SWIG_ErrorMsg());\n");
-    // zend_error() should never return with the parameters we pass, but if it
-    // does, we really don't want to let SWIG_FAIL() return.  This also avoids
-    // a warning about returning from a function marked as "__noreturn__".
-    Append(s_header, "    abort();\n");
-    Append(s_header, "}\n\n");
-
     Printf(s_header, "static void %s_init_globals(zend_%s_globals *globals ) {\n", module, module);
     Printf(s_header, "  globals->error_msg = default_error_msg;\n");
     Printf(s_header, "  globals->error_code = default_error_code;\n");
@@ -857,7 +843,8 @@ public:
     Printf(f->code, "SWIG_ErrorCode() = E_ERROR;\n");
     Printf(f->code, "SWIG_ErrorMsg() = \"No matching function for overloaded '%s'\";\n", symname);
     Printv(f->code, "SWIG_FAIL();\n", NIL);
-
+    Printv(f->code, "thrown:\n", NIL);
+    Printv(f->code, "return;\n", NIL);
     Printv(f->code, "}\n", NIL);
     Wrapper_print(f, s_wrappers);
 
@@ -2060,25 +2047,6 @@ public:
 	p = nextSibling(p);
       }
 
-      /* exception handling */
-      bool error_used_in_typemap = false;
-      tm = Swig_typemap_lookup("director:except", n, Swig_cresult_name(), 0);
-      if (!tm) {
-	tm = Getattr(n, "feature:director:except");
-	if (tm)
-	  tm = Copy(tm);
-      }
-      if ((tm) && Len(tm) && (Strcmp(tm, "1") != 0)) {
-	if (Replaceall(tm, "$error", "error")) {
-	  /* Only declare error if it is used by the typemap. */
-	  error_used_in_typemap = true;
-	  Append(w->code, "int error = SUCCESS;\n");
-	}
-      } else {
-	Delete(tm);
-	tm = NULL;
-      }
-
       if (!idx) {
 	Printf(w->code, "zval *args = NULL;\n");
       } else {
@@ -2098,25 +2066,27 @@ public:
       Append(w->code, "#if PHP_MAJOR_VERSION < 8\n");
       Printf(w->code, "zval swig_funcname;\n");
       Printf(w->code, "ZVAL_STRINGL(&swig_funcname, \"%s\", %d);\n", funcname, strlen(funcname));
-      if (error_used_in_typemap) {
-	Append(w->code, "error = ");
-      }
       Printf(w->code, "call_user_function(EG(function_table), &swig_self, &swig_funcname, &swig_zval_result, %d, args);\n", idx);
       Append(w->code, "#else\n");
       Printf(w->code, "zend_string *swig_funcname = zend_string_init(\"%s\", %d, 0);\n", funcname, strlen(funcname));
       Append(w->code, "zend_function *swig_zend_func = zend_std_get_method(&Z_OBJ(swig_self), swig_funcname, NULL);\n");
       Append(w->code, "zend_string_release(swig_funcname);\n");
       Printf(w->code, "if (swig_zend_func) zend_call_known_instance_method(swig_zend_func, Z_OBJ(swig_self), &swig_zval_result, %d, args);\n", idx);
-      if (error_used_in_typemap) {
-	Append(w->code, "else error = FAILURE;\n");
-      }
       Append(w->code, "#endif\n");
-      Append(w->code, "}\n");
 
-      if (tm) {
+      /* exception handling */
+      tm = Swig_typemap_lookup("director:except", n, Swig_cresult_name(), 0);
+      if (!tm) {
+	tm = Getattr(n, "feature:director:except");
+	if (tm)
+	  tm = Copy(tm);
+      }
+      if ((tm) && Len(tm) && (Strcmp(tm, "1") != 0)) {
+	Replaceall(tm, "$error", "EG(exception)");
 	Printv(w->code, Str(tm), "\n", NIL);
-	Delete(tm);
       }
+      Append(w->code, "}\n");
+      Delete(tm);
 
       /* marshal return value from PHP to C/C++ type */