[PATCH] [C++11] Add predecessors(BasicBlock *) / successors(BasicBlock *) iterator ranges.
David Blaikie
dblaikie at gmail.com
Sat Jul 19 11:14:52 PDT 2014
Generally looks good - some optional comments, but they're potentially orthogonal/follow-up work.
================
Comment at: lib/Analysis/BranchProbabilityInfo.cpp:533
@@ -532,4 +532,3 @@
BI != BE; ++BI) {
- for (succ_const_iterator SI = succ_begin(BI), SE = succ_end(BI);
- SI != SE; ++SI) {
- printEdgeProbability(OS << " ", BI, *SI);
+ for (const BasicBlock *Succ : successors(BI)) {
+ printEdgeProbability(OS << " ", BI, Succ);
----------------
You could drop the {} from this loop (and the outer loop, actually... ), but I don't think it's necessary in this cleanup (and may even be better as a separate commit anyway)
================
Comment at: lib/Analysis/BranchProbabilityInfo.cpp:533
@@ -532,4 +532,3 @@
BI != BE; ++BI) {
- for (succ_const_iterator SI = succ_begin(BI), SE = succ_end(BI);
- SI != SE; ++SI) {
- printEdgeProbability(OS << " ", BI, *SI);
+ for (const BasicBlock *Succ : successors(BI)) {
+ printEdgeProbability(OS << " ", BI, Succ);
----------------
David Blaikie wrote:
> You could drop the {} from this loop (and the outer loop, actually... ), but I don't think it's necessary in this cleanup (and may even be better as a separate commit anyway)
I think we've been pretty forward about using "auto" more aggressively - especially in "obvious" cases, which I think this qualifies (BI might one day be "BB" when that outer loop is changed to a range-based-for, but either name connotes "basic block" pretty pervasively in LLVM - and iterating over the predecessors or successors of a basic block clearly consists of more basic blocks).
So maybe these should be "auto *" and "const auto *" throughout the refactoring.
http://reviews.llvm.org/D4481
More information about the llvm-commits
mailing list