[webkit-reviews] review granted: [Bug 204998] [WebAssembly] Fix LLIntGenerator's checkConsistency contract : [Attachment 385124] Patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Tadeu Zagallo <tzagallo at apple.com>'s
request for review:
Bug 204998: [WebAssembly] Fix LLIntGenerator's checkConsistency contract
https://bugs.webkit.org/show_bug.cgi?id=204998

Attachment 385124: Patch

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




--- Comment #3 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 385124
  --> https://bugs.webkit.org/attachment.cgi?id=385124
Patch

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

> Source/JavaScriptCore/ChangeLog:18
> +	   In this patch I also removed the consistency check from calls to
push in addArguments and addLocal, since at
> +	   that point the expression stack must be empty. If that wasn't the
case, those calls would be incorrect for the
> +	   same reason as above: we perform several pushes in a row, which
means that the stacks would be out of sync.

At the top of FunctionParser<Context>::parse(), I suggest
ASSERT(m_expressionStack.isEmpty()).  This ensures and documents that a
consistency check is not needed there.	Alternatively, you can just call
checkConsistency() there anyway, which makes it even clearer.  It's effectively
a no-op, but will clearly document that we know the expression stack is
consistent at that point.