Add DNF types support for internal functions

[ju1ius]

Description

Hi, sorry if I'm mistaking, but gen_stub.php doesn't seem to support DNF types ATM.

Input stub file:

<?php
/**
 * @generate-class-entries
 */
class DnfClass {
  public array|(Traversable&Countable)|\Iterator $dnfProp;
  public function dnf(array|(Traversable&Countable)|Iterator $dnf): void {}
}

$ php build/gen_stub.php -f --generate-classsynopses --generate-methodsynopses

Generated arginfo header:

ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_DnfClass_dnf, 0, 1, IS_VOID, 0)
  ZEND_ARG_OBJ_TYPE_MASK(0, dnf, Traversable|Countable|Iterator, MAY_BE_ARRAY, NULL)
ZEND_END_ARG_INFO()

ZEND_METHOD(DnfClass, dnf);

static const zend_function_entry class_DnfClass_methods[] = {
  ZEND_ME(DnfClass, dnf, arginfo_class_DnfClass_dnf, ZEND_ACC_PUBLIC)
  ZEND_FE_END
};

static zend_class_entry *register_class_DnfClass(void) {
  zend_class_entry ce, *class_entry;

  INIT_CLASS_ENTRY(ce, "DnfClass", class_DnfClass_methods);
  class_entry = zend_register_internal_class_ex(&ce, NULL);

  zend_string *property_dnfProp_class_Traversable = zend_string_init("Traversable", sizeof("Traversable") - 1, 1);
  zend_string *property_dnfProp_class_Countable = zend_string_init("Countable", sizeof("Countable") - 1, 1);
  zend_string *property_dnfProp_class_Iterator = zend_string_init("Iterator", sizeof("Iterator") - 1, 1);
  zend_type_list *property_dnfProp_type_list = malloc(ZEND_TYPE_LIST_SIZE(3));
  property_dnfProp_type_list->num_types = 3;
  property_dnfProp_type_list->types[0] = (zend_type) ZEND_TYPE_INIT_CLASS(property_dnfProp_class_Traversable, 0, 0);
  property_dnfProp_type_list->types[1] = (zend_type) ZEND_TYPE_INIT_CLASS(property_dnfProp_class_Countable, 0, 0);
  property_dnfProp_type_list->types[2] = (zend_type) ZEND_TYPE_INIT_CLASS(property_dnfProp_class_Iterator, 0, 0);
  zend_type property_dnfProp_type = ZEND_TYPE_INIT_UNION(property_dnfProp_type_list, MAY_BE_ARRAY);

  zval property_dnfProp_default_value;
  ZVAL_UNDEF(&property_dnfProp_default_value);
  zend_string *property_dnfProp_name = zend_string_init("dnfProp", sizeof("dnfProp") - 1, 1);
  zend_declare_typed_property(class_entry, property_dnfProp_name, &property_dnfProp_default_value, ZEND_ACC_PUBLIC, NULL, property_dnfProp_type);
  zend_string_release(property_dnfProp_name);

  return class_entry;
}

Generated class/method synopses

<classsynopsis>
  <!-- header ommited -->
  <classsynopsisinfo role="comment">&Properties;</classsynopsisinfo>
  <fieldsynopsis>
    <modifier>public</modifier>
    <type class="union">
      <type>array</type><type>Traversable</type><type>Countable</type><type>Iterator</type>
    </type>
    <varname linkend="dnfclass.props.dnfprop">dnfProp</varname>
  </fieldsynopsis>

  <classsynopsisinfo role="comment">&Methods;</classsynopsisinfo>
  <methodsynopsis role="DnfClass">
    <modifier>public</modifier> <type>void</type><methodname>DnfClass::dnf</methodname>
    <methodparam>
        <type class="union">
          <type>array</type><type>Traversable</type><type>Countable</type><type>Iterator</type> 
        </type>
        <parameter>dnf</parameter></methodparam>
  </methodsynopsis>

</classsynopsis>

Expected class/method synopses

  <fieldsynopsis>
    <modifier>public</modifier>
    <type class="union">
      <type>array</type>
      <type class="intersection">
        <type>Traversable</type><type>Countable</type>
      </type>
      <type>Iterator</type>
    </type>
    <varname linkend="dnfclass.props.dnfprop">dnfProp</varname>
  </fieldsynopsis>

  <methodsynopsis role="DnfClass">
    <modifier>public</modifier> <type>void</type><methodname>DnfClass::dnf</methodname>
    <methodparam>
        <type class="union">
          <type>array</type>
          <type class="intersection">
            <type>Traversable</type><type>Countable</type>
          </type>
          <type>Iterator</type>
        </type>
        <parameter>dnf</parameter></methodparam>
  </methodsynopsis>

Expected arginfo header

I'm not super familiar with the new DNF macros API, so WRT the arginfo for DnfClass::dnf() IDK, but for the DnfClass::$dnfProp property I expected the output to look more like the following:

// <snip>, names shortened for readability
zend_string *class_Traversable = zend_string_init("Traversable", sizeof("Traversable") - 1, 1);
zend_string *class_Countable = zend_string_init("Countable", sizeof("Countable") - 1, 1);
zend_string *class_Iterator = zend_string_init("Iterator", sizeof("Iterator") - 1, 1);

zend_type_list *traversable_countable_list = malloc(ZEND_TYPE_LIST_SIZE(2));
traversable_countable_list->num_types = 2;
traversable_countable_list->types[0] = (zend_type) ZEND_TYPE_INIT_CLASS(class_Traversable, 0, 0);
traversable_countable_list->types[1] = (zend_type) ZEND_TYPE_INIT_CLASS(class_Countable, 0, 0);
zend_type traversable_countable_type = ZEND_TYPE_INIT_INTERSECTION(traversable_countable_list, 0);

zend_type_list *dnfProp_type_list = malloc(ZEND_TYPE_LIST_SIZE(2));
dnfProp_type_list->num_types = 2;
dnfProp_type_list->types[0] = traversable_countable_type;
dnfProp_type_list->types[1] = (zend_type) ZEND_TYPE_INIT_CLASS(class_Iterator, 0, 0);
zend_type dnfProp_type = ZEND_TYPE_INIT_UNION(dnfProp_type_list, MAY_BE_ARRAY);
// </snip>

PHP Version

PHP-8.2 (master branch)

Operating System

irrelevant

[cmb69]

@kocsismate, are there plans to add DNF support?

[Girgias]

We don't even support intersection types properly in the stubs, and the union type support is a bit of a hack as far as I understand (see discussion in #9100).

So this behaviour is totally expected at the time being.

[ju1ius]

@Girgias After reading the comments in the aforementioned PR, I would be tempted to conclude that DNF and intersection types are not supported for internal functions, and that union type support is limited to only a single class type.

Is it correct ?

[ju1ius]

FWIW, I tried registering functions w/ dnf-typed arguments manually (without gen_stubs.php):

• on a debug build, it fails because of this assertion:

if (ZEND_TYPE_IS_COMPLEX(new_arg_info[i].type))
  ZEND_ASSERT(ZEND_TYPE_HAS_NAME(new_arg_info[i].type) && "Should be stored as simple name");

Which is obviously wrong since new_arg_info[i].type->ptr can now point to a zend_type_list...

• on a non-debug build, it gets passed the assertion but (obviously) produces garbage and segfaults...

IMO, gen_stubs.php should handle internal functions the same way it handles properties: generate a proper c function returning a valid zend_internal_arg_info[].

[ju1ius]

With the following patch:

diff --git a/Zend/zend_API.c b/Zend/zend_API.c
index b74fa0fc7a..7032db1dfa 100644
--- a/Zend/zend_API.c
+++ b/Zend/zend_API.c
@@ -2858,9 +2858,11 @@ ZEND_API zend_result zend_register_functions(zend_class_entry *scope, const zend
                        memcpy(new_arg_info, arg_info, sizeof(zend_arg_info) * num_args);
                        reg_function->common.arg_info = new_arg_info + 1;
                        for (i = 0; i < num_args; i++) {
-                               if (ZEND_TYPE_IS_COMPLEX(new_arg_info[i].type)) {
-                                       ZEND_ASSERT(ZEND_TYPE_HAS_NAME(new_arg_info[i].type)
-                                               && "Should be stored as simple name");
+                               if (ZEND_TYPE_HAS_LIST(new_arg_info[i].type)) {
+                                       // assume type has been correctly defined
+                                       continue;
+                               }
+                               if (ZEND_TYPE_HAS_NAME(new_arg_info[i].type)) {
                                        const char *class_name = ZEND_TYPE_LITERAL_NAME(new_arg_info[i].type);

                                        size_t num_types = 1;

I can manually register DNF types for my internal functions.

[Girgias]

No one is saying it shouldn 't, but we haven't got round to it nor thought about it.

Moreover, your patch just completely ignores the arginfo and means the DNF types is completely useless.

[ju1ius]

No one is saying it shouldn 't, but we haven't got round to it nor thought about it.

I never said that anyone said otherwise either, I was just stating an opinion. 😉
And given the complexity of the task, I perfectly understand that nobody found the time to attempt to refactor this.

Moreover, your patch just completely ignores the arginfo and means the DNF types is completely useless.

Mmm... May I humbly suggest that you misunderstood this patch?
The arginfo is not ignored at all since it is already memcpy'ed into a new arg_info at this point...

So far, the only way that this part of the code can encounter a zend_type whose ptr field is a zend_type_list* is if the extension author has allocated it himself (I mean without the code generated by gen_stubs or the various zend macros). So the patch just faithfully assumes that the author has provided a correct union/intersection type definition, instead of the current behaviour that tries to parse the zend_type_list* as a const char*, thus corrupting memory.

I tested the patch with a test extension and the generated ReflectionTypes are correct. But of course I might have overlooked something and it might only be working by sheer accident. 😆

Anyway, not saying that this patch is any good for inclusion anywhere, just trying to be helpful with the limited means I have.