Bug 190163

Summary: Create a RELEASE_AND_RETURN macro for ExceptionScopes
Product: WebKit Reporter: Keith Miller <keith_miller>
Component: New BugsAssignee: Keith Miller <keith_miller>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ews-watchlist, mark.lam, msaboff, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

Description Keith Miller 2018-10-01 14:32:19 PDT
Create a RELEASE_AND_RETURN macro for ExceptionScopes
Comment 1 Keith Miller 2018-10-01 14:34:34 PDT
Created attachment 351300 [details]
Patch
Comment 2 Mark Lam 2018-10-01 14:44:19 PDT
Comment on attachment 351300 [details]
Patch

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

r=me with unnecessary braces removed.

> Source/JavaScriptCore/dfg/DFGOperations.cpp:266
>      if (prototype.isObject()) {
> -        scope.release();
> -        return constructEmptyObject(exec, asObject(prototype));
> +        RELEASE_AND_RETURN(scope, constructEmptyObject(exec, asObject(prototype)));
>      }

Don't need braces anymore.  Ditto many places below.

> Source/JavaScriptCore/runtime/ExceptionScope.h:96
> +#define RELEASE_AND_RETURN(scope__, expression__) do { \

Pity that this name is so much more generic that RETURN_IF_EXCEPTION, but no one else has this idiom yet.  So, I'm ok going with this until there's a need to rename it.
Comment 3 Keith Miller 2018-10-01 15:10:38 PDT
Comment on attachment 351300 [details]
Patch

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

>> Source/JavaScriptCore/dfg/DFGOperations.cpp:266
>>      }
> 
> Don't need braces anymore.  Ditto many places below.

Weird that the linter didn't catch that :(

>> Source/JavaScriptCore/runtime/ExceptionScope.h:96
>> +#define RELEASE_AND_RETURN(scope__, expression__) do { \
> 
> Pity that this name is so much more generic that RETURN_IF_EXCEPTION, but no one else has this idiom yet.  So, I'm ok going with this until there's a need to rename it.

How would you feel about RELEASE_SCOPE_AND_RETURN?
Comment 4 Mark Lam 2018-10-01 15:32:55 PDT
(In reply to Keith Miller from comment #3)
> How would you feel about RELEASE_SCOPE_AND_RETURN?

RELEASE_SCOPE_AND_RETURN is slightly less generic but also more verbose.  I think we should just leave it as RELEASE_AND_RETURN for now.  Don't fix it if it ain't broke.
Comment 5 Keith Miller 2018-10-01 15:44:12 PDT
Created attachment 351317 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2018-10-01 16:23:16 PDT
Comment on attachment 351317 [details]
Patch for landing

Clearing flags on attachment: 351317

Committed r236697: <https://trac.webkit.org/changeset/236697>
Comment 7 WebKit Commit Bot 2018-10-01 16:23:18 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2018-10-01 16:24:22 PDT
<rdar://problem/44925105>