<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Hey, Sergey. It definitely makes sense to refactor this, but I don't think putting methods on SILInstruction is the way to go about it. There's no reason most clients of SIL need to know anything about parsing; from a separation-of-concerns perspective it belongs in a separate header and very likely a separate component (i.e. not lib/SIL/).</div><div class=""><br class=""></div><div class="">If this were Swift, we could use an extension, but alas. :-)</div><div class=""><br class=""></div><div class="">That said, I'm not a heavy user of SIL, so someone else from the Swift team with more of a stake should comment.</div><div class=""><br class=""></div><div class="">Jordan</div><div class=""><br class=""></div><br class=""><div><blockquote type="cite" class=""><div class="">On Jan 13, 2016, at 13:27, Sergey Bolshedvorsky via swift-dev <<a href="mailto:swift-dev@swift.org" class="">swift-dev@swift.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Hi everyone,<br class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><div class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><br class=""></div><div class="">I’m interested in taking <a href="https://bugs.swift.org/browse/SR-340" class="">SR-340</a> issue:</div><div class=""><span style="color: rgb(51, 51, 51); background-color: rgb(255, 255, 255);" class="">"parseSILInstruction is horrible and makes me cry every time I see it. It is a method that is ~1900 lines with a huge switch in it. We should refactor it into a visitor structure. In fact it is large enough that we should consider moving it into its own file if it is possible."</span></div><div class=""><br class=""></div><div class="">Here is the first draft of the myl idea, how it could be approached:</div><div class=""><br class=""></div><div class="">1. We will define an abstract method on ValueBase class:</div><div class=""><br class=""></div><div class=""><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> <span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">class</span> ValueBase</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> {</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(187, 44, 162);" class=""><span style="" class=""> </span>public<span style="" class="">:</span></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> <span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">virtual</span> <span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">void</span> parse(<span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">class</span> SILParseInstruction*) = <span style="font-variant-ligatures: no-common-ligatures; color: #272ad8" class="">0</span>;</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> };</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""> <br class="webkit-block-placeholder"></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""><br class=""></div><div style="margin: 0px; line-height: normal; min-height: 13px;" class="">2. Each of the SIL instruction classes will override this method and will provide the following implementation:</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""> <br class="webkit-block-placeholder"></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> <span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">class</span> SILArgument: <span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">public</span> <span style="font-variant-ligatures: no-common-ligatures; color: #4f8187" class="">ValueBase</span></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> {</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(187, 44, 162);" class=""><span style="" class=""> </span>public<span style="" class="">:</span></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(79, 129, 135);" class=""><span style="" class=""> </span><span style="font-variant-ligatures: no-common-ligatures; color: #008400" class="">/*virtual*/</span><span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">void</span><span style="" class=""> parse(</span>SILParseInstruction<span style="" class="">*);</span></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> };</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(79, 129, 135);" class=""><span style="" class=""> </span><span style="color: rgb(187, 44, 162);" class="">void</span><span style="" class=""> </span>SILArgument<span style="" class="">::parse(</span>SILParseInstruction<span style="" class=""> *i) {</span></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(49, 89, 93);" class=""><span style="" class=""> i-></span>parseInstruction<span style="" class="">(</span><span style="color: rgb(187, 44, 162);" class="">this</span><span style="" class="">);</span></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> }</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""> <br class="webkit-block-placeholder"></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> <span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">class</span> PartialApplyInst: <span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">public</span> <span style="font-variant-ligatures: no-common-ligatures; color: #4f8187" class="">ValueBase</span></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> {</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(187, 44, 162);" class=""><span style="" class=""> </span>public<span style="" class="">:</span></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(79, 129, 135);" class=""><span style="" class=""> </span><span style="font-variant-ligatures: no-common-ligatures; color: #008400" class="">/*virtual*/</span><span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">void</span><span style="" class=""> parse(</span>SILParseInstruction<span style="" class="">*);</span></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> };</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(79, 129, 135);" class=""><span style="" class=""> </span><span style="color: rgb(187, 44, 162);" class="">void</span><span style="" class=""> </span>PartialApplyInst<span style="" class="">::parse(</span>SILParseInstruction<span style="" class=""> *i)</span></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> {</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(49, 89, 93);" class=""><span style="" class=""> i-></span>parseInstruction<span style="" class="">(</span><span style="color: rgb(187, 44, 162);" class="">this</span><span style="" class="">);</span></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> }</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""> <br class="webkit-block-placeholder"></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""><br class=""></div><div style="margin: 0px; line-height: normal; min-height: 13px;" class="">3. We will define an abstract class for the callbacks from each of the SIL instruction classes</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""><br class=""></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> <span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">class</span> SILParseInstruction</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> {</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(187, 44, 162);" class=""><span style="" class=""> </span>public<span style="" class="">:</span></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> <span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">virtual</span> <span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">void</span> parseInstruction(<span style="font-variant-ligatures: no-common-ligatures; color: #4f8187" class="">SILArgument</span>*) = <span style="font-variant-ligatures: no-common-ligatures; color: #272ad8" class="">0</span>;</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> <span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">virtual</span> <span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">void</span> parseInstruction(<span style="font-variant-ligatures: no-common-ligatures; color: #4f8187" class="">PartialApplyInst</span>*) = <span style="font-variant-ligatures: no-common-ligatures; color: #272ad8" class="">0</span>;</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> };</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""><br class=""></div><div style="margin: 0px; line-height: normal;" class="">4. SILParser will implement these callbacks with the actual handling operations for each instruction. </div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""> <br class="webkit-block-placeholder"></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(79, 129, 135);" class=""><span style="" class=""> </span><span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">class</span><span style="" class=""> SILParser: </span><span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">public</span><span style="" class=""> </span>SILParseInstruction</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> {</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(187, 44, 162);" class=""><span style="" class=""> </span>public<span style="" class="">:</span></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> <span style="font-variant-ligatures: no-common-ligatures; color: #008400" class="">/*virtual*/</span><span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">void</span> parseInstruction(<span style="font-variant-ligatures: no-common-ligatures; color: #4f8187" class="">SILArgument</span> *r)</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> {</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 132, 0);" class=""><span style="" class=""> </span>// Parse StringLiteralInst instruction</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> }</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> <span style="font-variant-ligatures: no-common-ligatures; color: #008400" class="">/*virtual*/</span><span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">void</span> parseInstruction(<span style="font-variant-ligatures: no-common-ligatures; color: #4f8187" class="">PartialApplyInst</span> *b)</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> {</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 132, 0);" class=""><span style="" class=""> </span>// Parse PartialApplyInst instruction</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> }</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> };</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""> <br class="webkit-block-placeholder"></div><div style="margin: 0px; line-height: normal;" class="">5. The huge switch statement will be replaced with a single call:</div></div><div class=""><span class="Apple-tab-span" style="white-space:pre"> </span><font face="Menlo" style="font-size: 11px;" class="">Opcode->parse(this);</font></div><div class=""><br class=""></div><div class="">What are your thoughts?</div><div class=""><br class=""></div><div class="">Sergey</div></div></div>
<img src="https://u2002410.ct.sendgrid.net/wf/open?upn=RoDF4MveSEMYBIqIJA6ub1g8cOZ-2BVYvqV-2FqygPhjPn8DXSAHdGimVwCJdbdajCDyeSHtd1ByAZl9RxQ3p5kmTiN0lndQaLVlU0eSFKtWbFhj0BLZkW0xZ492vEFobaMeldZV5xGrLzj6Wqi0brJQrhh7AZ7lmYe0iqPgl8OglUERL5AG4vFwiWcljlYj4TP-2Bm5JDS7ARfJWFqs6aMi2DzdpWU1SBxyR-2BfhDEa26NOjY-3D" alt="" width="1" height="1" border="0" style="height:1px !important;width:1px !important;border-width:0 !important;margin-top:0 !important;margin-bottom:0 !important;margin-right:0 !important;margin-left:0 !important;padding-top:0 !important;padding-bottom:0 !important;padding-right:0 !important;padding-left:0 !important;" class="">
</div>
_______________________________________________<br class="">swift-evolution mailing list<br class=""><a href="mailto:swift-evolution@swift.org" class="">swift-evolution@swift.org</a><br class=""><a href="https://lists.swift.org/mailman/listinfo/swift-evolution" class="">https://lists.swift.org/mailman/listinfo/swift-evolution</a><br class=""></div></div><br class=""></div></div></div></div><br class="">
<img src="https://u2002410.ct.sendgrid.net/wf/open?upn=ZEz4qHYnXhPr3bBPu-2FxP4tN3HfWKL-2FtJpqkQ0gkOVSCa9a3wcdHPx5s4BGRtaJYucQOD6Eztozop9rdbMmO3mkNsdh34awlZXmqq-2BhV-2Feft4xK0M3ZTBDtMiq58x3wq2JD3hd2m7GtMywkdMJ-2FZDDcaAK9zVLHBUvfx34g2HXm5tAOMzQ344sWrYU-2BtrZ2esTn91T-2FHrc3YF6Q7cyoqTbHd9FRpexg5fh59xuhiR4Zk-3D" alt="" width="1" height="1" border="0" style="height:1px !important;width:1px !important;border-width:0 !important;margin-top:0 !important;margin-bottom:0 !important;margin-right:0 !important;margin-left:0 !important;padding-top:0 !important;padding-bottom:0 !important;padding-right:0 !important;padding-left:0 !important;" class="">
</div>
_______________________________________________<br class="">swift-dev mailing list<br class=""><a href="mailto:swift-dev@swift.org" class="">swift-dev@swift.org</a><br class="">https://lists.swift.org/mailman/listinfo/swift-dev<br class=""></div></blockquote></div><br class=""></body></html>