[webkit-reviews] review granted: [Bug 183256] We need to clear cached structures when having a bad time : [Attachment 334848] patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Saam Barati <sbarati at apple.com>'s
request for review:
Bug 183256: We need to clear cached structures when having a bad time
https://bugs.webkit.org/show_bug.cgi?id=183256

Attachment 334848: patch

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




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

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

r=me

> Source/JavaScriptCore/runtime/JSGlobalObject.cpp:1200
> +    auto isInEffectedGlobalObject = [&] (JSObject* object) {
> +	   for (JSObject* current = object; ;) {
> +	       if (current->globalObject() == m_globalObject)
> +		   return true;
> +	       
> +	       JSValue prototypeValue = current->getPrototypeDirect(vm);
> +	       if (prototypeValue.isNull())
> +		   return false;
> +	       current = asObject(prototypeValue);
> +	   }
> +    };

I think you need a "return false" after the for loop in order to be equivalent
to the original code.  But in this case, I think we should never get there. 
So, let's have a RELEASE_ASSERT_NOT_REACHED() there.

> Source/JavaScriptCore/runtime/JSGlobalObject.cpp:1211
> +		       bool isRelevantGlobalObject = structure->globalObject()
== m_globalObject

I suggest putting parens around the == expr.