Closed Bug 882468 Opened 7 years ago Closed 7 years ago

Simplify BooleanGetPrimitiveValue

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: till, Assigned: till)

References

Details

Attachments

(2 files)

No description provided.
This is even simpler than the version I gisted earlier, in that it makes the entire call path infallible.

Given how much simpler this is than what we currently have, I don't think the fact that we have to know about the input being a CCW matters too much.
Attachment #761763 - Flags: review?(jwalden+bmo)
Comment on attachment 761763 [details] [diff] [review]
Simplify BooleanGetPrimitiveValue

Review of attachment 761763 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsbool.h
@@ +25,1 @@
>  BooleanGetPrimitiveValue(JSContext *cx, HandleObject obj, Value *vp);

Make this return the boolean value directly, rather than by outparam.  (And put the context at the end to indicate infallibility.)
Attachment #761763 - Flags: review?(jwalden+bmo) → review+
On my Linux box I get three regtest failures because of this:

  ecma_5/JSON/stringify-boxed-primitives.js
  ecma_5/JSON/stringify.js
  ecma_5/JSON/stringify-primitives.js

In all three cases the cause is the same:

  Assertion failure: wrappedBool->isCrossCompartmentWrapper(), at /home/njn/moz/mi8/js/src/jsbool.cpp:205
How did I not see this?
Attachment #764665 - Flags: review?(evilpies)
Comment on attachment 764665 [details] [diff] [review]
Fix bustage caused by simplifying BooleanGetPrimitiveValue.

Review of attachment 764665 [details] [diff] [review]:
-----------------------------------------------------------------

:)
Attachment #764665 - Flags: review?(evilpies) → review+
So, this was actually backed out (https://hg.mozilla.org/integration/mozilla-inbound/rev/4703321564be), so I folded the two patches into one and relanded:
https://hg.mozilla.org/integration/mozilla-inbound/rev/03c829d7d4e7
https://hg.mozilla.org/mozilla-central/rev/03c829d7d4e7
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Depends on: 886094
Depends on: 903180
You need to log in before you can comment on or make changes to this bug.