Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:86773
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.212.171 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <5587334C.2080401@lerdorf.com>
References: <5586E5D1.4010901@lerdorf.com>
	<CAF+90c9Nxr70100SeBKgaTGGyD6MDQjWYSvjn6ZdkaJbrCZqLA@mail.gmail.com>
	<55870947.4060304@lerdorf.com>
	<CAJW__o1fGEszJ7c3oP7wz0=TGWwmwKKYS823=7yz12Pm6Uej4A@mail.gmail.com>
	<5587334C.2080401@lerdorf.com>
Date: Mon, 22 Jun 2015 00:04:54 +0200
Message-ID: <CAF+90c_gkv=u5+4kf1aHJ_KE+ULP20Y-0hgGU2Zu+vXvhjp-qg@mail.gmail.com>
To: Rasmus Lerdorf <rasmus@lerdorf.com>
Cc: Kalle Sommer Nielsen <kalle@php.net>, PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=047d7bf0bfc2ddc84e05190e583c
Subject: Re: [PHP-DEV] hasType() for internal function parameters?
From: nikita.ppv@gmail.com (Nikita Popov)

--047d7bf0bfc2ddc84e05190e583c
Content-Type: text/plain; charset=UTF-8

On Sun, Jun 21, 2015 at 11:57 PM, Rasmus Lerdorf <rasmus@lerdorf.com> wrote:

> On 06/21/2015 03:40 PM, Kalle Sommer Nielsen wrote:
> > Hi
> >
> > 2015-06-21 20:58 GMT+02:00 Rasmus Lerdorf <rasmus@lerdorf.com>:
> >> Or we bite the bullet and fix the arginfo everywhere to make it
> >> consistent. It wouldn't be that hard to write a script to generate the
> >> bulk of it.
> >
> > I think fixing arginfo sounds like the right solution, writing a
> > script for it should not be so trival.
>
> Even the ones that have some arginfo aren't all correct.
>
> For example:
>
>
> https://github.com/php/php-src/blob/master/Zend/zend_builtin_functions.c#L132
>
> which currently says:
>
> ZEND_BEGIN_ARG_INFO_EX(arginfo_define, 0, 0, 3)
>         ZEND_ARG_INFO(0, constant_name)
>         ZEND_ARG_INFO(0, value)
>         ZEND_ARG_INFO(0, case_insensitive)
> ZEND_END_ARG_INFO()
>
> That says there are 3 required params when it should clearly be 2.
>
> But really that block should be:
>
> ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_define, 0, 2, _IS_BOOL,
> 0, 0)
>     ZEND_ARG_TYPE_INFO(0, constant_name, IS_STRING, 0)
>     ZEND_ARG_INFO(0, value)
>     ZEND_ARG_TYPE_INFO(0, case_insensitive, _IS_BOOL, 0)
> ZEND_END_ARG_INFO()
>
> It seems way too lazy to not fix these before we release 7.0. Especially
> the ones that are outright wrong. So, having called us lazy, I better
> dig in and do something about it. I'll fix up all of the ones in
> zend_builtin_functions.c today. And move on to others after. Definitely
> a tedious task that we could use some extra hands on.
>

Before making more extensive use of arginfo types, I think we should first
do some adjustments to the way they are handled. In particular adding types
means that internal fcalls will take the branch in
http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#3669. As this is hot
code, it is likely important to avoid doing those duplicate type checks
(that will also happen in zpp anyway). As such I would suggest to make
arginfo for internal functions reflection-only. Return type hints are
already only enforced in debug mode.

Nikita

--047d7bf0bfc2ddc84e05190e583c--