<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body dir="auto"><div><div id="AppleMailSignature">Michael,</div><div id="AppleMailSignature"><br></div><div id="AppleMailSignature">The simplest way is to keep the switch statement in the parseSILInstruction method and move all logic into parser routines. Is it right?</div><div id="AppleMailSignature"><br></div><div id="AppleMailSignature">bool SILParser::parseSILInstruction(SILBasicBlock *BB) {</div><div id="AppleMailSignature"> // Header</div><div id="AppleMailSignature"><br></div><div id="AppleMailSignature"> switch (Opcode) {</div><div id="AppleMailSignature"> case ValueKind::SILArgument: </div><div id="AppleMailSignature"> return handleValueKindSILArgument(Opcode); </div><div id="AppleMailSignature"> break;</div><div id="AppleMailSignature"> case ValueKind::AllocBoxInst: </div><div id="AppleMailSignature"> return handleValueKindAllocBoxInst(Opcode);</div><div id="AppleMailSignature"> break;</div><div id="AppleMailSignature"> case ValueKind::ApplyInst:</div><div id="AppleMailSignature"> return handleValueKindApplyInst(Opcode);</div><div id="AppleMailSignature"> break;</div><div id="AppleMailSignature"> // ...</div><div id="AppleMailSignature"> }</div><div id="AppleMailSignature">}</div><div id="AppleMailSignature"><br></div><div id="AppleMailSignature">SILParser::handleValueKindSILArgument(ValueKind Opcode) {</div><div id="AppleMailSignature"> // Handle ValueKind::SILArgument case here</div><div id="AppleMailSignature">}</div><div id="AppleMailSignature"><br></div><div id="AppleMailSignature">SILParser::handleValueKindAllocBoxInst(ValueKind Opcode) {</div><div id="AppleMailSignature"> // Handle ValueKind::AllocBoxInst case here</div><div id="AppleMailSignature">}</div><div id="AppleMailSignature"><br></div><div id="AppleMailSignature">SILParser::handleValueKindApplyInst(ValueKind Opcode) {</div><div id="AppleMailSignature"> // Handle ValueKind::ApplyInst case here </div><div id="AppleMailSignature">}</div><div id="AppleMailSignature"><br></div><div id="AppleMailSignature">By the way, I’ve noticed that some instructions are getting parsed by parser routines already: parseSILFunctionRef or parseCallInstruction.</div><div id="AppleMailSignature"><br></div><div id="AppleMailSignature">Sergey</div><br>Sent from my iPhone</div><div><br>On 14 Jan 2016, at 06:00, Michael Gottesman via swift-dev <<a href="mailto:swift-dev@swift.org">swift-dev@swift.org</a>> wrote:<br><br></div><blockquote type="cite"><div><meta http-equiv="Content-Type" content="text/html charset=utf-8"><br class=""><div><blockquote type="cite" class=""><div class="">On Jan 13, 2016, at 9:08 PM, Michael Gottesman 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=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class="Apple-interchange-newline">On Jan 13, 2016, at 3:35 PM, John McCall via swift-dev <<a href="mailto:swift-dev@swift.org" class="">swift-dev@swift.org</a>> wrote:<br class=""><br class=""><blockquote type="cite" class="">On Jan 13, 2016, at 3:25 PM, Jordan Rose via swift-dev <<a href="mailto:swift-dev@swift.org" class="">swift-dev@swift.org</a>> wrote:<br 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/).<br class=""><br class="">If this were Swift, we could use an extension, but alas. :-)<br class=""><br 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.<br class=""></blockquote><br class="">I agree with you completely.<br class=""><br class="">Also, the SIL instruction parser does not actually have an instance of an instruction to perform virtual dispatch on. Nor should we introduce the concept of “wireframe” instructions just for the convenience of the parser. Redundancy between cases here should be addressed with normal redundancy elimination techniques, i.e. macros and templates.<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">The way to do this is to have a visitor parser that takes in a ValueKind and maps it to the parser routine to use.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div><br class=""></div><div>Let me rephrase. IIRC the way that this code is written is it first deserializes the value kind. Imagine if we had a visitor that was composed with the parser whose visitor methods would perform the relevant parsing.</div><br class=""><blockquote type="cite" class=""><div class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Michael</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class="">John.<br class="">_______________________________________________<br class="">swift-dev mailing list<br class=""><a href="mailto:swift-dev@swift.org" class="">swift-dev@swift.org</a><br class=""><a href="https://lists.swift.org/mailman/listinfo/swift-dev" class="">https://lists.swift.org/mailman/listinfo/swift-dev</a><br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">_______________________________________________</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">swift-dev mailing list</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><a href="mailto:swift-dev@swift.org" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">swift-dev@swift.org</a><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><a href="https://lists.swift.org/mailman/listinfo/swift-dev" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">https://lists.swift.org/mailman/listinfo/swift-dev</a></div></blockquote></div><br class="">
<img src="https://u2002410.ct.sendgrid.net/wf/open?upn=hmIxNMeUeKq1PNSKRTOoag-2BAEfT4iqdAHk7XD4YDHG6eTsM31HYaPE6C6WaYV5XF32DQ3MHVaDvNKugsnhDiODx4BlwkzdGrpOEuIEktokN8NyYNQhhXFxcG6UqqsS2OeLe22IlstjV5YxUPGT4UcvqUq4prklmJCA8MV0qylTQspgNOgv69paGtKOmClTeYU804T-2B-2B09tfXaR1RHX4N9qTu3DO12A2uN2-2Fk2hWupoM-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;">
</div></blockquote><blockquote type="cite"><div><span>_______________________________________________</span><br><span>swift-dev mailing list</span><br><span><a href="mailto:swift-dev@swift.org">swift-dev@swift.org</a></span><br><span><a href="https://lists.swift.org/mailman/listinfo/swift-dev">https://lists.swift.org/mailman/listinfo/swift-dev</a></span><br></div></blockquote></body></html>