[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