[PATCH] Add some GlobalList unit tests.

Peter Collingbourne peter at pcc.me.uk
Mon Jul 8 16:24:07 PDT 2013



================
Comment at: unittests/Transforms/Utils/GlobalList.cpp:50
@@ +49,3 @@
+  LLVMContext Ctx;
+  Module *M;
+  GlobalList *GL;
----------------
Alexey Samsonov wrote:
> Why do you need state here? I think creating the modules and blacklists in the test cases will make the code more straightforward and readable.
There are arguments for doing it either way, but I agree with you now; changed.

================
Comment at: unittests/Transforms/Utils/GlobalList.cpp:62
@@ +61,3 @@
+                 "src:hello\n");
+  EXPECT_TRUE(GL->isIn(*M));
+  EXPECT_TRUE(GL->isIn(*F));
----------------
Alexey Samsonov wrote:
> E.g. this case would look like:
> Module M("hello", Ctx);
> OwningPtr<Function> F(makeFunction("foo", &M));
> OwningPtr<GlobalVariable> GV(makeGlobal("bar", "type", &M));
> OwningPtr<BlackList> BL(makeBlackList("..."));
> 
> EXPECT_TRUE(BL->isIn(M));
> ...
> BL.reset(makeBlackList("..."));
> ...
> 
> BTW "Function *F" and "GlobalVariable *GV" are leaked in the current version.
I believe they are owned by the module, which is deleted.

================
Comment at: unittests/Transforms/Utils/GlobalList.cpp:37
@@ +36,3 @@
+  GlobalVariable *makeGlobal(StringRef Name, StringRef StructName) {
+    StructType* ST =
+        StructType::create(StructName, Type::getInt32Ty(Ctx), (Type*)0);
----------------
Alexey Samsonov wrote:
> Is it leaked?
I don't think so, types are owned by the LLVMContext.


http://llvm-reviews.chandlerc.com/D1091



More information about the llvm-commits mailing list