[webkit-reviews] review granted: [Bug 134860] Make improvements to Type Profiling : [Attachment 235326] patch

[bugzilla-daemon at webkit.org]

Filip Pizlo <fpizlo at apple.com> has granted Saam Barati <sbarati at apple.com>'s
request for review:
Bug 134860: Make improvements to Type Profiling
https://bugs.webkit.org/show_bug.cgi?id=134860

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

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


It's good modulo two small changes.  Can you file that bug, fix that
indentation, and upload a new patch so that I can land it?

> Source/JavaScriptCore/runtime/HighFidelityTypeProfiler.cpp:40
> +    TypeProfilerSearchDescriptor descriptor = location->m_globalVariableID
== HighFidelityReturnStatement ? TypeProfilerSearchDescriptorFunctionReturn
> +						 : location->m_globalVariableID
== HighFidelityThisStatement ? TypeProfilerSearchDescriptorThisStatement
> +						 :
TypeProfilerSearchDescriptorNormal;

I think that you should fix the style here.  In WebKit, any line that is a
continuation of the statement on the previous line should be indented just an
extra four spaces.

> Source/JavaScriptCore/runtime/HighFidelityTypeProfiler.cpp:131
> +    // FIXME: BestMatch should never be null past this point. This doesn't
hold currently because we ignore some Eval/With/VarInjection variable
assignments.

Can has bugzilla bug for this FIXME?  I think that every FIXME should have a
bug.