Bug 103584

Summary: SpeculatedType dumping should not use the static char buffer[thingy] idiom
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, benjamin, dglazkov, ggaren, gyuyoung.kim, mark.lam, mhahnenberg, msaboff, ojan, oliver, philn, rakuco, sam, webkit-ews, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 103586    
Attachments:
Description Flags
the patch
msaboff: review+, webkit-ews: commit-queue-
patch for landing
webkit-ews: commit-queue-
patch for landing none

Description Filip Pizlo 2012-11-28 17:43:08 PST
Instead it should have an appropriate dump(PrintStream&) method.
Comment 1 Filip Pizlo 2012-11-28 18:02:16 PST
Created attachment 176619 [details]
the patch

I still need to play with the other build systems, but other than that it's ready to review.
Comment 2 Early Warning System Bot 2012-11-28 18:11:07 PST
Comment on attachment 176619 [details]
the patch

Attachment 176619 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15010732
Comment 3 Early Warning System Bot 2012-11-28 18:13:38 PST
Comment on attachment 176619 [details]
the patch

Attachment 176619 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15015675
Comment 4 Michael Saboff 2012-11-28 18:20:11 PST
Comment on attachment 176619 [details]
the patch

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

r+.  Please update the change logs removing the extraneous namespace entries.

> Source/JavaScriptCore/ChangeLog:18
> +        (JSC):

Remove the (JSC): only lines that the tools shouldn't put in.

> Source/JavaScriptCore/ChangeLog:26
> +        (WTF):

Same here.

> Source/JavaScriptCore/ChangeLog:34
> +        (DFG):

Ditto.

> Source/WTF/ChangeLog:13
> +        (WTF):

Ditto.
Comment 5 Filip Pizlo 2012-11-28 18:34:52 PST
Created attachment 176623 [details]
patch for landing

I will wait for EWS to land because I'm worried about vsnprintf and va_copy not being available on some platforms.
Comment 6 Early Warning System Bot 2012-11-28 18:49:14 PST
Comment on attachment 176623 [details]
patch for landing

Attachment 176623 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15044095
Comment 7 Early Warning System Bot 2012-11-28 18:51:26 PST
Comment on attachment 176623 [details]
patch for landing

Attachment 176623 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15029586
Comment 8 WebKit Review Bot 2012-11-28 18:53:46 PST
Comment on attachment 176623 [details]
patch for landing

Attachment 176623 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15023643
Comment 9 Build Bot 2012-11-28 19:26:52 PST
Comment on attachment 176623 [details]
patch for landing

Attachment 176623 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15018673
Comment 10 Filip Pizlo 2012-11-28 20:27:53 PST
Created attachment 176632 [details]
patch for landing

Trying again
Comment 11 Filip Pizlo 2012-11-28 22:03:02 PST
Landed in http://trac.webkit.org/changeset/136096