924014 - Do not rely on std::numeric_limits<char16_t> being available

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[(no longer active)]

Attachment: Patch (v1)

Comment 1

[(no longer active)]

Attachment: Patch (v2)

Case insensitive filesystems suck!  This conflicts with <limits.h>. :(

Comment 2

[Chris Peterson [:cpeterson]]

Comment on attachment 814019 [details] [diff] [review]
Patch (v2)

Review of attachment 814019 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/ctypes/CTypes.cpp
@@ +1377,5 @@
>  JS_STATIC_ASSERT(sizeof(long long) == 8);
>  JS_STATIC_ASSERT(sizeof(size_t) == sizeof(uintptr_t));
>  JS_STATIC_ASSERT(sizeof(float) == 4);
>  JS_STATIC_ASSERT(sizeof(PRFuncPtr) == sizeof(void*));
> +JS_STATIC_ASSERT(NumericLimits<double>::is_signed);

If NumericLimits is only needed in code that may allow char16_t, why do you need NumericLimits<double> here?

Comment 3

[(no longer active)]

(In reply to comment #2)
> Comment on attachment 814019 [details] [diff] [review]
>   --> https://bugzilla.mozilla.org/attachment.cgi?id=814019
> Patch (v2)
> 
> Review of attachment 814019 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/ctypes/CTypes.cpp
> @@ +1377,5 @@
> >  JS_STATIC_ASSERT(sizeof(long long) == 8);
> >  JS_STATIC_ASSERT(sizeof(size_t) == sizeof(uintptr_t));
> >  JS_STATIC_ASSERT(sizeof(float) == 4);
> >  JS_STATIC_ASSERT(sizeof(PRFuncPtr) == sizeof(void*));
> > +JS_STATIC_ASSERT(NumericLimits<double>::is_signed);
> 
> If NumericLimits is only needed in code that may allow char16_t, why do you
> need NumericLimits<double> here?

For consistency in this file.

Comment 4

[Jeff Walden [:Waldo]]

Comment on attachment 814019 [details] [diff] [review]
Patch (v2)

Review of attachment 814019 [details] [diff] [review]:
-----------------------------------------------------------------

::: mfbt/NumericLimits.h
@@ +19,5 @@
> +/**
> + * The NumericLimits class provides a compatibility layer with std::numeric_limits
> + * for char16_t, otherwise it is exactly the same as std::numeric_limits.
> + * Code which does not need std::numeric_limits<char16_t> should avoid using
> + * NumericLimits.

I'm not sure I agree with this bit, exactly, but meh.

@@ +21,5 @@
> + * for char16_t, otherwise it is exactly the same as std::numeric_limits.
> + * Code which does not need std::numeric_limits<char16_t> should avoid using
> + * NumericLimits.
> + */
> +template<class T>

typename T, since this won't always be a class (stylistic nit, I know they're equally functional here :-) ).

Comment 5

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7d0c0b0b8913

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7d0c0b0b8913