...
Created attachment 362468 [details] Patch
Comment on attachment 362468 [details] Patch r=me
(In reply to Mark Lam from comment #2) > Comment on attachment 362468 [details] > Patch > > r=me Thanks!
Comment on attachment 362468 [details] Patch Clearing flags on attachment: 362468 Committed r241837: <https://trac.webkit.org/changeset/241837>
All reviewed patches have been landed. Closing bug.
<rdar://problem/48251809>
Why not put the crash on failure at the call sites? Seems strange to pass a boolean just to do a check that the caller could have done.
(In reply to Darin Adler from comment #7) > Why not put the crash on failure at the call sites? Seems strange to pass a > boolean just to do a check that the caller could have done. The patch adds missing tryAllocate feature into DebugHeap, and this style is aligned to the existing DebugHeap::memalign, bmalloc allocators, and JSC GC Heap Allocators (JavaScriptCore/heap/AllocationFailureMode.h). I think our (current) typical style is, creating low-level allocator with this style, and constructing tryAllocate / allocate APIs on these low-level allocators, and expose them to clients (e.g. JSArray::tryCreate). I'm not sure the original intent of this API design. But one pros is that we can gather CRASH into one place instead of putting it at the caller sites, while we still keep good "try" or "not-try" information in stacktrace's function name (`tryAllocate` or `allocate`). But I don't have strong opinion here as long as the style is consistent :)