Closed
Bug 1259248
Opened 9 years ago
Closed 9 years ago
Add an ArrayView class
Categories
(Core :: Graphics, defect)
Core
Graphics
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: jrmuizel, Unassigned)
Details
Attachments
(1 file, 1 obsolete file)
3.25 KB,
patch
|
botond
:
review+
|
Details | Diff | Splinter Review |
This is a quick and dirty implementation so that we have something to iterate on.
Reporter | ||
Updated•9 years ago
|
Attachment #8734119 -
Flags: review?(botond)
Updated•9 years ago
|
Attachment #8734119 -
Attachment is patch: true
Reporter | ||
Comment 1•9 years ago
|
||
Attachment #8734119 -
Attachment is obsolete: true
Attachment #8734119 -
Flags: review?(botond)
Attachment #8734132 -
Flags: review?(botond)
Comment 2•9 years ago
|
||
Comment on attachment 8734132 [details] [diff] [review]
array-view
Review of attachment 8734132 [details] [diff] [review]:
-----------------------------------------------------------------
LGTM as a first pass at a utility that we can start using right away. We should iterate on this (preferably matching the interface of one of the array_view-like classes being proposed for standardization) and move it to MFBT as a follow-up.
::: gfx/src/ArrayView.h
@@ +12,5 @@
> +
> +template<typename T>
> +class ArrayView
> +{
> + public:
nits: use two-space identation, and access-specifiers don't get indented by an extra level
@@ +17,5 @@
> + ArrayView(nsTArray<T>& aData) :
> + mData(aData.Elements()), mLength(aData.Length())
> + {
> + }
> + ArrayView(const T* aData, const size_t aLength) :
No need for the toplevel const on the second parameter
@@ +23,5 @@
> + {
> + }
> + const T& operator[](const size_t aIdx) const
> + {
> + return mData[aIdx];
MOZ_ASSERT() that the index is in range
Attachment #8734132 -
Flags: review?(botond) → review+
Comment 5•9 years ago
|
||
Comment on attachment 8734132 [details] [diff] [review]
array-view
Review of attachment 8734132 [details] [diff] [review]:
-----------------------------------------------------------------
Isn't this more or less the same as mfbt's Range class? https://dxr.mozilla.org/mozilla-central/source/mfbt/Range.h
Comment 6•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d95bbecda3af
https://hg.mozilla.org/mozilla-central/rev/5ed70dba80c6
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment 8•9 years ago
|
||
bugherder |
You need to log in
before you can comment on or make changes to this bug.
Description
•