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

[Michael Gottesman]

> On Jan 2, 2015, at 2:19 PM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Wed, Dec 31, 2014 at 10:14 PM, Michael Gottesman <mgottesman at apple.com <mailto:mgottesman at apple.com>> wrote:
> 
>> On Dec 31, 2014, at 6:47 PM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
>> 
>> 
>> 
>> On Wed, Dec 31, 2014 at 3:33 PM, Michael Gottesman <mgottesman at apple.com <mailto: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 <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 <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.

Sorry for the delay (r226424). As a bonus I hide the data inside TinyPtrVector. There is no good reason for that to be public.

Michael

> 
> - 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 <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 <mailto:llvm-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <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/20150118/831093cc/attachment.html>