It may be possible to do this with the current API, but what I was looking for is a common API for existing block types. For example there is no succ_begin for Machine BasicBlock.
I’m looking to make the CFGSuccessors and CFGPredecessors classes in CFGDiff.h templated, and this needs a common API for all types instantiations.
Does this clarify your question or did I misunderstand your suggestion?
Possibly some misunderstanding - sounds like Nicolai had the same suggestion phrased more clearly.
Yes, I considered declaring the “global” succ_begin for the other block types, but it seems like a more complex change (probably to declare the type as Dave described, and these need adding for 3 more types vs 2 now) and these wouldn’t be used anywhere else.
What do you mean by “adding 3 more types vs 2 now” - where the iterator types are written is pretty separate from this API change (even if the iterator types remain non-members - the succ_begin non-member → member functions with a member type could be achieved by using a member typedef rather than a member /type/ )
I meant adding the global APIs for MachineBasicBlock, VPBlockBase, clang::CFGBlock (3), vs adding member APIs in BasicBlock and VPBlockBase (2).
This is only because the use case I am looking at involves the DominatorTree and there are no instantiations around other types (like you mentioned: Interval).
Looking closer at the comments for Interval, I see that the global APIs were added to mirror the BasicBlock ones, and they’re essentially wrappers over iterators expressed as class members.
So currently we have:
(& some kind of Region thing in RegionIterator.h)
and you’d like to generalize this over more types, MachineBasicBlocks, VPBlockBase and clang::CFGBlock?
So the suggestion would be to add:
what would be the negative side of adding that, rather than porting the extra 3 to member functions?
I don’t think there is any negative, tbh. I simply found it clearer to reason about these as class member methods, and, looking at the existing cases, the “global” API approach looked to me to be the outlier.
I also found somewhat confusing that, for Instructions, there are defined successor iterators via the global APIs but not predecessors, and these are in a separate file (CFG.h) so it’s not as obvious this is the case. If these were class iterators, this would stand out.
Thinking more, I think your suggestion to add the global ones is easier to implement as as quick solution:
just like you suggested, as these are just wrappers over the class member methods (like those defined for Interval).
If folks feel this is a cleaner/better approach, I’m happy to work towards that.
The member methods still seem cleaner to me, but I realize I’ll have to sign up for changing the whole code-base to use that if going that route :-).
Again, as long as the end result is consistent, I’m flexible to go either way.