Bug 171280 – Make DFG SpeculatedType dumps easier to read.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

Adding a pretty printer to insert |s between each type and changing the dumped strings to match the SpeculatedType names case-wise.

[Mark Lam]

Created attachment 308114 [details]
proposed patch.

[Saam Barati]

Comment on attachment 308114 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=308114&action=review

> Source/JavaScriptCore/bytecode/SpeculatedType.cpp:63
> +struct PrettyPrinter {
> +    PrettyPrinter(PrintStream& out)
> +        : out(out)
> +    { }
> +    
> +    template<typename T>
> +    void print(const T& value)
> +    {
> +        if (!isFirst)
> +            out.print("|");
> +        out.print(value);
> +        isFirst = false;
> +    }
> +    
> +    PrintStream& out;
> +    bool isFirst { true };
> +};

I think you can just use CommaPrinter with "|" as the input.

[Mark Lam]

Comment on attachment 308114 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=308114&action=review

>> Source/JavaScriptCore/bytecode/SpeculatedType.cpp:63
>> +};
> 
> I think you can just use CommaPrinter with "|" as the input.

I can change PrettyPrinter to use CommaPrinter to print the |, but I'll still need the PrettyPrinter.  Note that dumpSpeculation() has 2 streams: the real output PrintStream, and a StringPrintStream.  That means I need to keep 2 CommaPrinter instances: one for each of the streams.  Using the PrinterPrinter makes it easier to track which CommaPrinter instance to use.

[Mark Lam]

Created attachment 308116 [details]
patch for landing.

[Mark Lam]

Thanks for the review.  Landed in r215747: <http://trac.webkit.org/r215747>.