Make it possible to pass any TArray (fallible or not) to a const ref taking any other TArray (fallible or not)

RESOLVED FIXED in mozilla20

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Tracking

unspecified
mozilla20
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Since if people don't cast away const they can't use the allocator anyway.
(Assignee)

Comment 1

6 years ago
Created attachment 690047 [details] [diff] [review]
part 1.  Make it possible to use the various-allocator nsTArrays interchangeably as long as you're working with const objects.
Attachment #690047 - Flags: review?(justin.lebar+bug)
(Assignee)

Comment 2

6 years ago
Created attachment 690049 [details] [diff] [review]
part 2.  Allow Nullable<> of various array types to work sanely.

Peter, I can understand if you don't want to take this black magic....
Attachment #690049 - Flags: review?(peterv)
(Assignee)

Comment 3

6 years ago
Created attachment 690051 [details] [diff] [review]
Part 2, even compiling
Attachment #690051 - Flags: review?(peterv)
(Assignee)

Updated

6 years ago
Attachment #690049 - Attachment is obsolete: true
Attachment #690049 - Flags: review?(peterv)
Comment on attachment 690047 [details] [diff] [review]
part 1.  Make it possible to use the various-allocator nsTArrays interchangeably as long as you're working with const objects.

> +  // Allow converting to a const array with a different kind of allocator,
> +  // Since the allocator doesn't matter for const arrays

Would you mind changing the second line so it begins with a lower-case letter and ends with a period?

r=me, and thanks for writing a test!
Attachment #690047 - Flags: review?(justin.lebar+bug) → review+
Comment on attachment 690051 [details] [diff] [review]
Part 2, even compiling

> +  // mIsNull MUST COME FIRST because otherwise the casting in our array
> +  // conversion operators would shift where it is found in the struct.
> +  bool mIsNull;
>    T mValue;
> -  bool mIsNull;
>  
>  public:
>    Nullable()
>      : mIsNull(true)
>    {}
>  
> -  Nullable(T aValue)
> +  explicit Nullable(T aValue)
>      : mValue(aValue)
>      , mIsNull(false)

-Werror on tinderbox doesn't like the fact that the initialization list's order doesn't match the order of the members.
Created attachment 693033 [details] [diff] [review]
Follow-up to part 2: Fix initialization list order in Nullable.h.
(Assignee)

Comment 7

6 years ago
Ah, thank you.  Good catch.

I'm being tempted to land part 1 and not worry about part 2, fwiw...
Comment on attachment 690051 [details] [diff] [review]
Part 2, even compiling

r=me, fwiw.  Are you OK if I land these two patches along with bug 819791?
Attachment #690051 - Flags: review+
(Assignee)

Comment 9

6 years ago
Yes, go for it!  Three patches, that is, counting your initializer reordering.
https://hg.mozilla.org/mozilla-central/rev/6ed3f784cdd3
https://hg.mozilla.org/mozilla-central/rev/51e3f82d820b
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Attachment #690051 - Flags: review?(peterv) → review+
You need to log in before you can comment on or make changes to this bug.