Dealing with the recursive INT_MAX et al. defs on MSVC 2008e

11 views
Skip to first unread message

Ryan Norton

unread,
Mar 11, 2011, 10:57:33 AM3/11/11
to Chromium-dev
These get flagged as errors on some projects due to the warnings as
errors flag. Most sites I've seen just recommend turning off the flag
due to this for MSVC through the gypi files, but a dev or two on IRC
didn't really like that. So, I've been trying to dig through and come
up with a somewhat elegant solution.

It's rough because the base problem is the windows.h itself doesn't
include intsafe.h (which has it's own INT_MAX etc. defs that aren't
wrapped around conditionals), but the atl headers do - you're SUPPOSED
to include the atl headers before anything else, but quite a few files
aren't following this rule.

Sort of a sum up:
1) ICU has it's own INT_MAX et al. that are wrapped around
conditionals, but intsafe.h gets pulled in through the atl headers
afterwards it will define them again.
2) Native Client also has it's own INT_MAX et al. and trigger warnings
and cause the same issue as ICU, and then some since it's versions
aren't wrapped around the appropriate conditionals.

Here's a somewhat workable patch combo:

Index: base/port.h
===================================================================
--- base/port.h (revision 77802)
+++ base/port.h (working copy)
@@ -52,4 +52,26 @@
#define API_CALL
#endif

+/* Pull in XX_MAX, i.e. INT8_MAX from the platform SDK, before
anything
+ else. Third party libs have them wrapped, but platform SDK ones
aren't
+ and the ATL headers pull them in (windows.h doesn't as of SDK 7.6/
A).
+ This prevents conflicts when including ICU before ATL
headers. */
+#if defined(OS_WIN) && defined(COMPILER_MSVC) && !
defined(_INTSAFE_H_INCLUDED_)
+
+/* Cleanup if the ICU headers were included before this */
+#ifdef __UMACHINE_H__
+# undef INT8_MIN
+# undef INT16_MIN
+# undef INT32_MIN
+# undef INT8_MAX
+# undef INT16_MAX
+# undef INT32_MAX
+# undef UINT8_MAX
+# undef UINT16_MAX
+# undef UINT32_MAX
+#endif
+
+#include <intsafe.h>
+#endif
+
#endif // BASE_PORT_H_

-------------
Native Client
--------------
Index: src/include/win/port_win.h
===================================================================
--- src/include/win/port_win.h (revision 4382)
+++ src/include/win/port_win.h (working copy)
@@ -78,21 +78,45 @@
#endif /* _WIN64 */


-#if !defined(__cplusplus) || defined(__STDC_LIMIT_MACROS)
+#if ( !defined(__cplusplus) || defined(__STDC_LIMIT_MACROS) )
/* only what we need so far */
+#ifndef UINT8_MAX
# define UINT8_MAX NACL_UMAX_VAL(uint8_t)
+#endif
+#ifndef INT8_MAX
# define INT8_MAX NACL_MAX_VAL(int8_t)
+#endif
+#ifndef INT8_MIN
# define INT8_MIN NACL_MIN_VAL(int8_t)
+#endif
+#ifndef UINT16_MAX
# define UINT16_MAX NACL_UMAX_VAL(uint16_t)
+#endif
+#ifndef INT16_MAX
# define INT16_MAX NACL_MAX_VAL(int16_t)
+#endif
+#ifndef INT16_MIN
# define INT16_MIN NACL_MIN_VAL(int16_t)
+#endif
+#ifndef UINT32_MAX
# define UINT32_MAX NACL_UMAX_VAL(uint32_t)
+#endif
+#ifndef INT32_MAX
# define INT32_MAX NACL_MAX_VAL(int32_t)
+#endif
+#ifndef INT32_MIN
# define INT32_MIN NACL_MIN_VAL(int32_t)
+#endif
+#ifndef UINT64_MAX
# define UINT64_MAX NACL_UMAX_VAL(uint64_t)
+#endif
+#ifndef INT64_MAX
# define INT64_MAX NACL_MAX_VAL(int64_t)
+#endif
+#ifndef INT64_MIN
# define INT64_MIN NACL_MIN_VAL(int64_t)
#endif
+#endif

EXTERN_C_BEGIN

=============================

This combo gets rid of the warnings that get flagged as errors and
gives you a build that compiles, but the first patch it isn't ideal
for obvious reasons (it gets compiled in on every platform despite
being win specific, etc.), could probably nix the STDC/cplusplus
conditional wrap in the native client one as well. A more elegant
solution I've been thinking about is just creating a shim header that
includes atlbase.h and handles the logic there and replacing the in-
project atlbase.h occurances with that - maybe someone here knows a
better idea though. Of course, the ideal solution is to simply change
the include order of the atl headers - but the hierarchy is fairly
involved.
Reply all
Reply to author
Forward
0 new messages