This way you can dataLog(anIntRect, someFloatPoint, "\n") and all will be well. Will save (Simon and I at least) lots of typing.
Created attachment 208452 [details] take one I'm somewhat tempted to put them all in one big implementation file (GeometryFormatters? I have no idea) instead of spreading datalog stuff throughout all these classes (especially because of IntPoint and IntSize lacking implementation files causing PrintStream.h inclusion in their headers -- but maybe PrintStream.h doesn't change often and that won't be a problem?).
Comment on attachment 208452 [details] take one What’s the rationale for using member functions for these? Easier to invoke from the debugger?
(In reply to comment #1) > especially because of IntPoint and IntSize lacking implementation files causing PrintStream.h inclusion in their headers Make IntPoint.cpp and IntSize.cpp instead; what’s the argument against that?
(In reply to comment #3) > (In reply to comment #1) > > especially because of IntPoint and IntSize lacking implementation files causing PrintStream.h inclusion in their headers > > Make IntPoint.cpp and IntSize.cpp instead; what’s the argument against that? Valid point :) (In reply to comment #2) > (From update of attachment 208452 [details]) > What’s the rationale for using member functions for these? Easier to invoke from the debugger? Is there a better way? It looked to me like DataLog falls back on calling T::dump() when it doesn't know about the type. Maybe pizlo can comment.
(In reply to comment #2) > (From update of attachment 208452 [details]) > What’s the rationale for using member functions for these? Easier to invoke from the debugger? Also, you don't invoke these manually; they get invoked via dataLog()
(In reply to comment #4) > (In reply to comment #2) > > What’s the rationale for using member functions for these? Easier to invoke from the debugger? > > Is there a better way? It looked to me like DataLog falls back on calling T::dump() when it doesn't know about the type. Maybe pizlo can comment. We should fix DataLog so it calls dump(const X&), which in turn can be a template that calls X::dump. That way you can make something dump-able without changing the class itself.
Comment on attachment 208452 [details] take one Attachment 208452 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/1359579
(In reply to comment #6) > (In reply to comment #4) > > (In reply to comment #2) > > > What’s the rationale for using member functions for these? Easier to invoke from the debugger? > > > > Is there a better way? It looked to me like DataLog falls back on calling T::dump() when it doesn't know about the type. Maybe pizlo can comment. > > We should fix DataLog so it calls dump(const X&), which in turn can be a template that calls X::dump. That way you can make something dump-able without changing the class itself. This already works. The canonical way to make something dump able is to add a dump(PrintStream&) method. But you can also add a WTF::printInternal(PrintStream&, const T&) function and get the same effect. In JSC the idiom I ultimately converged to was preferring a dump method over a printInternal function, except in the case of enums (obviously). I don't have a good reason for that preference other than it usually leads to less code, since a function in WTF means opening and then closing that namespace, and having to use whatever namespace T is in (or have a lot of JSC::T like code).
Comment on attachment 208452 [details] take one View in context: https://bugs.webkit.org/attachment.cgi?id=208452&action=review > Source/WebCore/platform/graphics/IntRect.h:211 > + void dump(WTF::PrintStream& out) const; Don't need to say "WTF::"
(In reply to comment #1) > Created an attachment (id=208452) [details] > take one > > I'm somewhat tempted to put them all in one big implementation file (GeometryFormatters? I have no idea) instead of spreading datalog stuff throughout all these classes (especially because of IntPoint and IntSize lacking implementation files causing PrintStream.h inclusion in their headers -- but maybe PrintStream.h doesn't change often and that won't be a problem?). It's already the case that a bunch of JSC and WTF headers include PrintStream.h - and no, it doesn't change often.
http://trac.webkit.org/changeset/153952
I remember seeing objections for such kind of print methods addition to these classes.
(In reply to comment #12) > I remember seeing objections for such kind of print methods addition to these classes. https://bugs.webkit.org/show_bug.cgi?id=79176 - [Bug 79176] Add dump method for WebCore::Int{Point, Rect, Size}
(In reply to comment #13) > (In reply to comment #12) > > I remember seeing objections for such kind of print methods addition to these classes. > > https://bugs.webkit.org/show_bug.cgi?id=79176 - [Bug 79176] Add dump method for WebCore::Int{Point, Rect, Size} Yeah, they've come back up again on #webkit too.