[webkit-reviews] review denied: [Bug 181310] Don't unconditionally lower op_inc to ArithAdd in the BytecodeParser : [Attachment 330499] patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has denied  review:
Bug 181310: Don't unconditionally lower op_inc to ArithAdd in the
BytecodeParser
https://bugs.webkit.org/show_bug.cgi?id=181310

Attachment 330499: patch

https://bugs.webkit.org/attachment.cgi?id=330499&action=review




--- Comment #5 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 330499
  --> https://bugs.webkit.org/attachment.cgi?id=330499
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=330499&action=review

>>> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:4574
>>> +		 if (op->hasNumberResult())
>>> +		     set(srcDstVirtualRegister, makeSafe(addToGraph(ArithAdd,
op, addToGraph(JSConstant, OpInfo(m_constantOne)))));
>>> +		 else
>>> +		     set(srcDstVirtualRegister, makeSafe(addToGraph(ValueAdd,
op, addToGraph(JSConstant, OpInfo(m_constantOne)))));
>> 
>> Why not unconditionally lower to ValueAdd and let prediction/fixup figure it
out?
> 
> That's what I originally thought as well. I decided to just follow what
op_add does though. I think we should either change both or keep this as is.

Oh wow, I think my code is horribly wrong. This will make:

let x = "foo";
x++;

have x === "foo1" at the end!

Obviously I need something else here.