> On Apr 27, 2017, at 12:01 AM, Bob Wilson <[email protected]> wrote: >> On Apr 26, 2017, at 6:43 PM, John McCall via swift-dev <[email protected] >> <mailto:[email protected]>> wrote: >> >>> On Apr 26, 2017, at 6:11 PM, Michael Gottesman <[email protected] >>> <mailto:[email protected]>> wrote: >>>> On Apr 26, 2017, at 1:44 PM, John McCall <[email protected] >>>> <mailto:[email protected]>> wrote: >>>> >>>>> On Apr 26, 2017, at 4:24 PM, Michael Gottesman via swift-dev >>>>> <[email protected] <mailto:[email protected]>> 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. >> >> Uh, sure, but this is also not something most people have to deal with a >> ton, and it's a learn-once-and-remember sort of thing. >> >>>> "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? >> >> A bit elaborate, but it could work. Honestly, this is not a type I have to >> write out much. > > How about just SILEdge?
SILEdge seems a little vague. Also less obvious to someone who isn't thinking of the CFG as primarily a graph rather than a control-flow relationship. But it could work. I agree that I also don't mind just sticking with SILSuccessor. In general, it would be good to see positive support from more than one person for this rename before agreeing to it. John. > > (Honestly I’d prefer to keep it as SILSuccessor and avoid the churn from > this, but if it is important to you, I won’t object to changing it.)
_______________________________________________ swift-dev mailing list [email protected] https://lists.swift.org/mailman/listinfo/swift-dev
