Bug 18735 - SQUIRRELFISH: closures are sometimes given an incorrect 'this' value when called
Summary: SQUIRRELFISH: closures are sometimes given an incorrect 'this' value when called
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
Depends on:
Blocks: 18624
  Show dependency treegraph
Reported: 2008-04-24 23:22 PDT by Cameron Zwarich (cpst)
Modified: 2008-04-24 23:50 PDT (History)
0 users

See Also:

Proposed patch (1.67 KB, patch)
2008-04-24 23:28 PDT, Cameron Zwarich (cpst)
no flags Details | Formatted Diff | Diff
Revised proposed patch (1.72 KB, patch)
2008-04-24 23:32 PDT, Cameron Zwarich (cpst)
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Cameron Zwarich (cpst) 2008-04-24 23:22:40 PDT
Consider the following example:


var text = "PASS";

function a()
  var text = "FAIL";
  function b()

  this.c = function()

(new a()).c();


This code should print PASS twice, but instead prints PASS once and then FAIL. This affects the JavaScriptCore test js1_5/Scope/scope-003.js.
Comment 1 Cameron Zwarich (cpst) 2008-04-24 23:28:01 PDT
Created attachment 20812 [details]
Proposed patch

This patch fixes js1_5/Scope/scope-003.js, and it also fixes ecma/String/
Comment 2 Cameron Zwarich (cpst) 2008-04-24 23:32:06 PDT
Created attachment 20813 [details]
Revised proposed patch

The ecma/String/ fix doesn't seem legitimate, so I modified the ChangeLog to reflect this.
Comment 3 Cameron Zwarich (cpst) 2008-04-24 23:41:47 PDT
(In reply to comment #2)
> Created an attachment (id=20813) [edit]
> Revised proposed patch

SunSpider shows no change for this patch.
Comment 4 Geoffrey Garen 2008-04-24 23:45:36 PDT
Comment on attachment 20813 [details]
Revised proposed patch

Comment 5 Geoffrey Garen 2008-04-24 23:48:19 PDT
Committed revision 32536.

Comment 6 Cameron Zwarich (cpst) 2008-04-24 23:50:29 PDT
Upon further review, this patch really did legitimately fix that test case after all.