Re: Use "ropes" for string concatenation

On Tue, Mar 24, 2015 at 1:08 PM, Dmitry Stogov <dmitry@zend.com> wrote: > > > On Tue, Mar 24, 2015 at 2:51 PM, Nikita Popov <nikita.ppv@gmail.com> > wrote: > >> On Tue, Mar 24, 2015 at 10:28 AM, Dmitry Stogov <dmitry@zend.com> wrote: >> >>> I thought about something like this :) >>> In my opinion UString is really not a proper way to implement Unicoide, >>> but I agree not break anything in current stage. >>> Anyway, please review the first PR (in my opinion it is safe to commit), >>> but you may find some other issues. >>> >>> Thanks. Dmitry. >>> >> >> First PR looks okay to me. One question: Why the separate INIT and ADD >> opcodes? They seem pretty much the same, just one using a hardcoded 0 >> instead of ex_val. >> > > Oh. It's historical, at first we allocated rope on heap. I'll check if > INIT_ROPE is still necessary. > Most probably we still need it, to know the size of rope vector. > > >> Regarding exception-safety - is the problem that doing an EG(exception) >> check and releasing the rope is too expensive? >> > > No. It's a common problem. Because exception may be thrown in some other > opcode and we don't clean IS_VAR/IS_TMP_VAR zvals. > For example the following script leaks independently from the patch > > <?php > function foo() { > throw new Exception(); > } > try { > $a = "a"; > $a = $a . $a . $a . foo() . $a . "\n"; > } catch (Exception $e) { > } > ?> > Ah yes, I forgot about this issue. However, shouldn't we at least properly release memory if the exception occurs in the opcode? Nikita