[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