zend_long/zend_ulong without renaming everything else would be a perfect solution from my point of view.
Thanks. Dmitry. On Fri, Aug 22, 2014 at 12:49 AM, Nikita Popov <nikita....@gmail.com> wrote: > On Thu, Aug 21, 2014 at 7:23 PM, Dmitry Stogov <dmi...@zend.com> wrote: > >> Hi, >> >> Thanks to Anatol and Pierre the 64-bit patch is ready >> https://github.com/weltling/php-src >> >> I made quick code review and don't see any technical problems now. >> >> The performance and memory consumption difference is negligible. see >> >> https://docs.google.com/spreadsheets/d/1PD4oiiXz6B0JbeZYnUSat5fHoq3_jAiCeI2cGHJ3UtQ/edit#gid=0 >> >> The patch breaks one test on 32-bit Linux: >> ext/date/tests/bug53437_var3.phpt (seems to be a bogus test and wrong >> behavior in php5.6 and below) and one test on 64-bit Linux: >> ext/standard/tests/array/array_pad_variation2.phpt (has to be analyzed) >> >> The only thing that I don't like is a massive renaming described here >> https://wiki.php.net/rfc/size_t_and_int64_next#semantical_macro_renamings >> >> IS_LONG -> IS_INT >> Z_LVAL -> L_IVAL >> etc >> >> On one hand using INT may be more consistent, on the other hand it's going >> to break habits and make addition headache for merging from php-5 (I know, >> phpng already made problems) >> >> I'm not sure how to proceed. If I'm alone, lets go ahead with new names. >> If >> some others prefer old names we will probably need voting. >> >> Despite of renaming, I would like to see this patch in master ASAP. >> >> Thanks. Dmitry. >> > > I am against merging this with the long->int rename everywhere. This seems > like change for the sake of change. > > I am also concerned that we now have zend_uint_t (a 64-bit integer type) > and zend_uint (a 32-bit integer type). Notice the difference? Yes, it's the > missing _t. > > I would appreciate it if we could consider the following naming convention: > > * zend_(u)int - 32 bit integer type > * zend_(u)long - 64 bit integer type (on 64 bit systems) > > This retains the original meaning of the type, with the tweak that > zend_(u)long will be 64bit on LLP64 systems as well. This avoids the > confusion of having two types that only differ by a _t suffix and have > totally different meanings. It also removes any need to rename everything > from LONG to INT. > > Thanks, > Nikita >
