325331 - Make nsTArray a bit more useful

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Darin Fisher]

Make nsTArray a bit more useful:

  1. Support Append, Insert, and Replace from items of type A on an array of
     type B, provided instances of B can be constructed from A.

  2. Return pointer to newly constructed elements from Append, Insert, and
     Replace instead of a boolean flag.

The first change (#1) will make the following code more efficient:

  nsTArray<nsCString> ary;
  ary.AppendElement("foo");

Previously, this code was supported, but it resulted in a temporary nsCString being created.  That temporary is eliminated by #1.

The second change (#2) is just a nice to have for consistency with the versions of Append and Insert that construct an element using the element's default constructor.

Comment 1

[Darin Fisher]

Attachment: v1 patch

I've only verified that this compiles on VC7.1, but I have a feeling that it will work elsewhere too.  Crossing my fingers ;-)

Comment 2

[Benjamin Smedberg]

Comment on attachment 210243 [details] [diff] [review]
v1 patch

Nested templates should work, let's find out when you checkin ;-)

Comment 3

[Darin Fisher]

fixed-on-trunk

Comment 4

[Darin Fisher]

It looks like creature was unhappy with this change:

TestTArray.cpp
../../dist/include/xpcom\nsTArray.h(238) : error C2667: 'ReplaceElementsAt' : none of 2 overload have a best conversion
        ../../dist/include/xpcom\nsTArray.h(237) : while compiling class-template member function 'class nsTArray<class nsCString> &__thiscall nsTArray<class nsCString>::operator =(const class nsTArray<class nsCString> &)'
../../dist/include/xpcom\nsTArray.h(238) : error C2668: 'ReplaceElementsAt' : ambiguous call to overloaded function
        ../../dist/include/xpcom\nsTArray.h(237) : while compiling class-template member function 'class nsTArray<class nsCString> &__thiscall nsTArray<class nsCString>::operator =(const class nsTArray<class nsCString> &)'
make[5]: *** [TestTArray.obj] Error 2

I decided to back this patch out until I have a chance to debug it on a system with VC6.

Comment 5

[Darin Fisher]

Relanded this patch w/ a fix for VC6.

Comment 6

[Darin Fisher]

Comment on attachment 210243 [details] [diff] [review]
v1 patch

I'd like to keep nsTArray in sync between trunk and 1.8 branch if possible since the class is used by places.

Comment 7

[Benjamin Smedberg]

Comment on attachment 210243 [details] [diff] [review]
v1 patch

What was the VC6 fix?

Comment 8

[Darin Fisher]

I modified the copy constructor to call the 4 argument version of ReplaceElementsAt instead of the 3 argument version.  Interestingly enough, the 4 argument version works fine when called directly or indirectly via InsertElementsAt.  It was just some problem related to the copy constructor that was tripping up VC6 apparently.

Comment 9

[Darin Fisher]

Attachment: patch for 1.8 branch

Here's the patch for the 1.8 branch.

Comment 10

[Darin Fisher]

fixed1.8.1