[webkit-reviews] review denied: [Bug 194706] Cache CompactVariableMap::Handle instead of VariableEnvironment for UnlinkedFunctionExecutable : [Attachment 362498] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has denied Tadeu Zagallo <tzagallo at apple.com>'s
request for review:
Bug 194706: Cache CompactVariableMap::Handle instead of VariableEnvironment for
UnlinkedFunctionExecutable
https://bugs.webkit.org/show_bug.cgi?id=194706

Attachment 362498: Patch

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




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

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

> Source/JavaScriptCore/runtime/CachedTypes.cpp:947
> +	   return decoder.vm().m_compactVariableMap->get(environment,
isNewEntry);
> +	   if (isNewAllocation && !isNewEntry) {
> +	       decoder.addFinalizer([=] {
> +		   delete environment;
> +	       });
> +	   }

This can't be right ;-)

I vote for adding some tests at this point to ensure you get the behavior you
want.