Use isfinite instead of finite on Mac

Use isfinite instead of finite on Mac

According to math.h in the 10.6 SDK, finite is deprecated in favor of isfinite, and finite isn't available on iOS.

BUG=None
TEST=None


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=145876

[rohitrao (ping after 24h), 2012-07-10 11:57:36]

https://chromiumcodereview.appspot.com/10704126/diff/2001/base/float_util.h
File base/float_util.h (right):

https://chromiumcodereview.appspot.com/10704126/diff/2001/base/float_util.h#n...
base/float_util.h:22: // C99 says isfinite() replaceds finite(), and iOS does
not provide the
Typo: replaceds

https://chromiumcodereview.appspot.com/10704126/diff/2001/base/float_util.h#n...
base/float_util.h:26: return finite(number) != 0;
Can we just call isfinite() on all POSIX systems, or is that method not
guaranteed to exist?

[stuartmorgan, 2012-07-10 12:04:33]

https://chromiumcodereview.appspot.com/10704126/diff/2001/base/float_util.h
File base/float_util.h (right):

https://chromiumcodereview.appspot.com/10704126/diff/2001/base/float_util.h#n...
base/float_util.h:22: // C99 says isfinite() replaceds finite(), and iOS does
not provide the
On 2012/07/10 11:57:36, rohitrao wrote:
> Typo: replaceds

Done. (It's almost as if TVL wrote this comment originally ;) )

https://chromiumcodereview.appspot.com/10704126/diff/2001/base/float_util.h#n...
base/float_util.h:26: return finite(number) != 0;
On 2012/07/10 11:57:36, rohitrao wrote:
> Can we just call isfinite() on all POSIX systems, or is that method not
> guaranteed to exist?

I have no idea. Given all the flavors that we semi-support and I have no way of
testing, I'd be afraid to change it.

[Mark Mentovai, 2012-07-10 12:19:29]

https://chromiumcodereview.appspot.com/10704126/diff/6001/base/float_util.h
File base/float_util.h (right):

https://chromiumcodereview.appspot.com/10704126/diff/6001/base/float_util.h#n...
base/float_util.h:26: return finite(number) != 0;
If you’re afraid to change this, and I’m not, but IF you are…

Another alternative is

#if defined(OS_POSIX)
#include <cmath>
#endif

inline bool IsFinite(const double& number) {
#if defined(OS_POSIX)
  return std::isfinite(number) != 0;
#endif
}

and then I think you can drop <ieeefp.h> too.

[rohitrao (ping after 24h), 2012-07-10 12:27:06]

What if we commit this more or less as-is and then unify all of the POSIX
codepaths in a followup CL?  That way if solaris ends up breaking, the revert
won't affect our upstreaming.  (We've got a bunch of CLs coming that depend on
this change, so reverting this would break everything else.)

In general, I think we should unify if we can.  I can write the followup CL.

[stuartmorgan, 2012-07-10 12:27:30]

See, but then I read something like http://www.alecjacobson.com/weblog/?p=1768
and I just don't want to thrash several times, then have someone helpfully fix
Solaris and break us again, etc.

[stuartmorgan, 2012-07-10 12:27:57]

On 2012/07/10 12:27:06, rohitrao wrote:
> What if we commit this more or less as-is and then unify all of the POSIX
> codepaths in a followup CL?  That way if solaris ends up breaking, the revert
> won't affect our upstreaming.  (We've got a bunch of CLs coming that depend on
> this change, so reverting this would break everything else.)

This.

[Mark Mentovai, 2012-07-10 12:35:26]

Whatever.

But note that nobody’s going to revert a change that fixes what’s becoming an
important Chrome platform (iOS) for an unimportant one (Solaris). Sorry,
Solaris.

[Mark Mentovai, 2012-07-10 12:37:36]

stuartmorgan@chromium.org wrote:

> On 2012/07/10 12:27:06, rohitrao wrote:
>
>> What if we commit this more or less as-is and then unify all of the POSIX
>> codepaths in a followup CL?  That way if solaris ends up breaking, the
>> revert
>> won't affect our upstreaming.  (We've got a bunch of CLs coming that
>> depend on
>> this change, so reverting this would break everything else.)
>>
>
> This.
>

This what?

[stuartmorgan, 2012-07-10 12:40:02]

On 2012/07/10 12:35:26, Mark Mentovai wrote:
> Whatever.

Can you give a 4-letter spelling of that? ;)

> But note that nobody’s going to revert a change that fixes what’s becoming an
> important Chrome platform (iOS) for an unimportant one (Solaris). Sorry,
> Solaris.

But it seems weird to land a fix that we basically know will break Solaris when
we have a version that doesn't...

[stuartmorgan, 2012-07-10 12:42:04]

On 2012/07/10 12:37:36, Mark Mentovai wrote:
> This what?

If we really want to make all of POSIX have the same isinfinite codepath, I'd
like to do one CL that fixes iOS in a non-destructive way, and then a follow-up
that unifies everything in the naive hope that it will work on all flavors, so
that someone can revert that second one if they need to without affecting iOS.

[Mark Mentovai, 2012-07-10 12:44:16]

LGTM

[Mark Mentovai, 2012-07-10 12:45:15]

stuartmorgan wrote:
> On 2012/07/10 12:37:36, Mark Mentovai wrote:
> > This what?
> 
> If we really want to make all of POSIX have the same isinfinite codepath, I'd
> like to do one CL that fixes iOS in a non-destructive way, and then a
follow-up
> that unifies everything in the naive hope that it will work on all flavors, so
> that someone can revert that second one if they need to without affecting iOS.

Oh, I see what you meant now. You meant “this” as a less eyeball-offending
version of +1, but I took it as the beginning of an aborted sentence.

[commit-bot: I haz the power, 2012-07-10 12:51:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stuartmorgan@chromium.org/10704126/6001

[commit-bot: I haz the power, 2012-07-10 14:11:39]

Change committed as 145876