Bug 190163 - Create a RELEASE_AND_RETURN macro for ExceptionScopes
Summary: Create a RELEASE_AND_RETURN macro for ExceptionScopes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Miller
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-10-01 14:32 PDT by Keith Miller
Modified: 2018-10-01 16:24 PDT (History)
6 users (show)

See Also:

Attachments
Patch (189.26 KB, patch)
2018-10-01 14:34 PDT, Keith Miller 		no flags Details | Formatted Diff | Diff
Patch for landing (197.93 KB, patch)
2018-10-01 15:44 PDT, Keith Miller 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>