Call for participation: Annotating internal function argument and return types View as Markdown

I'm happy to help.

Am I correct in thinking that the best way to locate outstanding ones is to
to search for any instances of ZEND_BEGIN_ARG_INFO or
ZEND_BEGIN_ARG_INFO_EX
in *.c files?
As once they've been converted they'll only appear in stub and header files?

Hi,

Lack of type information for internal functions in Reflection is a
long-standing issue. In PHP 8 we finally have all the necessary technical
groundwork done to support argument and return type annotations on internal
functions at scale.

The only thing left is to actually add those type annotations ... to many
hundreds of functions. This is something everyone can help with, as it does
not need expertise in C.

I've opened a sample PR to show the process:
https://github.com/php/php-src/pull/4499

Here, we take some existing arginfo structures in basic_functions.c and
convert them into stubs in basic_functions.stub.php. We can take the
argument names from the existing arginfo. To figure out the types, we need
to look at the implementation (the php.net documentation tends to lie about
details).

For example, the first function, set_time_limit is defined in
https://github.com/php/php-src/blob/172c71980df0fe4c9d62c3365f0a2cdb139e3e86/main/main.c#L1501.
We see that it accepts an "l" parameter, which is an int. We see that it
uses RETVAL_TRUE and RETVAL_FALSE, which means the return value is a bool.

Once this is done, we need to auto-generate new arginfo data. If you have a
development setup, this is done automatically when running "make".
Otherwise, it's possible to manually run "php scripts/dev/gen_stub.php
ext/standard/basic_functions.stub.php".

Any help would be appreciated :)

Regards,
Nikita

I’d like to help if I can.

Some extensions have what appear to be conditional arginfo (for example the LDAP extension, which I assume is to handle building against different versions of underlying ldap library that support different features) - is there a way / need to handle those in this stub format that the generator script understands?

Cheers

Stephen

Hi,

Lack of type information for internal functions in Reflection is a
long-standing issue. In PHP 8 we finally have all the necessary technical
groundwork done to support argument and return type annotations on internal
functions at scale.

Question - If every function needs looking through and parsing, would
this be a suitable time to move from the ZPP varadic to the macros?

Hi,

Lack of type information for internal functions in Reflection is a
long-standing issue. In PHP 8 we finally have all the necessary technical
groundwork done to support argument and return type annotations on internal
functions at scale.

The only thing left is to actually add those type annotations ... to many
hundreds of functions. This is something everyone can help with, as it does
not need expertise in C.

I've opened a sample PR to show the process:
https://github.com/php/php-src/pull/4499

Here, we take some existing arginfo structures in basic_functions.c and
convert them into stubs in basic_functions.stub.php. We can take the
argument names from the existing arginfo. To figure out the types, we need
to look at the implementation (the php.net documentation tends to lie
about details).

For example, the first function, set_time_limit is defined in
https://github.com/php/php-src/blob/172c71980df0fe4c9d62c3365f0a2cdb139e3e86/main/main.c#L1501.
We see that it accepts an "l" parameter, which is an int. We see that it
uses RETVAL_TRUE and RETVAL_FALSE, which means the return value is a bool.

Once this is done, we need to auto-generate new arginfo data. If you have
a development setup, this is done automatically when running "make".
Otherwise, it's possible to manually run "php scripts/dev/gen_stub.php
ext/standard/basic_functions.stub.php".

Any help would be appreciated :)

A couple of things I didn't mention but noticed during reviews:

• We can't add return types to methods (at least for non-final classes),
  because these would force inheriting classes to specify the return type as
  well. So return types on methods should be specified using @return for now
  -- we should discuss if we want to add them as proper types (with the BC
  break that implies).

• For by-reference parameters, very likely no type should be specified.
  This is because types are always input types. These are usually not
  constrained for reference parameters. The output type may be constrained,
  but there's no way to specify it in PHP right now.

• Many internal functions have defaults that cannot be expressed as a
  constant expression, e.g. because they depend on ini settings or other
  parameters. In this cases by convention "$foo = UNKNOWN" is used, just to
  mark that the argument is optional, but doesn't have a "proper" default
  value.

Nikita

Lack of type information for internal functions in Reflection is a
long-standing issue. In PHP 8 we finally have all the necessary technical
groundwork done to support argument and return type annotations on internal
functions at scale.

Well I tried to put together a very very ugly hack-job of a C
tokeniser/parser that could infer types based on a combination of
old-style ZPP and macros used to access them
(https://github.com/marandall/cpti) but too many hours later I came to
the conclusion that to get the job done properly would probably require
building out a full C AST parser to be able to handle things like
default values.

I did notice a few things in the process of making it (not least that
IMO GD needs several hundred things changing to throw
UnexpectedValueExceptions).

I noticed that in some cases, the internal constants don't match the
userland constants, for example the array functions use PHP_SORT_REGULAR
but the userland variants are just SORT_REGULAR. Which one would be used
for the stubs? In the example from Nikita the flag names do match
(PHP_OUTPUT_HANDLER_STDFLAGS).

I noticed you're documenting them as string|false rather than
string|bool. I assume this is something to do with parsing "false" to be
an error condition?

Before I re-clone the php-src repo and start editing, I just wanted to
check if there had been any consideration to opening up the protocol
style to add basic comments?

One thing I did see in the GD library with _php_image_create_from is
that ZPP is different depending on logical expressions, rather than
pre-processor statements. How should these be handled?

Mark Randall

Hi,

I'd be happy to donate some time to this effort as well. If you just
assign me a file or directory (not too much at once please) I can create
a PR that covers that part of the code base.

Would that work for you?

Regards,
Dik

Hi,

Lack of type information for internal functions in Reflection is a
long-standing issue. In PHP 8 we finally have all the necessary technical
groundwork done to support argument and return type annotations on internal
functions at scale.

The only thing left is to actually add those type annotations ... to many
hundreds of functions. This is something everyone can help with, as it does
not need expertise in C.

I've opened a sample PR to show the process:
https://github.com/php/php-src/pull/4499

Here, we take some existing arginfo structures in basic_functions.c and
convert them into stubs in basic_functions.stub.php. We can take the
argument names from the existing arginfo. To figure out the types, we need
to look at the implementation (the php.net documentation tends to lie about
details).

For example, the first function, set_time_limit is defined in
https://github.com/php/php-src/blob/172c71980df0fe4c9d62c3365f0a2cdb139e3e86/main/main.c#L1501.
We see that it accepts an "l" parameter, which is an int. We see that it
uses RETVAL_TRUE and RETVAL_FALSE, which means the return value is a bool.

Once this is done, we need to auto-generate new arginfo data. If you have a
development setup, this is done automatically when running "make".
Otherwise, it's possible to manually run "php scripts/dev/gen_stub.php
ext/standard/basic_functions.stub.php".

Any help would be appreciated :)

Regards,
Nikita

Hi,

Lack of type information for internal functions in Reflection is a
long-standing issue. In PHP 8 we finally have all the necessary technical
groundwork done to support argument and return type annotations on internal
functions at scale.

The only thing left is to actually add those type annotations ... to many
hundreds of functions. This is something everyone can help with, as it does
not need expertise in C.

I've opened a sample PR to show the process:
https://github.com/php/php-src/pull/4499

Here, we take some existing arginfo structures in basic_functions.c and
convert them into stubs in basic_functions.stub.php. We can take the
argument names from the existing arginfo. To figure out the types, we need
to look at the implementation (the php.net documentation tends to lie
about details).

For example, the first function, set_time_limit is defined in
https://github.com/php/php-src/blob/172c71980df0fe4c9d62c3365f0a2cdb139e3e86/main/main.c#L1501.
We see that it accepts an "l" parameter, which is an int. We see that it
uses RETVAL_TRUE and RETVAL_FALSE, which means the return value is a bool.

Once this is done, we need to auto-generate new arginfo data. If you have
a development setup, this is done automatically when running "make".
Otherwise, it's possible to manually run "php scripts/dev/gen_stub.php
ext/standard/basic_functions.stub.php".

Any help would be appreciated :)

Regards,
Nikita

Thanks everyone for your help with this project!

I think at this point we have about half of the extensions covered, here's
a hopefully up to date list of the extensions that are done and those that
are still missing stubs:

Complete:
ext/bcmath
ext/bz2
ext/calendar
ext/com_dotnet
ext/ctype
ext/curl
ext/date
ext/enchant
ext/filter
ext/ftp
ext/gd
ext/gettext
ext/gmp
ext/iconv
ext/json
ext/openssl
ext/pcre
ext/posix
ext/readline
ext/shmop
ext/sqlite3
ext/sysvmsg
ext/sysvsem
ext/sysvshm
ext/tokenizer
ext/zip
ext/zlib

Pending PRs:
ext/exif
ext/fileinfo
ext/ldap
ext/session
ext/simplexml

Incomplete:
ext/standard

Missing:
ext/dba
ext/dom
ext/ffi
ext/hash
ext/imap
ext/intl
ext/libxml
ext/mbstring
ext/mysqli
ext/mysqlnd
ext/oci8
ext/odbc
ext/opcache
ext/pcntl
ext/pdo
ext/pdo_*
ext/pgsql
ext/phar
ext/pspell
ext/reflection
ext/snmp
ext/soap
ext/sockets
ext/sodium
ext/spl
ext/tidy
ext/xml
ext/xmlreader
ext/xmlrpc
ext/xmlwriter
ext/xsl

Regards,
Nikita