JS_IsAssigning does not work for increment/decrement operators

VERIFIED WONTFIX

Status

()

VERIFIED WONTFIX
18 years ago
17 years ago

People

(Reporter: jband_mozilla, Assigned: jband_mozilla)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Assignee)

Description

18 years ago
[from news://news.mozilla.org/netscape.public.mozilla.jseng]

Subject: JS_IsAssigning and the increment/decrement operators?

In the new XPConnect code I'm using the same cloned function
object to do double duty as both a setter and a getter. I'm
determining which activity is indicated by calling
JS_IsAssigning. It turns out this does not work in today's
SpiderMonkey for the increment and decrement operators.

I'm wondering about how to deal with this. Is there some other
way to sniff out whether we are in a get or set situation? Since
only one bytecode op is used to indicate both the get and set of
this operator I don't see a strategy similar to the current
JS_IsAssigning js_CodeSpec scheme working.

We could burn the (currently) spare JSPackedBool in the
JSStackFrame struct to indicate this. But we'll have to pay the
runtime overhead of setting and clearing it.

Any other ideas?

It will suck if I need to make separate getter/setter function
objects to deal with this one anomaly. And, of course, we really
don't want JS_IsAssigning to be exposing disinformation.

Thanks,

John.
JS_IsAssigning merely needs to test for JOF_INCDEC as well as JOF_SET.  Can you
do the patch?  I've still got too many changes in my tree (jsapi.c has a change
from the patch for bug 72884, which I hope you'll review soon).  This one is
easy, and shouldn't bottleneck thru me.

/be
Assignee: brendan → jband
(Assignee)

Comment 2

18 years ago
> JS_IsAssigning merely needs to test for JOF_INCDEC as well as JOF_SET.

No. That won't work. The interpreter does both a get and set while executing 
that one op code. 
(Assignee)

Comment 3

18 years ago
Also, I'm happy to do the actual fix. I'm asking for advice. I think we need a 
flag in the frame. I'm wondering if a better solution will present itself. Also, 
can you think of other situations where checking JS_IsAssigning is not a safe 
way to tell whether a property accessor represents set and not a get? Is relying 
on this function really safe? 
(Assignee)

Comment 4

18 years ago
Ugh. I didn't notice that brendan reassigned to me w/o adding himself to the cc 
list. Please note the 'why you're wrong' comments above.
Assignee: jband → brendan
Duh!  Sorry, I didn't get your meaning.  I withdraw that JS_IsAssigning idea,
I'm not convinced testing JOF_INCDEC is right.

But your shared getter/setter function is trivial to handle.  Just test argc to
decide whether the function is being invoked as a getter or a setter.  Or am I
being dense again?

/be
Assignee: brendan → jband
Make sure you spec the function (if native, via JSFunctionSpec or via params to
JS_NewFunction) to take 0 minimum-expected args.  Then when called as a getter,
argc will be 0.  When called as a setter, argc will be 1.  I'm curious why this
didn't occur before the thought of calling JS_IsAssigning?  Maybe the lack of
comments.... </me hides>.

/be
(Assignee)

Comment 7

18 years ago
Yes, that works for me. Part of the reason this solution did not occur to me was 
that I've never really *understood* what the 'minimum-expected args' param to 
JS_NewFunction meant. So, I was misusing it.

So, I'm happy for my bit of code. But we are left with JS_IsAssigning 
giving a less accurate answer than some (like me apparently) might expect. Do we 
want to just document it and mark this bug invalid? or what?
I am not going to add yet more runtime cost to frame construction and bytecode
execution just to make JS_IsAssigning better.  I would rather deprecate this
API, which was added before JSNewResolveOp, and which the flags to
JSNewResolveOp (once I fix bug 72354) makes unnecessary.  Marking WONTFIX.

/be
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → WONTFIX
(Assignee)

Comment 9

18 years ago
I'm good with that. Thanks!
Status: RESOLVED → VERIFIED
So what about

  nativeFunctionReturningLValue()++;

?  Don't such functions have to check JS_IsAssigning?
js> function f() { 
  a++;
  func()++;
3: invalid increment operand:
3:   func()++;
3: ........^

Well, that sucks.

Reference types suck.

/be
Bug 75688 asks for the "reference-type-returning native function can't be the
operand of an inc/decrement operator" bug shaver found to be fixed.

/be
You need to log in before you can comment on or make changes to this bug.