Bug 75359

Summary: It should be easier to iterate a Vector backwards
Product: WebKit Reporter: Sam Weinig <sam>
Component: New BugsAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch andersca: review+, webkit.review.bot: commit-queue-

Description Sam Weinig 2011-12-29 09:13:15 PST
It should be easier to iterate a Vector backwards
Comment 1 Sam Weinig 2011-12-29 09:14:39 PST
The current method of iterating Vectors backwards is a bit ugly and doesn't work with C++11 range-based for loops.  I want an easier way!
Comment 2 Sam Weinig 2011-12-29 09:24:15 PST
Created attachment 120753 [details]
Patch
Comment 3 WebKit Review Bot 2011-12-29 09:26:53 PST
Attachment 120753 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/wtf/Vector.h:484:  reverse_iterator is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/Vector.h:485:  const_reverse_iterator is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 2 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Anders Carlsson 2011-12-29 09:27:56 PST
Comment on attachment 120753 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=120753&action=review

> Tools/ChangeLog:11
> +        to work on windows.

Please explain why we set GTEST_HAS_TR1_TUPLE to 0 here.

> Tools/TestWebKitAPI/Tests/Vector.cpp:82
> +// Vector::reveresed() can only be tested if auto and range-for support is
> +// availiable.

Typo, reveresed. Also, this isn't true. You could do something like

for (Vector<int>::const_iterator it = intVector.reversed.begin(), end = intVector.reversed.end(); it != end; ++it) { }
Comment 5 Sam Weinig 2011-12-29 09:35:25 PST
(In reply to comment #4)
> (From update of attachment 120753 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=120753&action=review
> 
> > Tools/ChangeLog:11
> > +        to work on windows.
> 
> Please explain why we set GTEST_HAS_TR1_TUPLE to 0 here.

Ok.

> 
> > Tools/TestWebKitAPI/Tests/Vector.cpp:82
> > +// Vector::reveresed() can only be tested if auto and range-for support is
> > +// availiable.
> 
> Typo, reveresed. Also, this isn't true. You could do something like
> 
> for (Vector<int>::const_iterator it = intVector.reversed.begin(), end = intVector.reversed.end(); it != end; ++it) { }

Doh. I will change to test that way. weinig--.
Comment 6 Sam Weinig 2011-12-29 09:52:17 PST
Actually, if I don't need to use auto, I don't really need to change TestWebKitAPI to use C++11, but that might be nice on its own since then we could add tests for the rvalue stuff I added to RetainPtr and friends.

Thoughts? Do all the bots have new enough clang/libc++ for this not to break stuff?
Comment 7 WebKit Review Bot 2011-12-29 10:06:57 PST
Comment on attachment 120753 [details]
Patch

Attachment 120753 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11035925
Comment 8 Sam Weinig 2011-12-29 20:50:30 PST
Committed r103833: <http://trac.webkit.org/changeset/103833>