Summary: | Remove the Vector::append overload that takes a Vector | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Anders Carlsson <andersca> | ||||||||||||||||||||||||||||||||
Component: | New Bugs | Assignee: | Anders Carlsson <andersca> | ||||||||||||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||||||||
Severity: | Normal | CC: | benjamin, darin, eflews.bot, gtk-ews, gyuyoung.kim, philn, rego+ews, webkit-ews, xan.lopez | ||||||||||||||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||||||||||||
Attachments: |
|
Description
Anders Carlsson
2013-05-02 16:31:33 PDT
Created attachment 200368 [details]
Patch
Created attachment 200370 [details]
Patch
Comment on attachment 200370 [details] Patch Attachment 200370 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/393266 Comment on attachment 200370 [details] Patch Attachment 200370 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/393265 Comment on attachment 200370 [details] Patch Attachment 200370 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/393269 Comment on attachment 200370 [details] Patch Attachment 200370 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/384287 Created attachment 200374 [details]
Patch
Created attachment 200375 [details]
Patch
Created attachment 200377 [details]
Patch
Comment on attachment 200377 [details] Patch Attachment 200377 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/332288 Comment on attachment 200377 [details] Patch Attachment 200377 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/333340 Comment on attachment 200377 [details] Patch Attachment 200377 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/332355 Created attachment 200420 [details]
Patch
Created attachment 200424 [details]
Patch
Created attachment 200428 [details]
Patch
Comment on attachment 200428 [details] Patch Attachment 200428 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/360492 Comment on attachment 200428 [details] Patch Attachment 200428 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/343470 Created attachment 200436 [details]
Patch
Comment on attachment 200436 [details] Patch Attachment 200436 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/392450 Comment on attachment 200436 [details] Patch Attachment 200436 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/343477 Created attachment 200438 [details]
Patch
Comment on attachment 200438 [details] Patch Attachment 200438 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/332516 Comment on attachment 200438 [details] Patch Attachment 200438 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/332521 Comment on attachment 200438 [details] Patch Attachment 200438 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/342020 Comment on attachment 200438 [details]
Patch
I think this is going in the wrong direction.
This:
target.appendRange(v.begin(), b.end());
reads worse than
target.appendContent(v);
IMHO.
The appendVector() in IDBLevelDBCoding.h is a bad sign.
(In reply to comment #25) > (From update of attachment 200438 [details]) > I think this is going in the wrong direction. > > This: > target.appendRange(v.begin(), b.end()); > reads worse than > target.appendContent(v); > IMHO. OK. (I've been doing enough STL programming lately that I don't think it's worse). How about we keep appendVector around for now but get rid of the Vector::append overload. > > The appendVector() in IDBLevelDBCoding.h is a bad sign. IDBLevelDBCoding is a giant mess - it creates a gazillion of temporary vectors which makes the encoding algorithm essentially O(n2). Created attachment 200590 [details]
Patch
Comment on attachment 200590 [details] Patch Attachment 200590 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/408110 Comment on attachment 200590 [details] Patch Attachment 200590 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/412106 Comment on attachment 200590 [details] Patch Attachment 200590 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/401692 Comment on attachment 200590 [details] Patch Attachment 200590 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/419046 Created attachment 200591 [details]
Patch
Comment on attachment 200591 [details] Patch Attachment 200591 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/413102 Comment on attachment 200591 [details] Patch Attachment 200591 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/410089 Comment on attachment 200591 [details] Patch Attachment 200591 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/417079 Created attachment 200592 [details]
Patch
Created attachment 200593 [details]
Patch
Created attachment 200594 [details]
Patch
Comment on attachment 200594 [details]
Patch
k
Committed r149579: <http://trac.webkit.org/changeset/149579> (In reply to comment #25) > I think this is going in the wrong direction. > > This: > target.appendRange(v.begin(), b.end()); > reads worse than > target.appendContent(v); > IMHO. > > The appendVector() in IDBLevelDBCoding.h is a bad sign. I agree with Ben and was glad to see this fixed. Passing begin/end pairs into functions just to do basic operations on containers is one of the awkward aspects of STL that I don’t want to replicate. I think there are better solutions in more-modern C++. Analogous to the new for loop, which hides the begin/end. |