Re: Undefined variables undefined order

Hi Sara, https://gist.github.com/dstogov/6a90601872b538d2ddd6 I see no problems committing this (running tests now). Thanks. Dmitry. On Wed, Jun 10, 2015 at 9:02 PM, Dmitry Stogov <dmitry@zend.com> wrote: > > > On Wed, Jun 10, 2015 at 6:00 PM, Sara Golemon <pollita@php.net> wrote: > >> On Wed, Jun 10, 2015 at 7:50 AM, Sara Golemon <pollita@php.net> wrote: >> > On Wed, Jun 10, 2015 at 7:42 AM, Sara Golemon <pollita@php.net> wrote: >> >> Dmitry, what's the reasoning behind this diff in the first place? >> >> Doesn't the compiler fold (<const-string> . <const-string>) >> >> already >> >> anyhow? How would we wind up with CONCAT_CONST_CONST at runtime? >> >> >> > Derp. Looked again, and it's (const *OR* string) && (const *OR* >> > string). I read those as ANDs initially. >> > >> > But now I'm confused again, because that makes it look like we can >> > reach this with an expression like "1 . 2" 1 and 2 are consts, but not >> > strings, so then we get to: >> > >> > zend_string *op1_str = Z_STR_P(op1); >> > >> > Which will lead to op1_str pointing at 0x00000001 and a segfault. >> > >> > What am I missing here? >> > >> Okay, answered my own question (I need to be gentler with the [Send] >> button). >> >> Zend/zend_compile.c sets up an assumption: >> >> if (left_node.op_type == IS_CONST) { >> convert_to_string(&left_node.u.constant); >> } >> if (right_node.op_type == IS_CONST) { >> convert_to_string(&right_node.u.constant); >> } >> >> So any const nodes passed to CONCAT will always be strings (having >> been preconverted), and the if check using an OR make sense, because >> if it's CONST, then we KNOW it's a string, and there's no point >> testing for it. We won't ever reach CONCAT_CONST_CONST (since that >> would be folded in the compiler), but we might reach CONCAT_CONST_CV >> or CONCAT_TMPVAR_CONST or some other combination thereof for which >> this case is set to handle. >> > > exectly. > anyway CONCAT_CONST_CONST may be removed at all, at least to reduce the > code size. > > TL;DR - Never mind me. I just didn't think it all the way through. >> >> -Sara >> >> P.S. - An assert(Z_TYPE_P(op1) == IS_STRING); and assert(Z_TYPE_P(op2) >> == IS_STRING); does seem reasonable though... Maybe even with a >> comment referencing that we can make that assumption because the >> compiler fixes up the types. >> > > ZEND_ASSERT() would be better. > > Thanks. Dmitry. > >