Fix broken C++ in PODInterval and PODIntervalTree
Created attachment 69592 [details] Patch
kbr: Please review. evan, hans: fyi
Attachment 69592 [details] did not build on chromium: Build output: http://queues.webkit.org/results/4162058
dglazkov: Which gcc version is cr-linux running? Builds fine for me locally with 4.0.1 and 4.2.1.
Turns out function templates can't be partially specialized. Please refrain from reviewing…
Thinking about this, the options seem to be: 1. Convert the function template into a class template, which can then use partial specialization like this: template <class T> struct ValueToString { static String value(const T& t); }; template <> struct<int> ValueToString { static String value(const int& t) { return intToStr(t); } }; etc. 2. Use argument-dependent lookup for the function. ADL only works for class types, so I'd need to declare the function for all basic types, and could then use ADL for the rest. 3. Just delete this code. It's only used for debugging and probably isn't worth all the complexity. I think we should go with 3. Comments?
(In reply to comment #6) > Thinking about this, the options seem to be: > > 1. Convert the function template into a class template, which can then use partial specialization like this: > template <class T> > struct ValueToString { > static String value(const T& t); > }; > template <> > struct<int> ValueToString { > static String value(const int& t) { return intToStr(t); } > }; > > etc. > > > 2. Use argument-dependent lookup for the function. ADL only works for class types, so I'd need to declare the function for all basic types, and could then use ADL for the rest. > > > 3. Just delete this code. It's only used for debugging and probably isn't worth all the complexity. > > > I think we should go with 3. Comments? I do not support simply deleting this code. It has occasionally proven essential for debugging purposes. The original code used iostreams and overloaded operator<< to provide printing functionality. Either option (1) or (2) sounds fine. I'm happy to make the change to (1) if you want.
Created attachment 69644 [details] Patch
Approach 1 it is.
Attachment 69644 [details] did not build on chromium: Build output: http://queues.webkit.org/results/4173065
Created attachment 69664 [details] Patch
Comment on attachment 69664 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=69664&action=review Code changes look fine; please note minor documentation changes needed. > WebCore/platform/graphics/gpu/PODInterval.h:64 > +// String string(const T& t); This should be documented as being static, as is used in the code. > WebCore/platform/graphics/gpu/PODInterval.h:67 > +// String string(const UserData& t); static. > WebCore/platform/graphics/gpu/PODRedBlackTree.h:48 > +// String string(const T& t); static.
Created attachment 69677 [details] Patch
Comment on attachment 69677 [details] Patch Looks fine. I think you need to wait for the r+ before marking it cq+, though.
Comment on attachment 69677 [details] Patch Clearing flags on attachment: 69677 Committed r69081: <http://trac.webkit.org/changeset/69081>
All reviewed patches have been landed. Closing bug.