Closed
Bug 1061459
Opened 10 years ago
Closed 8 years ago
Implement in-place reverse elements for nsTArray
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
INVALID
People
(Reporter: gkrizsanits, Assigned: qdot)
References
Details
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
While implementing HTML imports I bumped into the fact that nsTArray has no such feature. I would have to do the element reversal manually, which is quite ugly. I think the best would be to have some utility library for simple algorithms for our containers, but for the time being it would be great if nsTArray would support at least this simple thing. I can take this bug if someone is willing to review it or has a better idea where to put a trivial algorithm like that.
Comment 1•10 years ago
|
||
We started to collect generic tools like algorithms (e.g. BinarySearch.h) in MFBT.
I think this would be best done there.
Do you actually need to reverse them or would a reverse view be enough?
Comment 2•10 years ago
|
||
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #0)
> I can take this bug if someone is willing to review it or has a better idea where to put a
> trivial algorithm like that.
I am willing to review. MFBT would probably be a slightly better place to put it.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → kyle
Assignee | ||
Comment 3•8 years ago
|
||
Very simple iterator reverse algorithm. Needed for bug 1287588 and found this bug so did a general version.
Attachment #8782240 -
Flags: review?(nfroyd)
Comment 4•8 years ago
|
||
The situation has changed in the last two years; I think we can rely on the STL to provide a correct reverse() that works with move constructors. (We could rely on reverse, but not the move construction part.) Can we just use that instead?
Flags: needinfo?(kyle)
Assignee | ||
Comment 5•8 years ago
|
||
Sure! I think I'm still stuck in the old "Avoid std where possible" camp so this came up. Will just invalidate this. Thanks!
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(kyle)
Resolution: --- → INVALID
Comment 6•8 years ago
|
||
Comment on attachment 8782240 [details] [diff] [review]
Patch 1 (v1) - mfbt In-Place Reverse utility
Review of attachment 8782240 [details] [diff] [review]:
-----------------------------------------------------------------
Great, canceling this review, then.
Attachment #8782240 -
Flags: review?(nfroyd)
You need to log in
before you can comment on or make changes to this bug.
Description
•