On Aug 25, 2015 7:19 PM, "Matt Wilmas" <php_li...@realplain.com> wrote:
> > I didn't reply in last week's thread about the overflow checks in OpenSSL... > > But it is *definitely* fine to be optimistic and rely on compiler to do basic, basic stuff like this. No reason to make things more complicated just to think one is helping the compiler. > > Proof? Have you seen the whole VM in the last, like, 10 years? Using your logic, we have major, major, MAJOR problems literally everywhere there! > > Now can anyone point to problems compiling the default, specialized VM? I rest my case. :-) There's conditional stuff ALL over the place (not macros), and propagated to inline functions, etc. where the compiler is "counted on" to remove the constant conditions and all that stuff. > > The current FAST_ZPP parameter parsing, any unnecessary parts are optimized out (it'd be bad if that wasn't the case). My new proposal (coming soon...) has a lot of more complicated-looking source code, but almost all deleted by the compiler. > Transforming zpp's type spec string at compile time, which MSVC can't do because it does indeed suck for some reason, involves *dozens* of KB of source, many inline functions, etc. but nearly 100% of it is deleted, correctly, by GCC and Clang. > > > Anyway, that's why I say if "some" compiler can't do basic optimizations, it *sucks*, and has much bigger problems in the rest of the source, so there's no reason to worry about it... At all. Let me disagree on this comment. Most of it and its wording. >> Also it is probably a question of taste. SIZEOF_INT == SIZEOF_ZEND_LONG is >> something with zero calculation and guaranteed to exclude any possible side >> effect, while sizeof(int) == sizeof(zend_long) depends on the optimization >> choosen. Not disputing that your suggestion will do nearly the same job in >> the usual case, ofc. >> >> With the casts - probably yes, many cases will go by the standard type >> promotion rules, but having the casts only ensures it does what one actually >> intended to do and has no effect in the generated code. For example one >> would need a cast when comparing with INT_MAX + 1. Worse is with unsigned, >> because the result depends on the platform and concrete types and values. >> >> Hiding behind the macros makes full sense for two reasons at least >> >> - there are people with various skills that can just rely on what the API >> provides without going too deep into the detail >> - doing an internal change won't affect anything in the existing usages >> >>> > IMHO these and maybe more are missing after the 64-bit RFC. Do you >>> > think they would make sense? Or would make sense now, or later in >> >> master? >>> >>> >>> Also, why, exactly, has the zend_string length been defined as size_t >> >> (just >>> >>> because it's "supposed to" be?), which is incompatible with most >> >> everything in >>> >>> PHP, instead of zend_long (since we had int before)? >>> >>> e.g. Just look at strlen() -- oops, size_t no worky! :-/ Dmitry? We need >> >> range >>> >>> checking there, right? Conversion to double? No? Then there's *no >>> point* in using size_t, and would let the compiler auto-optimize the >> >> above- >>> >>> referenced comparisons with size_t... >>> >> Not sure what you mean here, 32-bit? > > > I mean, PHP's strlen() on any platform, why is it possible to have larger strings (size_t length) than can be returned in zend_long. That's a bug. It is wrong to use long for portability. It is also wrong (very) to use int* for length of a buffer. It is a very well known good practice (correct way) to use size_t, even more when interacting with external sources. I have to disagree about your statement here too. > So? Why does it matter if libraries are using size_t? The same libraries always have been and passed an int (from string length) before PHP 7. Pass them zend_long instead now... And we have issues because of this exact case. It also helps to use the right type for a given usage. In this case size_t. > > >> Before the 64-bit RFC my idea was actually to port the dependency libs for >> the full 64-bit support, but it's insane. So better to fix the side effects, >> as those are almost the same as in PHP5 on most of 64-bit. >> >> Regards >> >> Anatol > > > - Matt
