[PATCH] D33617: Rearrange Dom unittest to accommodate multiple tests

Sanjoy Das via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 26 18:43:04 PDT 2017


sanjoy accepted this revision.
sanjoy added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: unittests/IR/DominatorTreeTest.cpp:32
+  DominatorTree DT(*F);
+  DominatorTreeBase<BasicBlock> PDT(true);
+  PDT.recalculate(*F);
----------------
I'd write this as `DominatorTreeBase<BasicBlock> PDT(/*isPostDom*/true)`


================
Comment at: unittests/IR/DominatorTreeTest.cpp:39
+                                              const char *ModuleStr) {
+  SMDiagnostic Err;
+  return parseAssemblyString(ModuleStr, Err, Context);
----------------
I'd either `assert` or `ASSERT` that the module was successfully parsed.  (This part's optional) I'd also prefer `StringRef` as an argument, and also in the declaration of `ModuleString`.


https://reviews.llvm.org/D33617





More information about the llvm-commits mailing list