RESOLVED FIXED 94986
Finally inlining should correctly track the catch context
https://bugs.webkit.org/show_bug.cgi?id=94986
Summary Finally inlining should correctly track the catch context
Filip Pizlo
Reported 2012-08-24 17:20:47 PDT
Patch forthcoming. This will be a WIP for a few iterations.
Attachments
work in progress (11.65 KB, patch)
2012-08-24 17:22 PDT, Filip Pizlo
no flags
the patch (15.95 KB, patch)
2012-08-26 15:56 PDT, Filip Pizlo
sam: review+
webkit.review.bot: commit-queue-
Archive of layout-test-results from gce-cr-linux-05 (558.51 KB, application/zip)
2012-08-26 16:33 PDT, WebKit Review Bot
no flags
Filip Pizlo
Comment 1 2012-08-24 17:22:19 PDT
Created attachment 160528 [details] work in progress
Filip Pizlo
Comment 2 2012-08-26 15:56:21 PDT
Created attachment 160617 [details] the patch Still running tests to make sure I didn't break the world.
WebKit Review Bot
Comment 3 2012-08-26 15:59:26 PDT
Attachment 160617 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:515: The parameter name "tryData" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Sam Weinig
Comment 4 2012-08-26 16:01:58 PDT
Comment on attachment 160617 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=160617&action=review > Source/JavaScriptCore/ChangeLog:3 > + Finally inlining should correctly track the catch context Missing comma after Finally.
Filip Pizlo
Comment 5 2012-08-26 16:03:01 PDT
(In reply to comment #4) > (From update of attachment 160617 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=160617&action=review > > > Source/JavaScriptCore/ChangeLog:3 > > + Finally inlining should correctly track the catch context > > Missing comma after Finally. Finally! Finally inlining should correctly track the catch context.
WebKit Review Bot
Comment 6 2012-08-26 16:33:10 PDT
Comment on attachment 160617 [details] the patch Attachment 160617 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13599879 New failing tests: fast/js/throw-from-finally.html
WebKit Review Bot
Comment 7 2012-08-26 16:33:14 PDT
Created attachment 160618 [details] Archive of layout-test-results from gce-cr-linux-05 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-05 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Filip Pizlo
Comment 8 2012-08-26 17:37:39 PDT
(In reply to comment #7) > Created an attachment (id=160618) [details] > Archive of layout-test-results from gce-cr-linux-05 > > The attached test failures were seen while running run-webkit-tests on the chromium-ews. > Bot: gce-cr-linux-05 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid Looks like I need to tweak the test to ensure that shouldBe() knows that what I'm interested in is comparing the stringified form of the expression. I'll land after I fix that. Other than that, it appears that this change passes all tests.
Filip Pizlo
Comment 9 2012-08-26 17:49:28 PDT
Note You need to log in before you can comment on or make changes to this bug.