Bug 195428

Summary: Follow up refactoring in try-finally code after r242591.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, msaboff, saam, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch. saam: review+

Description Mark Lam 2019-03-07 13:26:49 PST
Patch coming.
Comment 1 Mark Lam 2019-03-07 13:43:22 PST
Created attachment 363920 [details]
proposed patch.
Comment 2 EWS Watchlist 2019-03-07 13:46:04 PST
Attachment 363920 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:4783:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:4784:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:4785:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:4815:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:4816:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:4848:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:4849:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:4850:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:4886:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Total errors found: 9 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Saam Barati 2019-03-07 13:46:29 PST
Comment on attachment 363920 [details]
proposed patch.

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

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:4790
> +            // 10: try {
> +            // 11:     for (... stuff ...) {
> +            // 12:         try {
> +            // 13:             continue; // Sets completionType to jumpID of top of the for loop.
> +            // 14:         } finally {
> +            // 15:         } // Jump to top of the for loop on completion.
> +            // 16:     }
> +            // 15: } finally {
> +            // 16: }

remove line numbers

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:4820
> +                // 10: try {
> +                // 11:     try {
> +                // 12:         return result; // Sets completionType to Return, and completionValue to result.
> +                // 13:     } finally {
> +                // 14:     } // Jump to outer finally on completion.
> +                // 15: } finally {
> +                // 16: }

remove line numbers

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:4888
> +                // 10: try {
> +                // 11:     return result; // Sets completionType to Return, and completionValue to result.
> +                // 12: } finally {
> +                // 13: } // Executes the return of the completionValue.

remove line numbers.
Comment 4 Saam Barati 2019-03-07 14:12:04 PST
Comment on attachment 363920 [details]
proposed patch.

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

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:4795
> +            // So, we'll set the completionType to Normal (oon behalf of the target) before we jump.

oon => on
Comment 5 Mark Lam 2019-03-07 15:02:15 PST
Thanks for the review.  I'm applied the fixes locally.  Landed in r242614: <http://trac.webkit.org/r242614>.
Comment 6 Radar WebKit Bug Importer 2019-03-07 15:10:35 PST
<rdar://problem/48692113>