Bug 115535

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:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	kling: review+

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.