Fix ArrayRef initializer_list Ctor Test

[erichkeane]

Summary

The InitializerList test had undefined behavior by creating a dangling pointer to the temporary initializer list. This patch removes the undefined behavior in the test by creating the initializer list directly.

[mehdi_amini]

LGTM.

unittests/ADT/ArrayRefTest.cpp

150

Why not turn it into a lambda so it stays local to the particular test that is using it?

[erichkeane]

unittests/ADT/ArrayRefTest.cpp

150

Good idea, New patch incoming.

[erichkeane]

Switched test to a lambda. BTW, I don't have commit rights, so if this gets approved and someone would commit for me, I'd be very appreciative.

Thanks!

[dblaikie]

I'd probably just keep one of these two tests - they don't seem to add a lot separately. Probably just keep the "more realistic" version, I would think (but put it in the existing test).

But no big deal either way - just my 2c :)

[erichkeane]

I'd probably just keep one of these two tests - they don't seem to add a lot separately. Probably just keep the "more realistic" version, I would think (but put it in the existing test).

But no big deal either way - just my 2c :)

I actually just realized that the ArgTest12 tests exactly the 'realistic' test anyway. I've removed the realistic test, and left this as simply the fix for the UB.

[erichkeane]

Redundant test removed

unittests/ADT/ArrayRefTest.cpp

 }
 
 TEST(ArrayRefTest, InitializerList) {
-  ArrayRef<int> A = { 0, 1, 2, 3, 4 };
+  std::initializer_list<int> init_list = { 0, 1, 2, 3, 4 };
+  ArrayRef<int> A = init_list;
   for (int i = 0; i < 5; ++i)
     EXPECT_EQ(i, A[i]);
 

   ArgTest12({1, 2});
 }
 
+// A more realistic test for the Init-list ctor
+TEST(ArrayRefTest, InitializerListTemporary) {
+  auto func =[](ArrayRef<int> A) { 
+    for (int i = 0; i < 5; ++i)
+      EXPECT_EQ(i, A[i]);
+  };
+
+  func({0, 1, 2, 3, 4 });
+}
+  
+
 // Test that makeArrayRef works on ArrayRef (no-op)
 TEST(ArrayRefTest, makeArrayRef) {
   static const int A1[] = {1, 2, 3, 4, 5, 6, 7, 8};