[webkit-reviews] review denied: [Bug 20296] OpcodeStats doesn't build on platforms which don't have mergesort() : [Attachment 22933] This is my new proposed patch.

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Csaba Osztrogonác
<oszi at inf.u-szeged.hu>'s request for review:
Bug 20296: OpcodeStats doesn't build on platforms which don't have mergesort()
https://bugs.webkit.org/show_bug.cgi?id=20296

Attachment 22933: This is my new proposed patch.
https://bugs.webkit.org/attachment.cgi?id=22933&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
This should not be defined in Platform.h -- that goes beyond the scope of what
Platform.h should contain; just configuration macros. It belongs in a header
like MathExtras.h -- perhaps StdLibExtras.h.

This should be an inline function, not a macro. If it was going to be a macro
it should be a macro with arguments, not an unqualified macro.

The name platformSort doesn't make sense to me. If we need mergesort
specifically, then it needs to be mergesort. If it can be either qsort or
mergesort, then we can come up with a name that describes the reason we want
mergesort if we have it -- what is that reason? Why is mergesort better than
qsort for these purposes? I don't think "platform" enters into it, despite
Eric's earlier suggestion.

For example, in JSArray.cpp it's pretty clear what we want is a stable sort. In
those cases, using qsort is actually incorrect, so I don't think we should try
to make it more elegant -- the lack of a stable sort there is a problem needs
to be fixed.

Is stable sorting the issue in Opcode.cpp too? If we don't have any special
requirements, then it we can just change Opcode.cpp to use qsort instead of
doing this whole thing.