<p dir="ltr">Will be able to have a look in a few hours. Feel free to revert in the interim.</p>
<div class="gmail_quote">On Apr 21, 2014 6:03 PM, "Quentin Colombet" <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word">Hi David,<div><br></div><div>Looks like your commit broke some bots:</div><div><a href="http://lab.llvm.org:8013/builders/clang-x86_64-darwin11-nobootstrap-RAincremental/builds/14682" target="_blank">http://lab.llvm.org:8013/builders/clang-x86_64-darwin11-nobootstrap-RAincremental/builds/14682</a></div>
<div><br></div><div><pre style="font-family:'Courier New',courier,monotype;font-size:medium"><span>******************** TEST 'LLVM-Unit :: Transforms/Utils/Release+Asserts/UtilsTests/CloneInstruction.Exact' FAILED ********************
Note: Google Test filter = CloneInstruction.Exact
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from CloneInstruction
[ RUN      ] CloneInstruction.Exact
UtilsTests(60096) malloc: *** error for object 0x104f036c0: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
0  UtilsTests        0x0000000104d9ee48 llvm::sys::PrintStackTrace(__sFILE*) + 40
1  UtilsTests        0x0000000104d9f324 _ZL13SignalHandleri + 564
2  libsystem_c.dylib 0x00007fff94461cfa _sigtramp + 26
3  libsystem_c.dylib 0x0000000000000190 _sigtramp + 1807344816
4  libsystem_c.dylib 0x00007fff9445f84c free + 389
5  UtilsTests        0x0000000104cbc2c0 std::__1::__tree<std::__1::unique_ptr<llvm::Value, std::__1::default_delete<llvm::Value> >, std::__1::less<std::__1::unique_ptr<llvm::Value, std::__1::default_delete<llvm::Value> > >, std::__1::allocator<std::__1::unique_ptr<llvm::Value, std::__1::default_delete<llvm::Value> > > >::destroy(std::__1::__tree_node<std::__1::unique_ptr<llvm::Value, std::__1::default_delete<llvm::Value> >, void*>*) + 64
6  UtilsTests        0x0000000104cb46e0 (anonymous namespace)::CloneInstruction::TearDown() + 64
7  UtilsTests        0x0000000104ccfed0 testing::TestInfo::Run() + 832
8  UtilsTests        0x0000000104cd0430 testing::TestCase::Run() + 448
9  UtilsTests        0x0000000104cd75d5 testing::internal::UnitTestImpl::RunAllTests() + 1317
10 UtilsTests        0x0000000104cd709a testing::UnitTest::Run() + 106
11 UtilsTests        0x0000000104ce5061 main + 65
12 UtilsTests        0x0000000104cacd54 start + 52
13 UtilsTests        0x0000000000000002 start + 4214567650

********************</span></pre><div><br></div></div><div><br></div><div>Could you take a look please?</div><div><br></div><div>Thanks,<br><div>
<div style="text-indent:0px;letter-spacing:normal;font-variant:normal;text-align:-webkit-auto;font-style:normal;font-weight:normal;line-height:normal;text-transform:none;white-space:normal;font-family:Helvetica;word-wrap:break-word;word-spacing:0px">
-Quentin</div>

</div>
<br><div><div>On Apr 21, 2014, at 4:47 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br><blockquote type="cite">Author: dblaikie<br>Date: Mon Apr 21 18:47:47 2014<br>
New Revision: 206828<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=206828&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=206828&view=rev</a><br>Log:<br>Use unique_ptr to handle ownership of Value*s in Cloning unit tests.<br>
<br>Modified:<br>    llvm/trunk/unittests/Transforms/Utils/Cloning.cpp<br><br>Modified: llvm/trunk/unittests/Transforms/Utils/Cloning.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Utils/Cloning.cpp?rev=206828&r1=206827&r2=206828&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Utils/Cloning.cpp?rev=206828&r1=206827&r2=206828&view=diff</a><br>
==============================================================================<br>--- llvm/trunk/unittests/Transforms/Utils/Cloning.cpp (original)<br>+++ llvm/trunk/unittests/Transforms/Utils/Cloning.cpp Mon Apr 21 18:47:47 2014<br>
@@ -10,7 +10,6 @@<br> #include "llvm/Transforms/Utils/Cloning.h"<br> #include "llvm/ADT/ArrayRef.h"<br> #include "llvm/ADT/STLExtras.h"<br>-#include "llvm/ADT/SmallPtrSet.h"<br> #include "llvm/IR/Argument.h"<br>
 #include "llvm/IR/Constant.h"<br> #include "llvm/IR/DebugInfo.h"<br>@@ -25,6 +24,8 @@<br> #include "llvm/IR/LLVMContext.h"<br> #include "gtest/gtest.h"<br><br>+#include <set><br>
+<br> using namespace llvm;<br><br> namespace {<br>@@ -38,34 +39,38 @@ protected:<br>   template <typename T><br>   T *clone(T *V1) {<br>     Value *V2 = V1->clone();<br>-    Orig.insert(V1);<br>-    Clones.insert(V2);<br>
+    std::unique_ptr<Value> V(V1);<br>+    if (!Orig.insert(std::move(V)).second)<br>+      V.release(); // this wasn't the first time we added the element, so the<br>+                   // set already had ownership<br>
+    Clones.insert(std::unique_ptr<Value>(V2));<br>     return cast<T>(V2);<br>   }<br><br>-  void eraseClones() {<br>-    DeleteContainerPointers(Clones);<br>-  }<br>+  void eraseClones() { Clones.clear(); }<br>
<br>   virtual void TearDown() {<br>     eraseClones();<br>-    DeleteContainerPointers(Orig);<br>-    delete V;<br>+    Orig.clear();<br>+    V.reset();<br>   }<br><br>-  SmallPtrSet<Value *, 4> Orig;   // Erase on exit<br>
-  SmallPtrSet<Value *, 4> Clones; // Erase in eraseClones<br>+  std::set<std::unique_ptr<Value>> Orig;   // Erase on exit<br>+  std::set<std::unique_ptr<Value>> Clones; // Erase in eraseClones<br>
<br>   LLVMContext context;<br>-  Value *V;<br>+  std::unique_ptr<Value> V;<br> };<br><br> TEST_F(CloneInstruction, OverflowBits) {<br>-  V = new Argument(Type::getInt32Ty(context));<br>+  V = make_unique<Argument>(Type::getInt32Ty(context));<br>
<br>-  BinaryOperator *Add = BinaryOperator::Create(Instruction::Add, V, V);<br>-  BinaryOperator *Sub = BinaryOperator::Create(Instruction::Sub, V, V);<br>-  BinaryOperator *Mul = BinaryOperator::Create(Instruction::Mul, V, V);<br>
+  BinaryOperator *Add =<br>+      BinaryOperator::Create(Instruction::Add, V.get(), V.get());<br>+  BinaryOperator *Sub =<br>+      BinaryOperator::Create(Instruction::Sub, V.get(), V.get());<br>+  BinaryOperator *Mul =<br>
+      BinaryOperator::Create(Instruction::Mul, V.get(), V.get());<br><br>   BinaryOperator *AddClone = this->clone(Add);<br>   BinaryOperator *SubClone = this->clone(Sub);<br>@@ -131,12 +136,12 @@ TEST_F(CloneInstruction, OverflowBits) {<br>
 }<br><br> TEST_F(CloneInstruction, Inbounds) {<br>-  V = new Argument(Type::getInt32PtrTy(context));<br>+  V = make_unique<Argument>(Type::getInt32PtrTy(context));<br><br>   Constant *Z = Constant::getNullValue(Type::getInt32Ty(context));<br>
   std::vector<Value *> ops;<br>   ops.push_back(Z);<br>-  GetElementPtrInst *GEP = GetElementPtrInst::Create(V, ops);<br>+  GetElementPtrInst *GEP = GetElementPtrInst::Create(V.get(), ops);<br>   EXPECT_FALSE(this->clone(GEP)->isInBounds());<br>
<br>   GEP->setIsInBounds();<br>@@ -144,9 +149,10 @@ TEST_F(CloneInstruction, Inbounds) {<br> }<br><br> TEST_F(CloneInstruction, Exact) {<br>-  V = new Argument(Type::getInt32Ty(context));<br>+  V = make_unique<Argument>(Type::getInt32Ty(context));<br>
<br>-  BinaryOperator *SDiv = BinaryOperator::Create(Instruction::SDiv, V, V);<br>+  BinaryOperator *SDiv =<br>+      BinaryOperator::Create(Instruction::SDiv, V.get(), V.get());<br>   EXPECT_FALSE(this->clone(SDiv)->isExact());<br>
<br>   SDiv->setIsExact(true);<br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div></blockquote></div>