On Mon, Dec 18, 2017 at 2:31 PM, Sara Golemon <poll...@php.net> wrote: > On Mon, Dec 18, 2017 at 4:11 PM, Sara Golemon <poll...@php.net> wrote: >> On Mon, Dec 18, 2017 at 3:38 PM, Levi Morrison <le...@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. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
