[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