[PATCH] Add some GlobalList unit tests.
Alexey Samsonov
samsonov at google.com
Thu Jul 4 05:31:16 PDT 2013
Thanks for working on this!
================
Comment at: unittests/Transforms/Utils/GlobalList.cpp:50
@@ +49,3 @@
+ LLVMContext Ctx;
+ Module *M;
+ GlobalList *GL;
----------------
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.
================
Comment at: unittests/Transforms/Utils/GlobalList.cpp:62
@@ +61,3 @@
+ "src:hello\n");
+ EXPECT_TRUE(GL->isIn(*M));
+ EXPECT_TRUE(GL->isIn(*F));
----------------
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.
================
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);
----------------
Is it leaked?
http://llvm-reviews.chandlerc.com/D1091
More information about the llvm-commits
mailing list