258098 - nsValueArray needs a SetSize()

Status: RESOLVED WONTFIX

(Core :: XPCOM, defect)

Description

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

<CTho> do we need to implement a setsize for nsvaluearray?
<NeilZZZ> it looks like it, the reverse code needs it and the thread sort code
needs it
<CTho> so setsize needs to either truncate or grow and fill with empty values? 
<NeilZZZ> right... not as easy as replace was

Comment 1

[Régis Caspar]

Attachment: proposed patch + new test for nsValueArray

Attached a patch proposal. Is it normal that the [] operator fails (returns a pointer (?) here and not a nvValueArrayValue)?
Is this patch the suitable place to add a test?

Comment 2

[[e-mail]]

(In reply to comment #1)
>Attached a patch proposal.
Thanks for the patch, but I don't think this quite does what we were looking for; this bug was filed to make nsUInt32Array.cpp obsolete, and its SetSize changes the count (and clears new items) as well as the checking the capacity.

>Is this patch the suitable place to add a test?
Yes.

Comment 3

[Régis Caspar]

Thanks for your answer Neil, I will take a look at nsUInt32Array.cpp and will submit a new patch if no-one did it before.

Comment 4

[Régis Caspar]

Attachment: proposed patch + new test for nsValueArray

Here is a new patch.

Comment 5

[Régis Caspar]

Attachment: test results

This is my test program output

Comment 6

[[e-mail]]

OK, so the next step is to find someone to review your patch. [e-mail] [e-mail] and [e-mail] are listed on the module owners document as current peers although benjamin looks to be the most active.

Comment 7

[Régis Caspar]

Comment on attachment 274274 [details] [diff] [review]
proposed patch + new test for nsValueArray

Thanks for the directions Neil :)

Comment 8

[Benjamin Smedberg]

What are the clients of nsValueArray, and why do we want to add this API to it? I would prefer that all new consumers use nsTArray<PRInt32> or something like that.

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

guess this'd be for bug 250811, which predates nsTArray

Comment 10

[Benjamin Smedberg]

Comment on attachment 274274 [details] [diff] [review]
proposed patch + new test for nsValueArray

I would prefer not to extend this class any more, but switch clients to nsTArray (unless there is a compelling reason I haven't heard yet).

Comment 11

[Régis Caspar]

OK (I don't know the background history of that bug..)

-> Removing the "helpwanted" for now.