If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

trap() changes TypeError message

RESOLVED WORKSFORME

Status

()

Core
JavaScript Engine
--
minor
RESOLVED WORKSFORME
9 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: h4writer)

Tracking

(Blocks: 1 bug, {testcase})

Trunk
x86
Mac OS X
testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

9 years ago
js> f = function() { var x; x.y; }
function () {
    var x;
    x.y;
}

js> dis(f);
flags: LAMBDA INTERPRETED
main:
00000:  getlocal 0
00003:  pop
00004:  getlocalprop 0 "y"
00009:  pop
00010:  stop

Source notes:
  0:     0 [   0] decl     offset 0

js> f();
typein:1: TypeError: x is undefined
js> trap(f, 0, "");
js> f();
typein:1: TypeError: x.y is undefined
(Assignee)

Comment 1

6 years ago
Using latest mozilla-inbound I get the following when executing the testcase:

js> f = function() { var x; x.y; }
(function () {var x;x.y;})
js> dis(f);
flags: LAMBDA NULL_CLOSURE
loc     op
-----   --
main:
00000:  getlocal 0
00003:  pop
00004:  getlocal 0
00007:  getprop "y"
00010:  pop
00011:  stop

Source notes:
 ofs  line    pc  delta desc     args
---- ---- ----- ------ -------- ------
  0:    1     0 [   0] decl     offset 0
  2:    1     7 [   7] pcbase   offset 3

js> f();
typein:1: TypeError: x is undefined
js> trap(f, 0, "");
js> f();
typein:1: TypeError: x is undefined
js> 

Could not find the original bug report fixing this bug,
so I mark it as WORKSFORME.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → WORKSFORME
Flags: in-testsuite?
(Assignee)

Comment 2

6 years ago
Created attachment 577606 [details] [diff] [review]
Add testcase

@Ryan: I suppose that flag means you want a testcase? I added a patch containing the testcase. I hope it is in the right place with the right parameters (debug needed!)? I added you as reviewer. Feel free to point it to somebody else, but it's only a testcase ...
Assignee: general → hv1989
Status: RESOLVED → REOPENED
Attachment #577606 - Flags: review?(ryanvm)
Resolution: WORKSFORME → ---
Comment on attachment 577606 [details] [diff] [review]
Add testcase

In general, it's always good to add tests for bugs that go away whether FIXED or WORKSFORME. I'm not the right reviewer for it though. Sending over to Jesse. I can push it to try later, though.
Attachment #577606 - Flags: review?(ryanvm) → review?(jruderman)
(Reporter)

Updated

6 years ago
Attachment #577606 - Flags: review?(jruderman) → review+
(Reporter)

Comment 4

6 years ago
Thanks for converting the test, Hannes :)
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/c7d671e9cf58

For future reference, it would be awesome to have a checkin comment and User/From line in the patch!
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → WORKSFORME
Flags: in-testsuite? → in-testsuite+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c7d671e9cf58
You need to log in before you can comment on or make changes to this bug.