> On Apr 27, 2017, at 12:01 AM, Bob Wilson <bob.wil...@apple.com> wrote:
>> On Apr 26, 2017, at 6:43 PM, John McCall via swift-dev <swift-dev@swift.org 
>> <mailto:swift-dev@swift.org>> wrote:
>> 
>>> On Apr 26, 2017, at 6:11 PM, Michael Gottesman <mgottes...@apple.com 
>>> <mailto:mgottes...@apple.com>> wrote:
>>>> On Apr 26, 2017, at 1:44 PM, John McCall <rjmcc...@apple.com 
>>>> <mailto:rjmcc...@apple.com>> wrote:
>>>> 
>>>>> On Apr 26, 2017, at 4:24 PM, Michael Gottesman via swift-dev 
>>>>> <swift-dev@swift.org <mailto:swift-dev@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.
>> 
>> 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
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

Reply via email to