Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:101369
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: softfail (pb1.pair.com: domain golemon.com does not designate 209.85.216.194 as permitted sender)
MIME-Version: 1.0
Sender: php@golemon.com
In-Reply-To: <CAESVnVp-eHAZU3DNVq-u8hJQEwgQXaPvwv+EnCfScRios-huaQ@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>
Date: Mon, 18 Dec 2017 16:31:24 -0500
Message-ID: <CAESVnVqnYfARGKvHPTDu46E2UDKuUgJYqJdjWaNxRjckQ9svew@mail.gmail.com>
To: Levi Morrison <levim@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: pollita@php.net (Sara Golemon)

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