[webkit-reviews] review granted: [Bug 129519] Support caching of custom setters : [Attachment 225916] Patch

[bugzilla-daemon at webkit.org]

Filip Pizlo <fpizlo at apple.com> has granted Oliver Hunt <oliver at apple.com>'s
request for review:
Bug 129519: Support caching of custom setters
https://bugs.webkit.org/show_bug.cgi?id=129519

Attachment 225916: Patch
https://bugs.webkit.org/attachment.cgi?id=225916&action=review

------- Additional Comments from Filip Pizlo <fpizlo at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=225916&action=review


Fix indentation.  Or, rather, bring it back to the way it was.	Note that the
style bot is totally busted with respect to constructor indentation.

> Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp:137
> -    : JITByIdGenerator(codeBlock, codeOrigin, usedRegisters, base, value,
registersFlushed)
> -    , m_scratch(scratch)
> -    , m_ecmaMode(ecmaMode)
> -    , m_putKind(putKind)
> +	   : JITByIdGenerator(codeBlock, codeOrigin, usedRegisters, base,
value, spillMode)
> +	   , m_scratch(scratch)
> +	   , m_ecmaMode(ecmaMode)
> +	   , m_putKind(putKind)

The original indentation was correct.

> Source/JavaScriptCore/jit/JITInlineCacheGenerator.h:100
> +    : JITByIdGenerator(codeBlock, codeOrigin, usedRegisters, base, value,
spillMode)

The original indentation was correct.

> Source/JavaScriptCore/jit/Repatch.cpp:1150
> +    GPRReg scratchGPR1 = InvalidGPRReg;
> +    if (prototypeChain) {
> +	   scratchGPR1 = tempRegisters.getFreeGPR();
> +	   RELEASE_ASSERT(scratchGPR1 != InvalidGPRReg);
> +    }

Why not just always call tempRegisters.getFreeGPR(), and always assert that you
got one?  No point in predicating this on prototypeChain.