"getter" keyword still decompiles as "(null)" in this case

VERIFIED FIXED

Status

()

Core
JavaScript Engine
VERIFIED FIXED
12 years ago
12 years ago

People

(Reporter: Jesse Ruderman, Assigned: Robert Sayre)

Tracking

({testcase, verified1.8.1})

Trunk
PowerPC
Mac OS X
testcase, verified1.8.1
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

12 years ago
The "setter" keyword still decompiles as "(null)" if the thing that precedes it is just an identifier:

js> function() { foo setter = function(){} }
function () {
    foo (null)= function () {};
}

The patch for bug 349616 fixed this for the case of "foo.bar setter":

js> function() { foo.bar setter = function(){} }
function () {
    foo.bar setter= function () {};
}
(Assignee)

Comment 1

12 years ago
Created attachment 237638 [details] [diff] [review]
check for getter/setter elsewhere

js> var x = function() { foo setter = function(){} }
js> x
function () {
    foo (null)= function () {};
}
js> dis(x)
flags: LAMBDA
main:
00000:  bindname "foo"
00003:  anonfunobj (function () {})
00006:  setter
00007:  setname "foo"
00010:  pop
00011:  stop

Source notes:
  0:     6 [   6] assignop
js> var x = function() { foo.bar setter = function(){} }
js> x
function () {
    foo.bar setter= function () {};
}
js> dis(x)
flags: LAMBDA
main:
00000:  name "foo"
00003:  anonfunobj (function () {})
00006:  setter
00007:  setprop "bar"
00010:  pop
00011:  stop

Source notes:
  0:     6 [   6] assignop
  1:     7 [   1] pcbase   offset 7
Assignee: general → sayrer
Status: NEW → ASSIGNED
Attachment #237638 - Flags: review?(brendan)
(Assignee)

Comment 2

12 years ago
Comment on attachment 237638 [details] [diff] [review]
check for getter/setter elsewhere

this can happen on JSOP_SETELEM as well. 

js> var x = function(){ var y = new Array(); y[0] getter = function(){}; } 
js> x
function () {
    var y = new Array;
    y[0] (null)= function () {};
}
js> dis(x)
flags: LAMBDA
main:
00000:  name "Array"
00003:  pushobj
00004:  new 0
00007:  setvar 0
00010:  pop
00011:  getvar 0
00014:  zero
00015:  anonfunobj (function () {})
00018:  getter
00019:  setelem
00020:  pop
00021:  stop

Source notes:
  0:     4 [   4] pcbase   offset 4
  2:     7 [   3] decl     offset 0
  4:    18 [  11] xdelta  
  5:    18 [   0] assignop
  6:    19 [   1] pcbase   offset 8
Attachment #237638 - Flags: review?(brendan)
(Assignee)

Comment 3

12 years ago
Created attachment 237651 [details] [diff] [review]
deal with getter/setter in arrays, xml, and plain identifiers

>              case JSOP_SETELEM:
>                rval = POP_STR();
>                op = JSOP_NOP;           /* turn off parens */
>                xval = POP_STR();
>                op = saveop;
>                lval = POP_STR();
>                if (*xval == '\0')
>                    goto do_setlval;
>                sn = js_GetSrcNote(jp->script, pc - 1);
>                todo = Sprint(&ss->sprinter,
>                              (js_CodeSpec[lastop].format & JOF_XMLNAME)
>                              ? "%s.%s %s= %s"
>                              : "%s[%s] %s= %s",

It's not clear to me where we're going to get the first format string here. Also, the introductory comment in jsopcode.tbl glosses over the fact that not all of JOF_ASSIGNING has a precedence of 3. 

>                              lval, xval,
>                              (sn && SN_TYPE(sn) == SRC_ASSIGNOP)
>                                ? (lastop == JSOP_GETTER)
>                                ? js_getter_str
>                                : (lastop == JSOP_SETTER)
>                                ? js_setter_str
>                                : js_CodeSpec[lastop].token
>                              : "",
>                              rval);
Attachment #237638 - Attachment is obsolete: true
Attachment #237651 - Flags: review?(brendan)
(Assignee)

Comment 4

12 years ago
Are we looking for that first format string here?

js> var x = function(){ var foo = <foo bar="baz"/>; foo.@bar getter = function(){}; }
js> x
function () {
    var foo = <foo bar="baz"/>;
    foo[@bar] getter= function () {};
}
(In reply to comment #3)
> >                todo = Sprint(&ss->sprinter,
> >                              (js_CodeSpec[lastop].format & JOF_XMLNAME)
> >                              ? "%s.%s %s= %s"
> >                              : "%s[%s] %s= %s",
> 
> It's not clear to me where we're going to get the first format string here.

D'oh, lastop is wrong -- it's JSOP_GETTER or JSOP_SETTER of course.  We want ss->opcodes[ss->top+1] (xval's op, still on the stack after xval was POP_STR'ed). Can you give that a whirl?

> Also, the introductory comment in jsopcode.tbl glosses over the fact that not
> all of JOF_ASSIGNING has a precedence of 3.

No, the comment is right and I missed three cases:

$ grep JOF_ASSIGNING jsopcode.tbl|grep -v '3,  JOF_'
 *  3           =, +=, etc.             JSOP_SETNAME, etc. (all JOF_ASSIGNING)
OPDEF(JSOP_BINDNAME,  108,"bindname",   NULL,         3,  0,  1,  0,  JOF_CONST|JOF_NAME|JOF_SET|JOF_ASSIGNING)
OPDEF(JSOP_SETCALL,     132, "setcall",     NULL,     3, -1,  2, 17,  JOF_UINT16|JOF_SET|JOF_ASSIGNING)
OPDEF(JSOP_BINDXMLNAME,   170,"bindxmlname",NULL,     1,  1,  2,  0,  JOF_BYTE|JOF_SET|JOF_ASSIGNING)

Could you fix these in a new patch here?  Thanks.

/be
(Assignee)

Comment 6

12 years ago
Created attachment 237704 [details] [diff] [review]
fix format string, opcode table
Attachment #237651 - Attachment is obsolete: true
Attachment #237704 - Flags: review?(brendan)
Attachment #237651 - Flags: review?(brendan)
Comment on attachment 237704 [details] [diff] [review]
fix format string, opcode table

Nice, thanks.

/be
Attachment #237704 - Flags: review?(brendan)
Attachment #237704 - Flags: review+
Attachment #237704 - Flags: approval1.8.1?
(Assignee)

Comment 8

12 years ago
Checking in jsopcode.c;
/cvsroot/mozilla/js/src/jsopcode.c,v  <--  jsopcode.c
new revision: 3.166; previous revision: 3.165
done
Checking in jsopcode.tbl;
/cvsroot/mozilla/js/src/jsopcode.tbl,v  <--  jsopcode.tbl
new revision: 3.72; previous revision: 3.71
done
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Comment on attachment 237704 [details] [diff] [review]
fix format string, opcode table

a=beltzner on behalf of 181drivers
Attachment #237704 - Flags: approval1.8.1? → approval1.8.1+
(Assignee)

Comment 10

12 years ago
Checking in jsopcode.c;
/cvsroot/mozilla/js/src/jsopcode.c,v  <--  jsopcode.c
new revision: 3.89.2.45; previous revision: 3.89.2.44
done
Checking in jsopcode.tbl;
/cvsroot/mozilla/js/src/jsopcode.tbl,v  <--  jsopcode.tbl
new revision: 3.43.2.13; previous revision: 3.43.2.12
done
Keywords: fixed1.8.1

Comment 11

12 years ago
Checking in regress-352060.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-352060.js,v  <--  regress-352060.js
initial revision: 1.1
done
Flags: in-testsuite+

Comment 12

12 years ago
verified fixed 1.8 20060914 windows/linux 1.9 20060914 windows/mac*/linux
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1 → verified1.8.1
You need to log in before you can comment on or make changes to this bug.