Kasper, thanks a lot for review! Responses inline. On Wed, Jun 24, 2009 at 9:34 PM, Kasper Lund<[email protected]> wrote: > LGTM. > > I'm okay with this as is, but maybe it would make sense to move it to > conversions.cc
I'm perfectly fine with it---I definitely lack the overall understanding of the project structure and therefore tried to retain the constant in header to allow to include into other files if such a need exists. If not, it'd be wiser indeed to put definition of infinity into conversion.cc itself. > and avoid redefining it for windows by doing something > ala: > > #ifdef WIN32 > #define V8_INFINITY HUGE_VALUE > #else > #define V8_INFINITY INFINITY > #endif > > <<check for weird gcc version and redefine if needed>> Up to you once again. I just wanted to make both redefinitions to look similar to ease comprehension (like some other guy may ask why there is a difference). YMMV of course. > > Another more radical approach would be to try to use > std::numeric_limits<double>::infinity() for all platforms, but I don't > know if it's supported everywhere. I would definitely wait with this > part though and fix the bleeding_edge build bustage as soon as > possible. Neither do I. yours, anton. > On Wed, Jun 24, 2009 at 7:24 PM, <[email protected]> wrote: >> Reviewers: Kasper Lund, >> >> Description: >> Do not use common INFINITY name as it might be overriden by some other >> include file. Let's use >> project specific name. >> >> >> Please review this at http://codereview.chromium.org/146096 >> >> SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ >> >> Affected files: >> M src/conversions.cc >> M src/platform.h >> >> >> Index: src/conversions.cc >> =================================================================== >> --- src/conversions.cc (revision 2265) >> +++ src/conversions.cc (working copy) >> @@ -327,7 +327,7 @@ >> index++; >> if (!SubStringEquals(str, index, "Infinity")) >> return JUNK_STRING_VALUE; >> - result = is_negative ? -INFINITY : INFINITY; >> + result = is_negative ? -V8_INFINITY : V8_INFINITY; >> index += 8; >> } >> } >> Index: src/platform.h >> =================================================================== >> --- src/platform.h (revision 2265) >> +++ src/platform.h (working copy) >> @@ -44,6 +44,8 @@ >> #ifndef V8_PLATFORM_H_ >> #define V8_PLATFORM_H_ >> >> +#define V8_INFINITY INFINITY >> + >> // Windows specific stuff. >> #ifdef WIN32 >> >> @@ -58,7 +60,8 @@ >> FP_NORMAL >> }; >> >> -#define INFINITY HUGE_VAL >> +#undef V8_INFINITY >> +#define V8_INFINITY HUGE_VAL >> >> namespace v8 { >> namespace internal { >> @@ -100,8 +103,8 @@ >> // __GNUC_PREREQ is not defined in GCC for Mac OS X, so we define our own >> macro >> #if __GNUC_VERSION__ >= 29600 && __GNUC_VERSION__ < 40100 >> #include <limits> >> -#undef INFINITY >> -#define INFINITY std::numeric_limits<double>::infinity() >> +#undef V8_INFINITY >> +#define V8_INFINITY std::numeric_limits<double>::infinity() >> #endif >> >> #endif // __GNUC__ >> >> >> > --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
