Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:44817
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.220.224 as permitted sender)
DomainKey-Status: bad
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=mime-version:in-reply-to:references:from:date:message-id:subject:to
         :cc:content-type:content-transfer-encoding;
        b=lFORjx5RnU8Je4Y91Lomc24UmuRG4ah9lUHQX5U9fBrUHroASN/hBy4SyX7pW3SYSt
         pU9EMhn5GHbrEY5j6pTkepzSzuvWubxzjQs7Y/yg7eqisxBo+T9g1K3/yhzqOj6yyXym
         WaCxbkJVW3b/YyRcsF5EQCjDcu/X+1/Vh7cI4=
MIME-Version: 1.0
In-Reply-To: <C7F0588B-3BF7-49BA-8469-A0CDE0635955@prohost.org>
References: <2D0F5226-EBCA-4B45-BF01-8ED1C643976C@prohost.org> 
	<1247006344.3760.164.camel@goldfinger.johannes.nop> <af54caaf0907071607r68b9e790ofa717ad7f8fd70d0@mail.gmail.com> 
	<C7F0588B-3BF7-49BA-8469-A0CDE0635955@prohost.org>
Date: Wed, 8 Jul 2009 09:25:24 +0100
Message-ID: <af54caaf0907080125n4bb7742dwc2d86a03fd3b9a25@mail.gmail.com>
To: Ilia Alshanetsky <ilia@prohost.org>
Cc: =?UTF-8?Q?Johannes_Schl=C3=BCter?= <johannes@php.net>, 
	PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Type hinting/casting request for vote
From: paul.biggar@gmail.com (Paul Biggar)

2009/7/8 Ilia Alshanetsky <ilia@prohost.org>:
> All of the identified issues can be resolved and none of them represent a
> major challenge to address. However, if there is no consensus to put this=
 in

OK, but you had not said you would resolve them. I would appreciate
some detail on what you will do to address them.


> the near future (which at this point is 5.3), I have hard time justifying
> spending further time on this. The original patch that was posted, that d=
id
> break BC was far simpler and featureless, the changes since (which took a
> fair amount of work) were specifically made to address some of the main
> concerns that were on the list. I feel what is on the table right now is
> pretty close to what a final product could be, to have a vote on it. If

Personally, I had gotten the impression that this was the final
product, and that we could take it or leave it.


> decision is made to proceed within a practical release schedule, then
> suffice to say that I'd be more then happy to put further time to address
> the minor issues indicated.

I recommend:

 - stop the vote
 - address the issues (possibly deferring them until after voting, or whate=
ver)
 - write an RFC
 - wait for Lukas to finish what he's doing
 - new vote, more options (5.3.x/5.4/6.0, Lukas'/yours, make it clear
what we're voting for)


Thanks,
Paul



> On 7-Jul-09, at 7:07 PM, Paul Biggar wrote:
>
>> 2009/7/7 Johannes Schl=C3=BCter <johannes@php.net>:
>>>
>>> On Mon, 2009-07-06 at 20:52 -0400, Ilia Alshanetsky wrote:
>>>>
>>>> Last week or so there was a fairly detailed discussion on the
>>>> internals list regarding type hinting based on my original patch.
>>
>>
>>> Having an "old" 5.3 extension with a typehint expecting an array
>>> arg_info.array_type_hint will be set to 1.
>>> The newly compiled engine with this patch will then do
>>>
>>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* existing type alr=
eady matches the hint or forced type
>>> */
>>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (Z_TYPE_P(arg) =
=3D=3D cur_arg_info->array_type_hint ||
>>> Z_TYPE_P(arg) =3D=3D (cur_arg_info->array_type_hint ^ (1<<7))) {
>>>
>>> as it's main type check, but Z_TYPE_P(arg) will be IS_ARRAY (5) which
>>> doesn't match the 1 provided by the old extension, other checks in ther=
e
>>> will fail too, so the valid param will be rejected whereas an integer
>>> (IS_LONG 1) will be accepted where the extension expects an array.
>>
>> I raised this in my review, to which Ilia replied "It should be fine"
>> (http://news.php.net/php.internals/44707). I would not have thought it
>> would be fine.
>>
>> I had been thinking that Ilia would have to hack it to make 1 mean
>> array in this case, which would be ugly, but workable. Based on the
>> arguments in this thread, I believe it shouldn't go into 5.3 at all.
>> Are we allowed break the ABI for 5.4 (I would think so, but amn't
>> sure).
>>
>>
>>
>> Overall, I'm very disappointed with the way this has been conducted.
>> When reviews were posted they are not replied to (Stas posted
>> http://news.php.net/php.internals/44710, I posted
>> http://news.php.net/php.internals/44706, and I dont see any replies
>> except a cursory response to mine). Furthermore:
>> =C2=A0- the RFC process has been wilfully ignored (despite multiple requ=
ests)
>> =C2=A0- a vote was asked for when Lukas was still trying to discuss his
>> proposal
>> =C2=A0- the vote was take it or leave it
>> =C2=A0- there has been a general attitude of "throwing the toys out of t=
he
>> pram"
>>
>>
>> I am mostly for the patch, and I 100% support the idea. However, I
>> feel I have to vote against it, and urge others to do the same, until
>> the entire mess is rectified.
>>
>> Ilia, I respect the work you have put into this, but I would ask you
>> to withdraw the patch and the vote until these things have been sorted
>> out.
>>
>>
>> -1
>>
>> Thanks,
>> Paul
>>
>>
>>
>> --
>> Paul Biggar
>> paul.biggar@gmail.com
>
>



--=20
Paul Biggar
paul.biggar@gmail.com