This patch introduces two new iterator ranges and updates existing code to use it. No functional change intended.
Details
Diff Detail
Event Timeline
Generally looks good - some optional comments, but they're potentially orthogonal/follow-up work.
lib/Analysis/BranchProbabilityInfo.cpp | ||
---|---|---|
533–534 | 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) | |
533–534 | 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. |
lib/Analysis/BranchProbabilityInfo.cpp | ||
---|---|---|
533–534 | Yes, I'll do these changes (rangify outer loop, remove braces) after committing this one. I'm not sure if it's better to use "auto" here -- it doesn't increase readability. I don't think "BasicBlock" is a very cumbersome name. "Succ" is a general name that could also mean "successor instruction" for example. |
LGTM
(In follow up many of these could be std::one_of, std::any_of, none_of, etc - though we could add llvm versions of those that work on ranges directly (rather than iterator begin/end))
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)