[swift-dev] Renaming SILSuccessor -> SILCFGEdge

Bob Wilson bob.wilson at apple.com
Wed Apr 26 18:20:21 CDT 2017


> On Apr 26, 2017, at 3:11 PM, Michael Gottesman via swift-dev <swift-dev at swift.org> wrote:
> 
> 
>> On Apr 26, 2017, at 1:44 PM, John McCall <rjmccall at apple.com> wrote:
>> 
>>> On Apr 26, 2017, at 4:24 PM, Michael Gottesman via swift-dev <swift-dev at swift.org> wrote:
>>> Hey everyone.
>>> 
>>> I am currently doing some small fixes to SILSuccessor (adding some comments and fixing some issues exposed by LLVM upstream). As I read the code it became pretty apparent that the name is a misnomer... SILSuccessor is not just representing a successor, rather it is representing a whole CFG edge. This can be seen in how SILSuccessor is used to iterate over the predecessors of the block.
>>> 
>>> With that in mind, I would like to rename SILSuccessor to SILCFGEdge. It will make it much clearer without knowing any context what this data structure is used for.
>>> 
>>> Any objections, disagreements, flames, etc?
>> 
>> It seems a little unnecessary to me.  The successor relationship is an edge, and all the edges of the local CFG are successor relationships.  I guess it looks a little funny that the edges into a block are represented by "successors", but I think when you think about it it makes sense.
> 
> IMO this is more of an issue than something "looking funny". Using code named "successor" to look up the "predecessors" of a block is misleading and results in unnecessary cognitive overhead for the reader who has to "think about it" for it to make sense.
> 
>> 
>> "SILCFGEdge" is also not a very attractive name because of the two abbreviations.  If you had a nice alternative to "CFGEdge" that was less biased to the beginning/end (like Successor/Predecessor are), I probably wouldn't object.
> 
> Ok. Maybe SILControlFlowEdge?

I can see your point here, but a big renaming change like this is disruptive. With all the other things we’re trying to get done now, is this really the right time to pay the cost of that?


More information about the swift-dev mailing list