Re: Change in type-hint representation

On Wed, Jan 11, 2017 at 9:37 AM, Dmitry Stogov <dmitry@zend.com> wrote: > The patch was updated according to feedback: added comments, better names and encapsulation, > less magic, better code reuse, keep a free bit in zend_type for future extension. > <z.ˆçC°ÇÚ > Ò!š5 https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1> > > > > <https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1> > > > https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1 > > > Thanks. Dmitry. > > > > ________________________________ > From: Derick Rethans <derick@php.net> > Sent: Wednesday, January 11, 2017 6:43:50 PM > To: Dmitry Stogov > Cc: PHP internals list; Bob Weinand; Joe Watkins; Zeev Suraski; Anatol Belski (ab@php.net); > Nikita Popov; Xinchen Hui > Subject: Re: [PHP-DEV] Change in type-hint representation > > On Wed, 11 Jan 2017, Dmitry Stogov wrote: > >> Hi, >> >> >> I propose to introduce a unified type representation (zend_type). >> >> Now it's going to be used for typing of arguments and return values. >> >> Later we should use it for properties and other things. >> >> >> >> https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1 >> >> >> The main changes are in zend_types.h and zend_compile.h, the rest is just an adoption for >> new type representation. >> >> I don't think we need RFC, because this is just an internal change that doesn't >> change behavior. >> >> >> I got the idea working on typed properties together with Bob and Joe. >> >> Áø&•½õp‡¡~ì‹Ã >> https://github.com/php/php-src/compare/master...bwoebi:typed_ref_properties >> >> I think it would be better to introduce zend_type and then continue work on typed >> properties. >> >> >> Any comments? > > 208 + if (new_arg_info[i].type > 0x3ff) { > > I wouldn't use a magical constant there, but do a define of what 0x3ff > actually is. > > 209 + /* this is a calss name */ > > That's spelled "class" (not "calss"). > > cheers, > Derick As part of this effort can we refactor the IS_LONG, IS_ARRAY, IS_OBJECT, etc macros to use an enum? Maybe zend_type_code if you like the code name for it? Also we already use "kind" in the AST; should it be ZEND_TYPE_KIND and zend_type_kind instead? Overall I think this cleanup is needed. Thanks, Dmitry.