Mojo: add support for {double,float}.{INFINITY,NEGATIVE_INFINITY,NAN}

Mojo: add support for {double,float}.{INFINITY,NEGATIVE_INFINITY,NAN}

With this change, it is possible to assign INFINITY, NEGATIVE_INFINITY or NAN as a default value for any double or float field.

BUG=402216
Committed: https://crrev.com/d9f2bcd21b589f5b60f474cf0fbcf54420105266
Cr-Commit-Position: refs/heads/master@{#291864}

[viettrungluu, 2014-08-14 22:32:25]

The way you're doing it also adds some possibility for confusion and/or
ambiguity.

E.g., there's nothing preventing you from declaring a constant named "NaN".

Now the meaning of NaN on the RHS an = is ambiguous, and may depend on the type
of the LHS.

(On the other hand, I'm hesitant to make Inf and NaN keywords, and take away
more possible names.)

https://codereview.chromium.org/474063002/diff/1/mojo/public/tools/bindings/g...
File mojo/public/tools/bindings/generators/mojom_cpp_generator.py (right):

https://codereview.chromium.org/474063002/diff/1/mojo/public/tools/bindings/g...
mojo/public/tools/bindings/generators/mojom_cpp_generator.py:229: return
"std::numeric_limits<double>::infinity()"
This (and everything else) probably creates a static initializer if used for
constant values, in C++03. It may be better to use the C macros from <math.h>.

https://codereview.chromium.org/474063002/diff/1/mojo/public/tools/bindings/p...
File mojo/public/tools/bindings/pylib/mojom/parse/lexer.py (right):

https://codereview.chromium.org/474063002/diff/1/mojo/public/tools/bindings/p...
mojo/public/tools/bindings/pylib/mojom/parse/lexer.py:148:
exponent_part+'?)|([0-9]+'+exponent_part+'))|[-+]?Inf|NaN)'
Currently, for numbers, we use unary minus (and plus), so that "- 123" is
allowed (instead of only allowing "-123").

(I have mixed feelings about allowing the space, but it's consistent with most
languages.)

We should probably re-use that for +/- infinity. (Also, this might have the side
effect of allowing "- -Inf" and maybe even "--Inf", etc.)

[viettrungluu, 2014-08-14 22:36:01]

On 2014/08/14 22:32:25, viettrungluu wrote:
> The way you're doing it also adds some possibility for confusion and/or
> ambiguity.
> 
> E.g., there's nothing preventing you from declaring a constant named "NaN".
> 
> Now the meaning of NaN on the RHS an = is ambiguous, and may depend on the
type
> of the LHS.

On second thought, I think I'm wrong about this. The lexer will presumably match
this as a floating point value first....

> 
> (On the other hand, I'm hesitant to make Inf and NaN keywords, and take away
> more possible names.)

This comment still applies, in that we're taking away short names from the list
of possible names, which may be bad.

> 
>
https://codereview.chromium.org/474063002/diff/1/mojo/public/tools/bindings/g...
> File mojo/public/tools/bindings/generators/mojom_cpp_generator.py (right):
> 
>
https://codereview.chromium.org/474063002/diff/1/mojo/public/tools/bindings/g...
> mojo/public/tools/bindings/generators/mojom_cpp_generator.py:229: return
> "std::numeric_limits<double>::infinity()"
> This (and everything else) probably creates a static initializer if used for
> constant values, in C++03. It may be better to use the C macros from <math.h>.
> 
>
https://codereview.chromium.org/474063002/diff/1/mojo/public/tools/bindings/p...
> File mojo/public/tools/bindings/pylib/mojom/parse/lexer.py (right):
> 
>
https://codereview.chromium.org/474063002/diff/1/mojo/public/tools/bindings/p...
> mojo/public/tools/bindings/pylib/mojom/parse/lexer.py:148:
> exponent_part+'?)|([0-9]+'+exponent_part+'))|[-+]?Inf|NaN)'
> Currently, for numbers, we use unary minus (and plus), so that "- 123" is
> allowed (instead of only allowing "-123").
> 
> (I have mixed feelings about allowing the space, but it's consistent with most
> languages.)
> 
> We should probably re-use that for +/- infinity. (Also, this might have the
side
> effect of allowing "- -Inf" and maybe even "--Inf", etc.)

[darin (slow to review), 2014-08-14 23:28:22]

On Thu, Aug 14, 2014 at 3:36 PM, <viettrungluu@chromium.org> wrote:

> On 2014/08/14 22:32:25, viettrungluu wrote:
>
>> The way you're doing it also adds some possibility for confusion and/or
>> ambiguity.
>>
>
>  E.g., there's nothing preventing you from declaring a constant named
>> "NaN".
>>
>
>  Now the meaning of NaN on the RHS an = is ambiguous, and may depend on the
>>
> type
>
>> of the LHS.
>>
>
> On second thought, I think I'm wrong about this. The lexer will presumably
> match
> this as a floating point value first....
>
>
>
>  (On the other hand, I'm hesitant to make Inf and NaN keywords, and take
>> away
>> more possible names.)
>>
>
> This comment still applies, in that we're taking away short names from the
> list
> of possible names, which may be bad.


Would you recommend longer names or some sort of name prefixing? I could
also just
match C and use INFINITY and NAN.



>
>
>
>
> https://codereview.chromium.org/474063002/diff/1/mojo/
> public/tools/bindings/generators/mojom_cpp_generator.py
>
>> File mojo/public/tools/bindings/generators/mojom_cpp_generator.py
>> (right):
>>
>
>
> https://codereview.chromium.org/474063002/diff/1/mojo/
> public/tools/bindings/generators/mojom_cpp_generator.py#newcode229
>
>> mojo/public/tools/bindings/generators/mojom_cpp_generator.py:229: return
>> "std::numeric_limits<double>::infinity()"
>> This (and everything else) probably creates a static initializer if used
>> for
>> constant values, in C++03. It may be better to use the C macros from
>> <math.h>.
>>
>
Hmm, OK... I'll have to check that out.



>
>
> https://codereview.chromium.org/474063002/diff/1/mojo/
> public/tools/bindings/pylib/mojom/parse/lexer.py
>
>> File mojo/public/tools/bindings/pylib/mojom/parse/lexer.py (right):
>>
>
>
> https://codereview.chromium.org/474063002/diff/1/mojo/
> public/tools/bindings/pylib/mojom/parse/lexer.py#newcode148
>
>> mojo/public/tools/bindings/pylib/mojom/parse/lexer.py:148:
>> exponent_part+'?)|([0-9]+'+exponent_part+'))|[-+]?Inf|NaN)'
>> Currently, for numbers, we use unary minus (and plus), so that "- 123" is
>> allowed (instead of only allowing "-123").
>>
>
>  (I have mixed feelings about allowing the space, but it's consistent with
>> most
>> languages.)
>>
>
>  We should probably re-use that for +/- infinity. (Also, this might have
>> the
>>
> side
>
>> effect of allowing "- -Inf" and maybe even "--Inf", etc.)
>>
>
>
Good thinking.



>
>
> https://codereview.chromium.org/474063002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[viettrungluu, 2014-08-15 00:42:26]

On 2014/08/14 23:28:22, darin wrote:
> On Thu, Aug 14, 2014 at 3:36 PM, <mailto:viettrungluu@chromium.org> wrote:
> 
> > On 2014/08/14 22:32:25, viettrungluu wrote:
> >
> >> The way you're doing it also adds some possibility for confusion and/or
> >> ambiguity.
> >>
> >
> >  E.g., there's nothing preventing you from declaring a constant named
> >> "NaN".
> >>
> >
> >  Now the meaning of NaN on the RHS an = is ambiguous, and may depend on the
> >>
> > type
> >
> >> of the LHS.
> >>
> >
> > On second thought, I think I'm wrong about this. The lexer will presumably
> > match
> > this as a floating point value first....
> >
> >
> >
> >  (On the other hand, I'm hesitant to make Inf and NaN keywords, and take
> >> away
> >> more possible names.)
> >>
> >
> > This comment still applies, in that we're taking away short names from the
> > list
> > of possible names, which may be bad.
> 
> 
> Would you recommend longer names or some sort of name prefixing? I could
> also just
> match C and use INFINITY and NAN.

Hmmm, the allcaps mostly puts them out of the set of names that you'd use for
other things, except for enum values, which is a bit unfortunate.

I wonder if we should have a "builtin" fake module/package/namespace, and use
something like builtin.kPlusInfinity, builtin.kMinusInfinity, and builtin.kNaN
or something like that (kMinusInfinity instead of just using unary minus and a
kInfinity, since we don't allow even unary expressions).

> 
> 
> 
> >
> >
> >
> >
> > https://codereview.chromium.org/474063002/diff/1/mojo/
> > public/tools/bindings/generators/mojom_cpp_generator.py
> >
> >> File mojo/public/tools/bindings/generators/mojom_cpp_generator.py
> >> (right):
> >>
> >
> >
> > https://codereview.chromium.org/474063002/diff/1/mojo/
> > public/tools/bindings/generators/mojom_cpp_generator.py#newcode229
> >
> >> mojo/public/tools/bindings/generators/mojom_cpp_generator.py:229: return
> >> "std::numeric_limits<double>::infinity()"
> >> This (and everything else) probably creates a static initializer if used
> >> for
> >> constant values, in C++03. It may be better to use the C macros from
> >> <math.h>.
> >>
> >
> Hmm, OK... I'll have to check that out.
> 
> 
> 
> >
> >
> > https://codereview.chromium.org/474063002/diff/1/mojo/
> > public/tools/bindings/pylib/mojom/parse/lexer.py
> >
> >> File mojo/public/tools/bindings/pylib/mojom/parse/lexer.py (right):
> >>
> >
> >
> > https://codereview.chromium.org/474063002/diff/1/mojo/
> > public/tools/bindings/pylib/mojom/parse/lexer.py#newcode148
> >
> >> mojo/public/tools/bindings/pylib/mojom/parse/lexer.py:148:
> >> exponent_part+'?)|([0-9]+'+exponent_part+'))|[-+]?Inf|NaN)'
> >> Currently, for numbers, we use unary minus (and plus), so that "- 123" is
> >> allowed (instead of only allowing "-123").
> >>
> >
> >  (I have mixed feelings about allowing the space, but it's consistent with
> >> most
> >> languages.)
> >>
> >
> >  We should probably re-use that for +/- infinity. (Also, this might have
> >> the
> >>
> > side
> >
> >> effect of allowing "- -Inf" and maybe even "--Inf", etc.)
> >>
> >
> >
> Good thinking.
> 
> 
> 
> >
> >
> > https://codereview.chromium.org/474063002/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[darin (slow to review), 2014-08-15 01:13:48]

double.POSITIVE_INFINITY, etc?
On Aug 14, 2014 5:42 PM, <viettrungluu@chromium.org> wrote:

> On 2014/08/14 23:28:22, darin wrote:
>
>> On Thu, Aug 14, 2014 at 3:36 PM, <mailto:viettrungluu@chromium.org>
>> wrote:
>>
>
>  > On 2014/08/14 22:32:25, viettrungluu wrote:
>> >
>> >> The way you're doing it also adds some possibility for confusion and/or
>> >> ambiguity.
>> >>
>> >
>> >  E.g., there's nothing preventing you from declaring a constant named
>> >> "NaN".
>> >>
>> >
>> >  Now the meaning of NaN on the RHS an = is ambiguous, and may depend on
>> the
>> >>
>> > type
>> >
>> >> of the LHS.
>> >>
>> >
>> > On second thought, I think I'm wrong about this. The lexer will
>> presumably
>> > match
>> > this as a floating point value first....
>> >
>> >
>> >
>> >  (On the other hand, I'm hesitant to make Inf and NaN keywords, and take
>> >> away
>> >> more possible names.)
>> >>
>> >
>> > This comment still applies, in that we're taking away short names from
>> the
>> > list
>> > of possible names, which may be bad.
>>
>
>
>  Would you recommend longer names or some sort of name prefixing? I could
>> also just
>> match C and use INFINITY and NAN.
>>
>
> Hmmm, the allcaps mostly puts them out of the set of names that you'd use
> for
> other things, except for enum values, which is a bit unfortunate.
>
> I wonder if we should have a "builtin" fake module/package/namespace, and
> use
> something like builtin.kPlusInfinity, builtin.kMinusInfinity, and
> builtin.kNaN
> or something like that (kMinusInfinity instead of just using unary minus
> and a
> kInfinity, since we don't allow even unary expressions).
>
>
>
>
>  >
>> >
>> >
>> >
>> > https://codereview.chromium.org/474063002/diff/1/mojo/
>> > public/tools/bindings/generators/mojom_cpp_generator.py
>> >
>> >> File mojo/public/tools/bindings/generators/mojom_cpp_generator.py
>> >> (right):
>> >>
>> >
>> >
>> > https://codereview.chromium.org/474063002/diff/1/mojo/
>> > public/tools/bindings/generators/mojom_cpp_generator.py#newcode229
>> >
>> >> mojo/public/tools/bindings/generators/mojom_cpp_generator.py:229:
>> return
>> >> "std::numeric_limits<double>::infinity()"
>> >> This (and everything else) probably creates a static initializer if
>> used
>> >> for
>> >> constant values, in C++03. It may be better to use the C macros from
>> >> <math.h>.
>> >>
>> >
>> Hmm, OK... I'll have to check that out.
>>
>
>
>
>  >
>> >
>> > https://codereview.chromium.org/474063002/diff/1/mojo/
>> > public/tools/bindings/pylib/mojom/parse/lexer.py
>> >
>> >> File mojo/public/tools/bindings/pylib/mojom/parse/lexer.py (right):
>> >>
>> >
>> >
>> > https://codereview.chromium.org/474063002/diff/1/mojo/
>> > public/tools/bindings/pylib/mojom/parse/lexer.py#newcode148
>> >
>> >> mojo/public/tools/bindings/pylib/mojom/parse/lexer.py:148:
>> >> exponent_part+'?)|([0-9]+'+exponent_part+'))|[-+]?Inf|NaN)'
>> >> Currently, for numbers, we use unary minus (and plus), so that "- 123"
>> is
>> >> allowed (instead of only allowing "-123").
>> >>
>> >
>> >  (I have mixed feelings about allowing the space, but it's consistent
>> with
>> >> most
>> >> languages.)
>> >>
>> >
>> >  We should probably re-use that for +/- infinity. (Also, this might have
>> >> the
>> >>
>> > side
>> >
>> >> effect of allowing "- -Inf" and maybe even "--Inf", etc.)
>> >>
>> >
>> >
>> Good thinking.
>>
>
>
>
>  >
>> >
>> > https://codereview.chromium.org/474063002/
>> >
>>
>
>  To unsubscribe from this group and stop receiving emails from it, send an
>>
> email
>
>> to mailto:chromium-reviews+unsubscribe@chromium.org.
>>
>
>
>
> https://codereview.chromium.org/474063002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[viettrungluu, 2014-08-18 17:53:12]

On 2014/08/15 01:13:48, darin wrote:
> double.POSITIVE_INFINITY, etc?

At first, I thought this was a bad idea. But I changed my mind. SGTM.

> On Aug 14, 2014 5:42 PM, <mailto:viettrungluu@chromium.org> wrote:
> 
> > On 2014/08/14 23:28:22, darin wrote:
> >
> >> On Thu, Aug 14, 2014 at 3:36 PM, <mailto:viettrungluu@chromium.org>
> >> wrote:
> >>
> >
> >  > On 2014/08/14 22:32:25, viettrungluu wrote:
> >> >
> >> >> The way you're doing it also adds some possibility for confusion and/or
> >> >> ambiguity.
> >> >>
> >> >
> >> >  E.g., there's nothing preventing you from declaring a constant named
> >> >> "NaN".
> >> >>
> >> >
> >> >  Now the meaning of NaN on the RHS an = is ambiguous, and may depend on
> >> the
> >> >>
> >> > type
> >> >
> >> >> of the LHS.
> >> >>
> >> >
> >> > On second thought, I think I'm wrong about this. The lexer will
> >> presumably
> >> > match
> >> > this as a floating point value first....
> >> >
> >> >
> >> >
> >> >  (On the other hand, I'm hesitant to make Inf and NaN keywords, and take
> >> >> away
> >> >> more possible names.)
> >> >>
> >> >
> >> > This comment still applies, in that we're taking away short names from
> >> the
> >> > list
> >> > of possible names, which may be bad.
> >>
> >
> >
> >  Would you recommend longer names or some sort of name prefixing? I could
> >> also just
> >> match C and use INFINITY and NAN.
> >>
> >
> > Hmmm, the allcaps mostly puts them out of the set of names that you'd use
> > for
> > other things, except for enum values, which is a bit unfortunate.
> >
> > I wonder if we should have a "builtin" fake module/package/namespace, and
> > use
> > something like builtin.kPlusInfinity, builtin.kMinusInfinity, and
> > builtin.kNaN
> > or something like that (kMinusInfinity instead of just using unary minus
> > and a
> > kInfinity, since we don't allow even unary expressions).
> >
> >
> >
> >
> >  >
> >> >
> >> >
> >> >
> >> > https://codereview.chromium.org/474063002/diff/1/mojo/
> >> > public/tools/bindings/generators/mojom_cpp_generator.py
> >> >
> >> >> File mojo/public/tools/bindings/generators/mojom_cpp_generator.py
> >> >> (right):
> >> >>
> >> >
> >> >
> >> > https://codereview.chromium.org/474063002/diff/1/mojo/
> >> > public/tools/bindings/generators/mojom_cpp_generator.py#newcode229
> >> >
> >> >> mojo/public/tools/bindings/generators/mojom_cpp_generator.py:229:
> >> return
> >> >> "std::numeric_limits<double>::infinity()"
> >> >> This (and everything else) probably creates a static initializer if
> >> used
> >> >> for
> >> >> constant values, in C++03. It may be better to use the C macros from
> >> >> <math.h>.
> >> >>
> >> >
> >> Hmm, OK... I'll have to check that out.
> >>
> >
> >
> >
> >  >
> >> >
> >> > https://codereview.chromium.org/474063002/diff/1/mojo/
> >> > public/tools/bindings/pylib/mojom/parse/lexer.py
> >> >
> >> >> File mojo/public/tools/bindings/pylib/mojom/parse/lexer.py (right):
> >> >>
> >> >
> >> >
> >> > https://codereview.chromium.org/474063002/diff/1/mojo/
> >> > public/tools/bindings/pylib/mojom/parse/lexer.py#newcode148
> >> >
> >> >> mojo/public/tools/bindings/pylib/mojom/parse/lexer.py:148:
> >> >> exponent_part+'?)|([0-9]+'+exponent_part+'))|[-+]?Inf|NaN)'
> >> >> Currently, for numbers, we use unary minus (and plus), so that "- 123"
> >> is
> >> >> allowed (instead of only allowing "-123").
> >> >>
> >> >
> >> >  (I have mixed feelings about allowing the space, but it's consistent
> >> with
> >> >> most
> >> >> languages.)
> >> >>
> >> >
> >> >  We should probably re-use that for +/- infinity. (Also, this might have
> >> >> the
> >> >>
> >> > side
> >> >
> >> >> effect of allowing "- -Inf" and maybe even "--Inf", etc.)
> >> >>
> >> >
> >> >
> >> Good thinking.
> >>
> >
> >
> >
> >  >
> >> >
> >> > https://codereview.chromium.org/474063002/
> >> >
> >>
> >
> >  To unsubscribe from this group and stop receiving emails from it, send an
> >>
> > email
> >
> >> to mailto:chromium-reviews+unsubscribe@chromium.org.
> >>
> >
> >
> >
> > https://codereview.chromium.org/474063002/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[darin (slow to review), 2014-08-20 22:25:16]

Now with double.INFINITY, etc.

PTAL

[viettrungluu, 2014-08-21 16:42:19]

https://codereview.chromium.org/474063002/diff/40001/mojo/public/tools/bindin...
File mojo/public/tools/bindings/pylib/mojom/parse/parser.py (right):

https://codereview.chromium.org/474063002/diff/40001/mojo/public/tools/bindin...
mojo/public/tools/bindings/pylib/mojom/parse/parser.py:322: |
identifier_wrapped"""
If you didn't add a lexer token for these, wouldn't these still be accepted as
identifier(_wrapped)? And shouldn't we just use that mechanism?

[darin (slow to review), 2014-08-21 17:08:50]

Hmm, yes... I could do that!
On Aug 21, 2014 9:42 AM, <viettrungluu@chromium.org> wrote:

>
> https://codereview.chromium.org/474063002/diff/40001/mojo/
> public/tools/bindings/pylib/mojom/parse/parser.py
> File mojo/public/tools/bindings/pylib/mojom/parse/parser.py (right):
>
> https://codereview.chromium.org/474063002/diff/40001/mojo/
> public/tools/bindings/pylib/mojom/parse/parser.py#newcode322
> mojo/public/tools/bindings/pylib/mojom/parse/parser.py:322: |
> identifier_wrapped"""
> If you didn't add a lexer token for these, wouldn't these still be
> accepted as identifier(_wrapped)? And shouldn't we just use that
> mechanism?
>
> https://codereview.chromium.org/474063002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[darin (slow to review), 2014-08-22 21:45:49]

PTAL

[viettrungluu, 2014-08-25 20:43:49]

lgtm

[darin (slow to review), 2014-08-25 23:57:56]

I gave up on negative infinity and NaN since they both cause visual studio fits.
We could revisit in the future if needed. It probably requires some #pragma
commands to suppress the warnings. No big deal, but it didn't seem worth it at
the moment since I only need support for infinity.

[darin (slow to review), 2014-08-25 23:58:07]

The CQ bit was checked by darin@chromium.org

[commit-bot: I haz the power, 2014-08-25 23:59:31]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/darin@chromium.org/474063002/120001

[commit-bot: I haz the power, 2014-08-26 01:54:42]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-26 02:01:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-26 02:01:10]

Try jobs failed on following builders:
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)

[darin (slow to review), 2014-08-26 06:21:18]

The CQ bit was checked by darin@chromium.org

[commit-bot: I haz the power, 2014-08-26 06:22:50]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/darin@chromium.org/474063002/200001

[commit-bot: I haz the power, 2014-08-26 07:57:54]

Committed patchset #11 (200001) as 718cbac44d133415cf3586ff536f61c985205fca

[commit-bot: I haz the power, 2014-09-10 02:41:19]

Patchset 11 (id:??) landed as
https://crrev.com/d9f2bcd21b589f5b60f474cf0fbcf54420105266
Cr-Commit-Position: refs/heads/master@{#291864}