Convert JSVAL_TO_INT and INT_TO_JSVAL to functions

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: bz, Assigned: jorendorff)

Tracking

Trunk
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment)

We can't do it right now because we have static asserts that use them.  But waldo says "C++0x constexpr might be able to save the day here, possibly"
(Assignee)

Comment 1

9 years ago
Created attachment 393400 [details] [diff] [review]
v1
Assignee: general → jorendorff
Attachment #393400 - Flags: review?(jwalden+bmo)

Updated

9 years ago
Attachment #393400 - Flags: review?(jwalden+bmo) → review+

Comment 2

9 years ago
Comment on attachment 393400 [details] [diff] [review]
v1

Better than nothing, I'm just worried we'll break JSAPI users doing this.  I guess it's easy to fix, and they can not upgrade if necessary, but still...
(Assignee)

Comment 3

9 years ago
http://hg.mozilla.org/tracemonkey/rev/d6f79987bf06

Cc-ing Brendan because of the concerns raised in comment 2.  But we have already turned several other macros into functions.  Type safety is worth the breakage.

Within SM we broke one thing, and it turned out to be a completely bogus call to INT_TO_JSVAL. See bug 509301.
(Assignee)

Updated

9 years ago
Whiteboard: fixed-in-tracemonkey
(Assignee)

Updated

9 years ago
Depends on: 509301

Comment 4

9 years ago
http://hg.mozilla.org/mozilla-central/rev/d6f79987bf06
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Depends on: 511836

Comment 5

9 years ago
That broke me, big-time, in multiple places.

I use JSVAL_ZERO as the case label in switch statemnts. I can change those to stacked if..else blocks, I suppose. So I can live with that.

The REAL big breaker is that I have enums of jsvals, e.g. 

typedef enum { jsve_uchar = INT_TO_JSVAL(inte_uchar}, ... } gffi_jsv_e;

I use this pattern in several places, where I need to expose constants to JavaScript and then act on which constant is chosen from C.  

Often, the value of the constant isn't even relevant except as a conditional, so I never even wind up converting from jsval-space back into int-space; I can just use the jsvals directly.

Is there a way to fix the API to keep code like mine running, without losing the type safety benefit?

Comment 6

9 years ago
Recap of IRC conversation with jorendorff:

 - INT_TO_JSVAL_CONSTEXPR() exists now and can take the place of old INT_TO_JSVAL() calls where macro is absolutely required

 - JSVAL_ZERO and JSVAL_ONE should be changed to use INT_TO_JSVAL_CONSTEXPR() at some point, so that JSAPI users can use them in switch cases
You need to log in before you can comment on or make changes to this bug.