Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:101370
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.68 as permitted sender)
MIME-Version: 1.0
Sender: morrison.levi@gmail.com
In-Reply-To: <CAESVnVqnYfARGKvHPTDu46E2UDKuUgJYqJdjWaNxRjckQ9svew@mail.gmail.com>
References: <CAESVnVobB2pjLiwR0D85q++Ymau69qO8bH2GcH+XF8B7cb5tWw@mail.gmail.com>
 <CAFMT4NrRu7vKy-uGoF_5tup_mZx8LBmJKc0N6UTN2TxUz8w_4g@mail.gmail.com>
 <CAESVnVp-eHAZU3DNVq-u8hJQEwgQXaPvwv+EnCfScRios-huaQ@mail.gmail.com> <CAESVnVqnYfARGKvHPTDu46E2UDKuUgJYqJdjWaNxRjckQ9svew@mail.gmail.com>
Date: Mon, 18 Dec 2017 15:47:40 -0700
Message-ID: <CAFMT4NoiRh88synA6P9eOJBMUx6xjA8W-aJfZmaeeJajyLw39g@mail.gmail.com>
To: Sara Golemon <pollita@php.net>
Cc: PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [PHP-DEV] C++ and FAST_ZPP macros
From: levim@php.net (Levi Morrison)

On Mon, Dec 18, 2017 at 2:31 PM, Sara Golemon <pollita@php.net> wrote:
> On Mon, Dec 18, 2017 at 4:11 PM, Sara Golemon <pollita@php.net> wrote:
>> On Mon, Dec 18, 2017 at 3:38 PM, Levi Morrison <levim@php.net> wrote:
>>>> Thoughts?  If I don't hear anything in a week, I'll just apply to 7.1
>>>> and merge up.
>>>>
>>> Is our macro `#define Z_EXPECTED_TYPE_STR(id, str) str,` ever used? If
>>> so there might be a change in perceived behavior because the first
>>> entry previously had "integer" and now it is "mixed".
>>>
>> It exists for the purpose of generating output message when the type
>> is not cast/coercible to the expected type.  The index of the string
>> entry corresponds 1:1 with the value of the enum, so it'll only show
>> "mixed" when the expect type was ANY and we failed to cast/coerce to
>> ANY (which will obviously never happen).
>>
>> In fact, the previous state where _expected_type was initialized to
>> IS_UNDEF (and by extension interpreted poorly as Z_EXPECTED_LONG)
>> would also never happen because the cast/coersion error is only
>> produced by P_PARAM_*() macros who have in turn explicitly reset
>> _expected_type to some specific value.
>>
>> The default initialization exists only to silence unhelpful compiler
>> warnings and not to provide any actual use or effect.
>>
> To clarify one last thing: Simply changing the IS_UNDEF on that
> initialization line to Z_EXPECTED_LONG would have also worked here
> because as stated above, it's never *actually* used without being
> reset to a meaningful case.  I went with a new enum value to make the
> intent more clear to someone reading this in the future.
>
> If it makes anyone more comfortable, I can make the 7.1/7.2 fix be
> that simple, and add the new enum value in master, or even just forgo
> the new enum value in favor of an inline comment explaining why the
> default value in Z_EXPECTED_LONG.
>
> -Sara

I am fine with the change; I just wanted to double-check that aspect.