Patch coming.
Created attachment 295324 [details] proposed patch.
Comment on attachment 295324 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=295324&action=review > Source/JavaScriptCore/runtime/JSGenericTypedArrayViewPrototypeFunctions.h:470 > + scope.release(); Why not put the scope.release() before the switch and also keep the old return statement?
Comment on attachment 295324 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=295324&action=review >> Source/JavaScriptCore/runtime/JSGenericTypedArrayViewPrototypeFunctions.h:470 >> + scope.release(); > > Why not put the scope.release() before the switch and also keep the old return statement? Because we want the scope.release() near the return statement. Having it before the switch increases the opportunity for new code to be inadvertantly added between the scope.release() and the switch.
(In reply to comment #3) > Comment on attachment 295324 [details] > proposed patch. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=295324&action=review > > >> Source/JavaScriptCore/runtime/JSGenericTypedArrayViewPrototypeFunctions.h:470 > >> + scope.release(); > > > > Why not put the scope.release() before the switch and also keep the old return statement? > > Because we want the scope.release() near the return statement. Having it > before the switch increases the opportunity for new code to be inadvertantly > added between the scope.release() and the switch. Saam pointed out to me offline that if we have 2 ThrowScopes between the scope.release() and the return, the 2nd ThrowScope will catch the unhandled exception from the first. So, my reasoning above does not apply. Anyway, I'll leave it as is for this patch because it does make it easier to see the return that corresponds to the scope.release().
Thanks for the review. Landed in r209031: <http://trac.webkit.org/r209031>.