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

[Justin Bogner via Phabricator via llvm-commits]

bogner accepted this revision.
bogner added inline comments.
This revision is now accepted and ready to land.


================
Comment at: unittests/FuzzMutate/OperationsTest.cpp:57-59
+namespace {
+std::unique_ptr<Module> parseAssembly(
+    const char *Assembly, LLVMContext &Context) {
----------------
igor-laevsky wrote:
> 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.
Works for me.


================
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())));
----------------
igor-laevsky wrote:
> 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.
I'm not totally convinced. Harder to write maybe, but at least in concert with figuring out what the checks are doing I feel it's easier to read and less error prone. It's not a big deal though, both ways are clear enough.


https://reviews.llvm.org/D40837