Bug 115535 - Remove the Vector::append overload that takes a Vector
Summary: Remove the Vector::append overload that takes a Vector
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Anders Carlsson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-02 16:31 PDT by Anders Carlsson
Modified: 2013-05-06 09:17 PDT (History)
9 users (show)

See Also:

Attachments
Patch (35.08 KB, patch)
2013-05-02 16:35 PDT, Anders Carlsson 		no flags Details | Formatted Diff | Diff
Patch (36.39 KB, patch)
2013-05-02 16:43 PDT, Anders Carlsson 		no flags Details | Formatted Diff | Diff
Patch (46.31 KB, patch)
2013-05-02 17:11 PDT, Anders Carlsson 		no flags Details | Formatted Diff | Diff
Patch (47.04 KB, patch)
2013-05-02 17:19 PDT, Anders Carlsson 		no flags Details | Formatted Diff | Diff
Patch (47.99 KB, patch)
2013-05-02 17:28 PDT, Anders Carlsson 		no flags Details | Formatted Diff | Diff
Patch (48.73 KB, patch)
2013-05-03 09:37 PDT, Anders Carlsson 		no flags Details | Formatted Diff | Diff
Patch (49.08 KB, patch)
2013-05-03 09:48 PDT, Anders Carlsson 		no flags Details | Formatted Diff | Diff
Patch (50.13 KB, patch)
2013-05-03 09:59 PDT, Anders Carlsson 		no flags Details | Formatted Diff | Diff
Patch (50.12 KB, patch)
2013-05-03 10:22 PDT, Anders Carlsson 		no flags Details | Formatted Diff | Diff
Patch (50.99 KB, patch)
2013-05-03 10:49 PDT, Anders Carlsson 		no flags Details | Formatted Diff | Diff
Patch (43.66 KB, patch)
2013-05-05 09:44 PDT, Anders Carlsson 		no flags Details | Formatted Diff | Diff
Patch (45.15 KB, patch)
2013-05-05 09:59 PDT, Anders Carlsson 		no flags Details | Formatted Diff | Diff
Patch (46.47 KB, patch)
2013-05-05 10:13 PDT, Anders Carlsson 		no flags Details | Formatted Diff | Diff
Patch (47.71 KB, patch)
2013-05-05 10:42 PDT, Anders Carlsson 		no flags Details | Formatted Diff | Diff
Patch (48.05 KB, patch)
2013-05-05 10:52 PDT, Anders Carlsson 		kling: review+
Details | Formatted Diff | Diff
Show Obsolete (14) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Anders Carlsson 2013-05-02 16:31:33 PDT 
Remove the Vector::append overload that takes a Vector
Comment 1 Anders Carlsson 2013-05-02 16:35:04 PDT 
Created attachment 200368 [details]
Patch
Comment 2 Anders Carlsson 2013-05-02 16:43:23 PDT 
Created attachment 200370 [details]
Patch
Comment 3 EFL EWS Bot 2013-05-02 16:57:43 PDT 
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 4 Early Warning System Bot 2013-05-02 16:58:45 PDT 
Comment on attachment 200370 [details]
Patch

Attachment 200370 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/393265
Comment 5 EFL EWS Bot 2013-05-02 17:01:15 PDT 
Comment on attachment 200370 [details]
Patch

Attachment 200370 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/393269
Comment 6 Early Warning System Bot 2013-05-02 17:02:10 PDT 
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
Comment 7 Anders Carlsson 2013-05-02 17:11:22 PDT 
Created attachment 200374 [details]
Patch
Comment 8 Anders Carlsson 2013-05-02 17:19:02 PDT 
Created attachment 200375 [details]
Patch
Comment 9 Anders Carlsson 2013-05-02 17:28:51 PDT 
Created attachment 200377 [details]
Patch
Comment 10 EFL EWS Bot 2013-05-02 17:42:19 PDT 
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 11 EFL EWS Bot 2013-05-02 17:49:26 PDT 
Comment on attachment 200377 [details]
Patch

Attachment 200377 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/333340
Comment 12 kov's GTK+ EWS bot 2013-05-02 22:57:15 PDT 
Comment on attachment 200377 [details]
Patch

Attachment 200377 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/332355
Comment 13 Anders Carlsson 2013-05-03 09:37:48 PDT 
Created attachment 200420 [details]
Patch
Comment 14 Anders Carlsson 2013-05-03 09:48:44 PDT 
Created attachment 200424 [details]
Patch
Comment 15 Anders Carlsson 2013-05-03 09:59:27 PDT 
Created attachment 200428 [details]
Patch
Comment 16 EFL EWS Bot 2013-05-03 10:16:32 PDT 
Comment on attachment 200428 [details]
Patch

Attachment 200428 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/360492
Comment 17 EFL EWS Bot 2013-05-03 10:17:06 PDT 
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
Comment 18 Anders Carlsson 2013-05-03 10:22:27 PDT 
Created attachment 200436 [details]
Patch
Comment 19 EFL EWS Bot 2013-05-03 10:39:33 PDT 
Comment on attachment 200436 [details]
Patch

Attachment 200436 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/392450
Comment 20 EFL EWS Bot 2013-05-03 10:44:04 PDT 
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
Comment 21 Anders Carlsson 2013-05-03 10:49:00 PDT 
Created attachment 200438 [details]
Patch
Comment 22 EFL EWS Bot 2013-05-03 11:11:03 PDT 
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 23 EFL EWS Bot 2013-05-03 11:23:08 PDT 
Comment on attachment 200438 [details]
Patch

Attachment 200438 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/332521
Comment 24 kov's GTK+ EWS bot 2013-05-03 12:40:48 PDT 
Comment on attachment 200438 [details]
Patch

Attachment 200438 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/342020
Comment 25 Benjamin Poulain 2013-05-04 18:23:21 PDT 
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.
Comment 26 Anders Carlsson 2013-05-05 08:21:50 PDT 
(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).
Comment 27 Anders Carlsson 2013-05-05 09:44:28 PDT 
Created attachment 200590 [details]
Patch
Comment 28 Early Warning System Bot 2013-05-05 09:54:01 PDT 
Comment on attachment 200590 [details]
Patch

Attachment 200590 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/408110
Comment 29 EFL EWS Bot 2013-05-05 09:54:26 PDT 
Comment on attachment 200590 [details]
Patch

Attachment 200590 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/412106
Comment 30 EFL EWS Bot 2013-05-05 09:55:23 PDT 
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 31 Early Warning System Bot 2013-05-05 09:55:51 PDT 
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
Comment 32 Anders Carlsson 2013-05-05 09:59:46 PDT 
Created attachment 200591 [details]
Patch
Comment 33 Early Warning System Bot 2013-05-05 10:09:14 PDT 
Comment on attachment 200591 [details]
Patch

Attachment 200591 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/413102
Comment 34 Early Warning System Bot 2013-05-05 10:11:38 PDT 
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 35 EFL EWS Bot 2013-05-05 10:12:26 PDT 
Comment on attachment 200591 [details]
Patch

Attachment 200591 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/417079
Comment 36 Anders Carlsson 2013-05-05 10:13:56 PDT 
Created attachment 200592 [details]
Patch
Comment 37 Anders Carlsson 2013-05-05 10:42:33 PDT 
Created attachment 200593 [details]
Patch
Comment 38 Anders Carlsson 2013-05-05 10:52:23 PDT 
Created attachment 200594 [details]
Patch
Comment 39 Andreas Kling 2013-05-05 11:59:52 PDT 
Comment on attachment 200594 [details]
Patch

k
Comment 40 Anders Carlsson 2013-05-05 12:03:12 PDT 
Committed r149579: <http://trac.webkit.org/changeset/149579>
Comment 41 Darin Adler 2013-05-06 09:17:56 PDT 
(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.