Correct JS_IsArrayObject documentation re proxies

RESOLVED FIXED in Firefox 64

Status

()

enhancement
P2
normal
RESOLVED FIXED
11 months ago
10 months ago

People

(Reporter: karlt, Assigned: karlt)

Tracking

Trunk
mozilla64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(1 attachment)

The documentation for JS_IsArrayObject says

/**
 * Returns true and sets |*isArray| indicating whether |obj| is an Array object
 * or a wrapper around one, otherwise returns false on failure.
 *
 * This method returns true with |*isArray == false| when passed a proxy whose
 * target is an Array, or when passed a revoked proxy.
 */

On reading the second sentence, I can interpret the first differently to make
it consistent:  The function fails whenever |obj| is is not an Array or a
wrapper around one; otherwise the function returns true, setting isArray to
true if an Array and to false if a wrapper.  This is not what the function
does, but it took some investigation to settle on that conclusion.
Thank you for looking at my suggestion, Jeff.

There are potentially two things involved here:

1. I'm not clear from the first sentence whether *isArray will be set to true
or false if |value| is a wrapper around an array.  Was the interpretation in
comment 0 correct, or alternatively would the proposed change to the first
sentence here be accurate?
https://phabricator.services.mozilla.com/D6703?id=18237#change-lZV20MqdA9JJ
I'm assuming the latter because the function succeeds when !value.isObject().

2. I don't know enough about the differences between wrappers and proxies.
This may be contributing to my confusion. My reading of the code was that
ForwardingProxyHandler::getBuiltinClass() would indicate whether the target
was an array or not.  Is ForwardingProxyHandler a wrapper or a proxy or both?
Am I reading the code wrong?
https://searchfox.org/mozilla-central/rev/ffe6eaf2f032e58ec3b0650a87df2c62ae4ca441/js/src/proxy/Wrapper.cpp#256
Flags: needinfo?(jwalden+bmo)
Blocks: 1473467
In an attempt to clarify 1, the doc says

  x is set indicating whether y is A or B.

This may be interpreted to mean that, if x is set, then y ∈ {A, B}
and the value of x will indicate which from the two possibilities.

I assume the intended meaning is instead

  x is set indicating whether or not y ∈ {A, B}.
Attachment #9011600 - Attachment description: Bug 1493806 correct and clarify JS_IsArrayObject documentation re proxies r? → Bug 1493806 clarify JS_IsArrayObject documentation re proxies
Flags: needinfo?(jwalden+bmo)
Pushed by ktomlinson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cd94fba04864
clarify JS_IsArrayObject documentation re proxies r=jwalden
Blocks: 1497388
https://hg.mozilla.org/mozilla-central/rev/cd94fba04864
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.