Bug 171280 - Make DFG SpeculatedType dumps easier to read.
Summary: Make DFG SpeculatedType dumps easier to read.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-25 10:31 PDT by Mark Lam
Modified: 2017-04-25 11:46 PDT (History)
4 users (show)

See Also:


Attachments
proposed patch. (10.61 KB, patch)
2017-04-25 10:35 PDT, Mark Lam
saam: review+
Details | Formatted Diff | Diff
patch for landing. (10.54 KB, patch)
2017-04-25 10:59 PDT, Mark Lam
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2017-04-25 10:31:16 PDT
Adding a pretty printer to insert |s between each type and changing the dumped strings to match the SpeculatedType names case-wise.
Comment 1 Mark Lam 2017-04-25 10:35:39 PDT
Created attachment 308114 [details]
proposed patch.
Comment 2 Saam Barati 2017-04-25 10:41:54 PDT
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.
Comment 3 Mark Lam 2017-04-25 10:48:05 PDT
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.
Comment 4 Mark Lam 2017-04-25 10:59:43 PDT
Created attachment 308116 [details]
patch for landing.
Comment 5 Mark Lam 2017-04-25 11:46:21 PDT
Thanks for the review.  Landed in r215747: <http://trac.webkit.org/r215747>.