[PATCH] D40837: [FuzzMutate] Allow only sized pointers for the GEP instruction

Igor Laevsky via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 6 03:06:56 PST 2017


igor-laevsky added inline comments.


================
Comment at: unittests/FuzzMutate/OperationsTest.cpp:57-59
+namespace {
+std::unique_ptr<Module> parseAssembly(
+    const char *Assembly, LLVMContext &Context) {
----------------
bogner wrote:
> I don't know gtest all that well, but is putting all of this in an anonymous namespace the right thing to do here? Why not just make parseAssembly static?
I see anonymous namespace in the official gtest examples and in many llvm test sets. So I guess it's the right thing to do.


================
Comment at: unittests/FuzzMutate/OperationsTest.cpp:179-180
   Module M("M", Ctx);
   Function *F = Function::Create(FunctionType::get(Type::getVoidTy(Ctx), {},
-                                                   /*isVarArg=*/false),
+                                     /*isVarArg=*/false),
                                  GlobalValue::ExternalLinkage, "f", &M);
----------------
bogner wrote:
> Apparently phabricator only shows the diff here in the email and not the we interface, but what's with the arbitrary whitespace changes  on these Function::Create calls? According to clang-format, the old way is correct.
Hm, didn't notice those. Will run clang format, thanks.


================
Comment at: unittests/FuzzMutate/OperationsTest.cpp:294-298
+  // Don't match %v
+  ASSERT_FALSE(Descr.SourcePreds[0].matches({}, &*BB.begin()));
+
+  // Match %a
+  ASSERT_TRUE(Descr.SourcePreds[0].matches({}, &*std::next(BB.begin())));
----------------
bogner wrote:
> I feel like these would be easier to understand / wouldn't need the comments if we create the instructions programmatically like in the other tests in this file rather than parsing it.
I tried this way but it seemed harder to read, harder to write and more error prone. After all we want to generate llvm ir code and there is no more natural way than to write it directly.


https://reviews.llvm.org/D40837





More information about the llvm-commits mailing list