From 64fa88c0eb03ac37983d93a024ff375ff268361d Mon Sep 17 00:00:00 2001
From: Olly Betts <olly@survex.com>
Date: Wed, 13 Jul 2022 16:01:59 +1200
Subject: [PATCH] [php] Omit incompatible return type declaraction

Under %feature("php:type", "compat") we don't generate return type
declaration for virtual methods if directors are enabled for that class.
However if a base class of the class has a method of the same name which
isn't directed this was still getting a return type declaration which
caused PHP to give an error when it tried to load the module.

Now we detect this situation and suppress the base class method's
return type declaration too.

Re-enable testcase director_redefined which now works again (it was
failing under PHP8 due to this issue).

See #2151
---
 Examples/test-suite/php/Makefile.in |  4 ----
 Source/Modules/php.cxx              | 33 ++++++++++++++++++++---------
 2 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/Examples/test-suite/php/Makefile.in b/Examples/test-suite/php/Makefile.in
index 9e6aca3b3..6a2a4590b 100644
--- a/Examples/test-suite/php/Makefile.in
+++ b/Examples/test-suite/php/Makefile.in
@@ -26,10 +26,6 @@ C_TEST_CASES += \
 	li_cdata_carrays \
 	multivalue \
 
-# These fail with PHP8 currently.  Aiming to fix before 4.1 release.  See #2151.
-FAILING_CPP_TESTS = \
-	director_redefined
-
 include $(srcdir)/../common.mk
 
 # Overridden variables here
diff --git a/Source/Modules/php.cxx b/Source/Modules/php.cxx
index 7656e5472..72f9baa1b 100644
--- a/Source/Modules/php.cxx
+++ b/Source/Modules/php.cxx
@@ -205,6 +205,17 @@ static Hash *create_php_type_flags() {
 
 static Hash *php_type_flags = create_php_type_flags();
 
+// php_class + ":" + php_method -> PHPTypes*
+// ":" + php_function -> PHPTypes*
+static Hash *all_phptypes = NewHash();
+
+// php_class_name -> php_parent_class_name
+static Hash *php_parent_class = NewHash();
+
+// Track if a method is directed in a descendent class.
+// php_class + ":" + php_method -> boolean (using SetFlag()/GetFlag()).
+static Hash *has_directed_descendent = NewHash();
+
 // Class encapsulating the machinery to add PHP type declarations.
 class PHPTypes {
   // List with an entry for each parameter and one for the return type.
@@ -333,7 +344,7 @@ public:
     Setitem(byref, key, ""); // Just needs to be something != None.
   }
 
-  void emit_arginfo() {
+  void emit_arginfo(String *key) {
     // We want to only emit each different arginfo once, as that reduces the
     // size of both the generated source code and the compiled extension
     // module.  The parameters at this level are just named arg1, arg2, etc
@@ -346,7 +357,9 @@ public:
     // arginfo_used Hash to see if we've already generated it.
     String *out_phptype = NULL;
     String *out_phpclasses = NewStringEmpty();
-    if (php_type_flag > 0 || (php_type_flag && !has_director_node)) {
+    if (php_type_flag &&
+	(php_type_flag > 0 || !has_director_node) &&
+	!GetFlag(has_directed_descendent, key)) {
       // We provide a simple way to generate PHP return type declarations
       // except for directed methods.  The point of directors is to allow
       // subclassing in the target language, and if the wrapped method has
@@ -422,13 +435,6 @@ public:
 
 static PHPTypes *phptypes = NULL;
 
-// php_class + ":" + php_method -> PHPTypes*
-// ":" + php_function -> PHPTypes*
-static Hash *all_phptypes = NewHash();
-
-// php_class_name -> php_parent_class_name
-static Hash *php_parent_class = NewHash();
-
 class PHP : public Language {
 public:
   PHP() {
@@ -628,7 +634,7 @@ public:
     /* Emit all the arginfo */
     for (Iterator ki = First(all_phptypes); ki.key; ki = Next(ki)) {
       PHPTypes *p = (PHPTypes*)Data(ki.item);
-      p->emit_arginfo();
+      p->emit_arginfo(ki.key);
     }
 
     SwigPHP_emit_pointer_type_registrations();
@@ -1421,6 +1427,13 @@ public:
       Append(f->code, "director = SWIG_DIRECTOR_CAST(arg1);\n");
       Wrapper_add_local(f, "upcall", "bool upcall = false");
       Printf(f->code, "upcall = (director && (director->swig_get_self()==Z_OBJ_P(ZEND_THIS)));\n");
+
+      String *parent = class_name;
+      while ((parent = Getattr(php_parent_class, parent)) != NULL) {
+	// Mark this method name as having a directed descendent for all
+	// classes we're derived from.
+	SetFlag(has_directed_descendent, NewStringf("%s:%s", parent, wname));
+      }
     }
 
     Swig_director_emit_dynamic_cast(n, f);