[swift-evolution] [Draft Proposal] Refactor SILParser::parseSILInstruction

[Sergey Bolshedvorsky]

Hi everyone,

I’m interested in taking SR-340 <https://bugs.swift.org/browse/SR-340> issue:
"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."

Here is the first draft of the myl idea, how it could be approached:

1. We will define an abstract method on ValueBase class:

    class ValueBase
    {
    public:
        virtual void parse(class SILParseInstruction*) = 0;
    };
    

2. Each of the SIL instruction classes will override this method and will provide the following implementation:
    
    class SILArgument: public ValueBase
    {
    public:
        /*virtual*/void parse(SILParseInstruction*);
    };
    void SILArgument::parse(SILParseInstruction *i) {
        i->parseInstruction(this);
    }
    
    class PartialApplyInst: public ValueBase
    {
    public:
        /*virtual*/void parse(SILParseInstruction*);
    };
    void PartialApplyInst::parse(SILParseInstruction *i)
    {
        i->parseInstruction(this);
    }
    

3. We will define an abstract class for the callbacks from each of the SIL instruction classes

    class SILParseInstruction
    {
    public:
        virtual void parseInstruction(SILArgument*) = 0;
        virtual void parseInstruction(PartialApplyInst*) = 0;
    };

4. SILParser will implement these callbacks with the actual handling operations for each instruction. 
    
    class SILParser: public SILParseInstruction
    {
    public:
        /*virtual*/void parseInstruction(SILArgument *r)
        {
            // Parse StringLiteralInst instruction
        }
        /*virtual*/void parseInstruction(PartialApplyInst *b)
        {
            // Parse PartialApplyInst instruction
        }
    };
    
5. The huge switch statement will be replaced with a single call:
	Opcode->parse(this);

What are your thoughts?

Sergey
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20160113/daf0caaf/attachment.html>