115535 – Remove the Vector::append overload that takes a Vector

RESOLVED FIXED 115535

Remove the Vector::append overload that takes a Vector

https://bugs.webkit.org/show_bug.cgi?id=115535

Summary Remove the Vector::append overload that takes a Vector

Anders Carlsson

Reported 2013-05-02 16:31:33 PDT

Remove the Vector::append overload that takes a Vector

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 attachment proposed patch, testcase, etc.

Anders Carlsson

Comment 1 2013-05-02 16:35:04 PDT

Created attachment 200368 [details] Patch

Anders Carlsson

Comment 2 2013-05-02 16:43:23 PDT

Created attachment 200370 [details] Patch

EFL EWS Bot

Comment 3 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

Early Warning System Bot

Comment 4 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

EFL EWS Bot

Comment 5 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

Early Warning System Bot

Comment 6 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

Anders Carlsson

Comment 7 2013-05-02 17:11:22 PDT

Created attachment 200374 [details] Patch

Anders Carlsson

Comment 8 2013-05-02 17:19:02 PDT

Created attachment 200375 [details] Patch

Anders Carlsson

Comment 9 2013-05-02 17:28:51 PDT

Created attachment 200377 [details] Patch

EFL EWS Bot

Comment 10 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

EFL EWS Bot

Comment 11 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

kov's GTK+ EWS bot

Comment 12 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

Anders Carlsson

Comment 13 2013-05-03 09:37:48 PDT

Created attachment 200420 [details] Patch

Anders Carlsson

Comment 14 2013-05-03 09:48:44 PDT

Created attachment 200424 [details] Patch

Anders Carlsson

Comment 15 2013-05-03 09:59:27 PDT

Created attachment 200428 [details] Patch

EFL EWS Bot

Comment 16 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

EFL EWS Bot

Comment 17 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

Anders Carlsson

Comment 18 2013-05-03 10:22:27 PDT

Created attachment 200436 [details] Patch

EFL EWS Bot

Comment 19 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

EFL EWS Bot

Comment 20 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

Anders Carlsson

Comment 21 2013-05-03 10:49:00 PDT

Created attachment 200438 [details] Patch

EFL EWS Bot

Comment 22 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

EFL EWS Bot

Comment 23 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

kov's GTK+ EWS bot

Comment 24 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

Benjamin Poulain

Comment 25 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.

Anders Carlsson

Comment 26 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).

Anders Carlsson

Comment 27 2013-05-05 09:44:28 PDT

Created attachment 200590 [details] Patch

Early Warning System Bot

Comment 28 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

EFL EWS Bot

Comment 29 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

EFL EWS Bot

Comment 30 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

Early Warning System Bot

Comment 31 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

Anders Carlsson

Comment 32 2013-05-05 09:59:46 PDT

Created attachment 200591 [details] Patch

Early Warning System Bot

Comment 33 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

Early Warning System Bot

Comment 34 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

EFL EWS Bot

Comment 35 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

Anders Carlsson

Comment 36 2013-05-05 10:13:56 PDT

Created attachment 200592 [details] Patch

Anders Carlsson

Comment 37 2013-05-05 10:42:33 PDT

Created attachment 200593 [details] Patch

Anders Carlsson

Comment 38 2013-05-05 10:52:23 PDT

Created attachment 200594 [details] Patch

Andreas Kling

Comment 39 2013-05-05 11:59:52 PDT

Comment on attachment 200594 [details] Patch k

Anders Carlsson

Comment 40 2013-05-05 12:03:12 PDT

Committed r149579: <http://trac.webkit.org/changeset/149579>

Darin Adler

Comment 41 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.

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware Unspecified

OS Unspecified

Product WebKit

Component New Bugs

Assignee

Anders Carlsson

Reported

2013-05-02 16:31 PDT

Modified

2013-05-06 09:17 PDT History

CC List

9 users Show

URL

Keywords

Depends on

Blocks