[llvm] r225055 - Add 2x constructors for TinyPtrVector, one that takes in one elemenet and the other that takes in an ArrayRef<EltTy>

[David Blaikie]

On Wed, Dec 31, 2014 at 10:14 PM, Michael Gottesman <mgottesman at apple.com>
wrote:

>
> On Dec 31, 2014, at 6:47 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Wed, Dec 31, 2014 at 3:33 PM, Michael Gottesman <mgottesman at apple.com>
> wrote:
>
>> Author: mgottesman
>> Date: Wed Dec 31 17:33:24 2014
>> New Revision: 225055
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=225055&view=rev
>> Log:
>> Add 2x constructors for TinyPtrVector, one that takes in one elemenet and
>> the other that takes in an ArrayRef<EltTy>
>>
>> Currently one can only construct an empty TinyPtrVector. These are just
>> missing
>> elements of the API.
>>
>> Modified:
>>     llvm/trunk/include/llvm/ADT/TinyPtrVector.h
>>     llvm/trunk/unittests/ADT/TinyPtrVectorTest.cpp
>>
>> Modified: llvm/trunk/include/llvm/ADT/TinyPtrVector.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/TinyPtrVector.h?rev=225055&r1=225054&r2=225055&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/ADT/TinyPtrVector.h (original)
>> +++ llvm/trunk/include/llvm/ADT/TinyPtrVector.h Wed Dec 31 17:33:24 2014
>> @@ -96,6 +96,13 @@ public:
>>      return *this;
>>    }
>>
>> +  /// Constructor from a single element.
>> +  explicit TinyPtrVector(EltTy Elt) : Val(Elt) {}
>>
>
> Do you need this one? ArrayRef<T> is already implicitly constructible from
> a single T, so the other ctor you added should suffice for this case.
>
> (that said, it's not uncommon that too many user-defined conversions fire
> and these sort of intermediate conversions are needed - so if that's what
> you've hit, that's cool)
>
>
> That is not the reason that I did this.
>
> The reason I did this was from the perspective of API completeness when
> one is looking and exploring the class. I can imagine a user who is not
> familiar with ArrayRef’s constructors wondering where this constructor is
> given the raison d’être of the class. That being said I am not attached to
> it and I am willing to remove it.
>

If the test pass without I'd suggest removing it (& if the tests don't pass
when you remove it, I'd be curious to understand why). We're pretty
pervasively using the single-arg ArrayRef ctor, so I doubt it'd take too
long for a new developer to discover the idiom & become familiar with it.

- David


>
> Michael
>
>
> - David
>
>
>> +
>> +  /// Constructor from an ArrayRef.
>> +  explicit TinyPtrVector(ArrayRef<EltTy> Elts)
>> +      : Val(new VecTy(Elts.begin(), Elts.end())) {}
>> +
>>    // implicit conversion operator to ArrayRef.
>>    operator ArrayRef<EltTy>() const {
>>      if (Val.isNull())
>>
>> Modified: llvm/trunk/unittests/ADT/TinyPtrVectorTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/TinyPtrVectorTest.cpp?rev=225055&r1=225054&r2=225055&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/unittests/ADT/TinyPtrVectorTest.cpp (original)
>> +++ llvm/trunk/unittests/ADT/TinyPtrVectorTest.cpp Wed Dec 31 17:33:24
>> 2014
>> @@ -412,3 +412,29 @@ TYPED_TEST(TinyPtrVectorTest, InsertRang
>>  }
>>
>>  }
>> +
>> +TEST(TinyPtrVectorTest, SingleEltCtorTest) {
>> +  int v = 55;
>> +  TinyPtrVector<int *> V(&v);
>> +
>> +  EXPECT_TRUE(V.size() == 1);
>> +  EXPECT_FALSE(V.empty());
>> +  EXPECT_TRUE(V.front() == &v);
>> +}
>> +
>> +TEST(TinyPtrVectorTest, ArrayRefCtorTest) {
>> +  int data_array[128];
>> +  std::vector<int *> data;
>> +
>> +  for (unsigned i = 0, e = 128; i != e; ++i) {
>> +    data_array[i] = 324 - int(i);
>> +    data.push_back(&data_array[i]);
>> +  }
>> +
>> +  TinyPtrVector<int *> V(data);
>> +  EXPECT_TRUE(V.size() == 128);
>> +  EXPECT_FALSE(V.empty());
>> +  for (unsigned i = 0, e = 128; i != e; ++i) {
>> +    EXPECT_TRUE(V[i] == data[i]);
>> +  }
>> +}
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150102/d9f153e5/attachment.html>