As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
Last Comment Bug 630117 - rename typed array slice() to subarray()
: rename typed array slice() to subarray()
Status: RESOLVED FIXED
[softblocker]
: dev-doc-complete
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Vladimir Vukicevic [:vlad] [:vladv]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-01-30 22:13 PST by Vladimir Vukicevic [:vlad] [:vladv]
Modified: 2011-02-03 11:08 PST (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
rename slice -> subset (5.82 KB, patch)
2011-01-31 13:01 PST, Vladimir Vukicevic [:vlad] [:vladv]
jwalden+bmo: review+
Details | Diff | Splinter Review
rename slice -> subarray (5.87 KB, patch)
2011-02-01 11:47 PST, Vladimir Vukicevic [:vlad] [:vladv]
jwalden+bmo: review+
Details | Diff | Splinter Review
update webgl tests (5.68 KB, patch)
2011-02-01 17:34 PST, Vladimir Vukicevic [:vlad] [:vladv]
no flags Details | Diff | Splinter Review

Description User image Vladimir Vukicevic [:vlad] [:vladv] 2011-01-30 22:13:18 PST
We decided last week that we screwed up with the slice() naming, and changed the name of that function to subset().  We should get this change in, so that we can get a real slice() in the future.  (Current slice() creates a new view object, but still references the same underlying data.)  Marking this as a softblocker for API consistency, but I'll post a patch tomorrow.
Comment 1 User image Brendan Eich [:brendan] 2011-01-30 23:43:44 PST
Why "subset"? There's no sane set-theoretic interpretation. As P. Tucker Withington pointed out, the view objects are like Common Lisp's "Displaced Arrays". Not that you should use that name. "sliceView"?

/be
Comment 2 User image Vladimir Vukicevic [:vlad] [:vladv] 2011-01-31 13:01:13 PST
Created attachment 508511 [details] [diff] [review]
rename slice -> subset

renamed, and tests updated; also added a test to ensure that subset() behaves as intended (references same buffer).
Comment 3 User image David Humphrey (:humph) 2011-01-31 13:24:32 PST
CC'ing some others who might get broken by this.  FYI, gentlemen.
Comment 4 User image Vladimir Vukicevic [:vlad] [:vladv] 2011-01-31 14:00:43 PST
I don't have a good answer as to "why subset", other than that Ken & I liked that name, and it didn't collide with "slice"; the slice wart we should've caught a while ago (and I think Brendan even mentioned it originally :/)
Comment 5 User image Dave Herman [:dherman] 2011-01-31 15:35:39 PST
Wow, "subset" smells pretty funky to me... neither the original array nor the result array is a set. I can see how "slice" could be misleading given that array.slice produces a fresh array. How about "subarray"?

Some of the original work on this kind of API was known as "subsequences" since it was about taking pieces of (possibly mutable) sequence types that share references:

    http://portal.acm.org/citation.cfm?id=133234

Seems like a good analogy: subarray : array :: subsequence : sequence. Especially since an array is a kind of sequence.

Dave
Comment 6 User image Jeff Walden [:Waldo] (remove +bmo to email) 2011-01-31 15:49:38 PST
subrange, subview, sharedRange, sharedSlice
Comment 7 User image Vladimir Vukicevic [:vlad] [:vladv] 2011-01-31 17:20:01 PST
I don't think we get to pick a new name at this point... Ken already made the change in WebKit.  Cc'ing him here -- really should get some JS folks involved in the typed array work :)
Comment 8 User image Jeff Walden [:Waldo] (remove +bmo to email) 2011-01-31 18:00:20 PST
Fair enough, throwing them out just in case.

As far as JS people getting involved, my understanding is that ECMA is doing its own thing in this space, looking to supplant typed arrays.  See comment 575688 comment 6.
Comment 9 User image Vladimir Vukicevic [:vlad] [:vladv] 2011-01-31 18:03:34 PST
Comment on attachment 508511 [details] [diff] [review]
rename slice -> subset

(Argh, I hate that you're not :waldo :-)

Yeah, TC39 is doing its own thing, but we have this now and will likely have it for quite a while... we're already depending on it very heavily for a lot of Fx4 launch demos and the like :-)
Comment 10 User image Jeff Walden [:Waldo] (remove +bmo to email) 2011-01-31 18:48:06 PST
Comment on attachment 508511 [details] [diff] [review]
rename slice -> subset

rs=me assuming it builds and tests out fine.

We should eventually move this over to use the atom stuff and shared const char[] in jsatom.h, but that certainly doesn't need to happen now, nor should it, really.
Comment 11 User image Kenneth Russell 2011-01-31 23:36:20 PST
Since we've already broken content with the slice -> subset renaming, I suppose if we quickly renamed it again it wouldn't be that bad.

Of the above suggestions, I like subarray the best. There are more view types of ArrayBuffers than just the Typed Arrays, in particular DataView, which doesn't support this operation. Therefore subview doesn't seem appropriate. subrange seems okay, but if we're adding more letters to the method name it seems to me we might as well keep the name focused on the array-like nature of these objects.

Vlad, what do you think?
Comment 12 User image Grant Galitz 2011-02-01 10:51:48 PST
"Since we've already broken content with the slice -> subset renaming, I suppose
if we quickly renamed it again it wouldn't be that bad." Let's go 'derper' with the renaming now: http://images0.memegenerator.net/ImageMacro/3784263/we-need-to-go-derper.jpg?imageSize=Medium&generatorName=Inception
Comment 13 User image Vladimir Vukicevic [:vlad] [:vladv] 2011-02-01 10:59:35 PST
If you're ok with an additional rename, subarray sounds fine, or 'subarrayview', but now maybe we're trying too hard.
Comment 14 User image Brendan Eich [:brendan] 2011-02-01 11:19:59 PST
+1 on subarray.

(subarrayview is overlong and violates camelCaps -- subarray does not if it's one word, which neologisms and back-formations quickly become, esp. in hacker jargon.)

/be
Comment 15 User image Kenneth Russell 2011-02-01 11:21:19 PST
If we're in agreement on subarray, I'll update the Typed Array spec and conformance tests and do yet another rename on the WebKit side. Vlad?
Comment 16 User image Vladimir Vukicevic [:vlad] [:vladv] 2011-02-01 11:24:20 PST
sounds good to me, i'll update this patch
Comment 17 User image Grant Galitz 2011-02-01 11:25:09 PST
ಠ_ಠ
Comment 18 User image Vladimir Vukicevic [:vlad] [:vladv] 2011-02-01 11:47:48 PST
Created attachment 508837 [details] [diff] [review]
rename slice -> subarray

slice -> subarray!
Comment 19 User image Grant Galitz 2011-02-01 11:49:52 PST
I hope I don't come back here to see that it's been changed another two times.
Comment 20 User image Jeff Walden [:Waldo] (remove +bmo to email) 2011-02-01 11:54:57 PST
Comment on attachment 508837 [details] [diff] [review]
rename slice -> subarray

rs=me again
Comment 21 User image Vladimir Vukicevic [:vlad] [:vladv] 2011-02-01 15:09:56 PST
http://hg.mozilla.org/mozilla-central/rev/84c8b33619e0

death to slice, long live subarray.
Comment 22 User image Vladimir Vukicevic [:vlad] [:vladv] 2011-02-01 15:10:23 PST
argh, the commit comment is wrong; I meant subarray! (code is correct)
Comment 23 User image Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-02-01 16:26:08 PST
This turned tests orange. I checked in a attempted fix:
http://hg.mozilla.org/mozilla-central/rev/808970eca6d5
Comment 24 User image Brendan Eich [:brendan] 2011-02-01 16:30:07 PST
(In reply to comment #23)
> This turned tests orange. I checked in a attempted fix:
> http://hg.mozilla.org/mozilla-central/rev/808970eca6d5

And the checkin comment/stale bug summary bites. Sicking renamed the calls from slice to subset!

Vlad, can you fix? rs=me

/be
Comment 25 User image Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-02-01 16:51:45 PST
Ugh, didn't realize that the checkin-comment didn't reflect reality. Backed out the whole thing since this has already kept the tree orange for several cycles and my fix was risky enough as I didn't have time to compile a fresh tree and test.

http://hg.mozilla.org/mozilla-central/rev/298c555d0c6e
Comment 26 User image Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-02-01 17:12:32 PST
For what it's worth, my patch didn't apparently cover all cases which needed s/slice/subfoo/
Comment 27 User image Vladimir Vukicevic [:vlad] [:vladv] 2011-02-01 17:27:53 PST
Yeah, sorry -- I tested JS, but forgot that the webgl tests had coverage of this :(  I have everything patched locally (with correct commit message this time), but refreshing my build to make sure everything's good.
Comment 28 User image Vladimir Vukicevic [:vlad] [:vladv] 2011-02-01 17:34:31 PST
Created attachment 508967 [details] [diff] [review]
update webgl tests

The additional patch.
Comment 29 User image Vladimir Vukicevic [:vlad] [:vladv] 2011-02-01 18:26:12 PST
relanded with additional webgl test fixes.
Comment 30 User image Eric Shepherd [:sheppy] 2011-02-03 11:08:38 PST
Reminder that this is one that should have gotten a dev-doc-needed keyword on it. :)

That said, documentation has been updated, thanks to Dave Humphrey's blog post catching my eye.

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