On Thu, Aug 13, 2015 at 1:12 AM, Matt Wilmas <php_li...@realplain.com> wrote:
> H Dmitry, > > ----- Original Message ----- > From: "Dmitry Stogov" > Sent: Tuesday, August 11, 2015 > > On Tue, Aug 11, 2015 at 11:10 PM, Matt Wilmas <php_li...@realplain.com> >> wrote: >> >> [...] >>> >>> Look at e.g. is_numeric() or strpos() (needle). Plain zval param >>> parsing, >>> so NO ZVAL_DEREF() occurs (FAST_ZPP or traditional). These 2 example >>> functions don't handle IS_REFERENCE type, so they would break. >>> >>> Or is there no way for them (or any function?) to get a IS_REFERENCE? >>> Then *why* is there ZVAL_DEREF() in param parsing functions? We could >>> remove it! >>> >>> >> We probably may remove ZVAL_DEREF() for functions arguments passed by >> value, but we don't know if argument was passed by value or by reference >> in >> ZPP functions. Actually, in FAST_ZPP for scalars we may probably assume >> passing by value and remove ZVAL_DEREF(), but I'm not sure if this is 100% >> safe. >> > > Yes, speaking of scalars, I had already noticed that it doesn't make sense > to have a "separate" argument for the FAST_ZPP scalar _EX macros, as > there's nothing to separate (same with traditional '/') on simple values. > I was changing those macros, and don't think they're used anyway. > > Also, I think those simple/scalar types wouldn't be used with by-reference > arguments (?), as there would be nothing to update... > > Like I said in reply to Nikita, it seems like separating is needed for > by-reference args, so only need DEREF if *also* separating? It looks like > that was the idea with Z_PARAM_ZVAL_EX(). And for other types, separating > only applies to arrays or strings, I believe. > > As far as not 100% safe, you mean IS_REFERENCE type causing an error about > wrong type? Shouldn't that only happen if developer indicated by-ref arg, > but then used wrong type in parsing? > At least this case. We may change ZVAL_DEREF(...) into ZEND_ASSERT(!Z_ISREF_P(...)), to prevent silence crashes. Thanks. Dmitry. > > On the other hand, it seems like always using ZVAL_DEREF() won't hurt > anything? e.g. part of my confusion was that I thought IS_REFERENCE was > wanting to be *kept* in some cases (after parsing), but it seems to not be > the case. :-) > > Speaking of confusion, it also seems the DEREF stuff is unnecessary in > type.c:php_is_type(), for example. > > Thanks. Dmitry. >> > > Thanks, > Matt >