On Mon, Mar 14, 2016 at 12:58 PM, Dmitry Stogov <dmi...@zend.com> wrote:
> > > On 03/14/2016 02:29 PM, Nikita Popov wrote: > > On Mon, Mar 14, 2016 at 11:56 AM, Dmitry Stogov <dmi...@zend.com> wrote: > >> Please take a quick look and let me know if you see any problems. >> >> >> https://gist.github.com/dstogov/23cc318dd3e411904e10 >> >> >> I'm going to commit this tomorrow. >> >> >> Thanks. Dmitry. >> > ZEND_ADD is not commutative for array operands. > > > Unfortunately, you are right :( > > ZEND_MUL may not be commutative if operator overloading is involved (e.g. > matrix multiplication is not abelian). > > > Right again. > > The NO_CONST_CONST cases are problematic: Opcache currently, in my eyes > incorrectly, evaluates constant expressions even if they throw notices > (currently I think mostly by suppressing them). If we do not evaluate > these, we may end up with CONST_CONST operations. (This is not a huge > problem just now because constant folding in opcache is limited -- however > with full constant propagation this leads to many test failures.) > > > The patch adds NO_CONST_CONST handling to opcache. All tests are passed, I > also didn't see any problems running ~10 real-life apps. > > The opcodes affected by the last point (limited to those included in your > patch) are SUB, MUL, POW, CONCAT (array operands) and BW_NOT (various). > However with Andrea's invalid numeric string RFC more will fall in this > category (evaluable but throwing). > > > I didn't get what is wrong with SUB, MUL, etc, because they are evaluated > at compile-time > Anyway, I see - the patch can't be committed in current state. > Thanks for review. > What I mean is, for example: var_dump(~(bool)true); echo "Done\n"; Will throw an exception without opcache, will result in no output (and no *visible* error) with opcache. My point here is that const-operand-only cases might be necessary to handle error cases like this. Nikita
