Yes, absolutely, it is worth catching this at compile-time.
But what you describe above wouldn't catch that at compile-time, because
the compiler would use this operator to get an implicit conversion to
size_t followed by implicit conversion from size_t to uint32_t.
You could use MFBT's new MOZ_EXPLICIT_CONVERSION to annotate this
conversion operator with 'explicit' on supporting C++11 compilers. But
instead, I would not bother trying to make this a conversion operator, I
would just make this a plain old getter method.
I would also not call that Size, but IndexOfResult. The problem is that
what IndexOf() returns is not necessarily a "size" or "index", it may also
be this special thing called "NoIndex". So I don't think that its return
type should "feel like" it _is_ just an index. Instead, its return type
should be something that is _either_ an index or NoIndex.
Something like this:
class IndexOfResult {
const index_type mIndex;
static const index_type NoIndex = index_type(-1);
public:
IndexOfResult(index_type index) : mIndex(index) {}
IndexOfResult(const IndexOfResult& other) : mIndex(other.mIndex) {}
bool Found() const {
return mIndex != NoIndex;
}
index_type Value() const {
MOZ_ASSERT(Found());
return mIndex;
}
};
Benoit
2014-05-11 10:25 GMT-04:00 Ehsan Akhgari <[email protected]>:
> On 2014-05-11, 10:15 AM, Benoit Jacob wrote:
>
>> Hi,
>>
>> Since Bug 1004098 landed, the type of nsTArray lengths and indices is now
>> size_t.
>>
>> Code using nsTArrays is encouraged to use size_t for indexing them; in
>> most
>> cases, this does not really matter; however there is one case where this
>> does matter, which is when user code stores the result of
>> nsTArray::IndexOf().
>>
>> Indeed, nsTArray::NoIndex used to be uint32_t(-1), which has the value
>> 2^32
>> - 1. Now, nsTArray::NoIndex is size_t(-1) which, on x86-64, has the value
>> 2^64 - 1.
>>
>> This means that code like this is no longer correct:
>>
>> uint32_t index = array.IndexOf(thing);
>>
>> Such code should be changed do:
>>
>> size_t index = array.IndexOf(thing);
>>
>> Or, better still (slightly pedantic but would have been correct all
>> along):
>>
>> ArrayType::index_type index = array.IndexOf(thing);
>>
> >
>
>> Where ArrayType is the type of that 'array' variable (one could use
>> decltype(array) too).
>>
>
> Do you think it's worth trying to make the bad code above not compile, by
> returning an object from IndexOf which only provides an implicit conversion
> to size_t and not uint32_t? Like:
>
> template <class T>
> class nsTArray {
> // ...
> struct Size {
> Size(size_t);
> operator size_t() const;
> };
> Size IndexOf(...);
> };
>
_______________________________________________
dev-platform mailing list
[email protected]
https://lists.mozilla.org/listinfo/dev-platform
