Bug (icu42) – build error with ICU 4.2.x on Windows + Visual studio

• Component: Web Template Framework
• Status: RESOLVED FIXED

[Jungshik Shin]

When I tried to build Chromium with ICU 4.2.x on Windows, I came across exactly the same error as Peter encountered a couple of month ago ( http://paste.lisp.org/display/81781 ) except that I'm getting it in wtf/unicode/icu/CollatorICU.cpp. 

He fixed the problem by  using Vector<char> instead of std::string.  ( http://trac.webkit.org/changeset/44643 ). 

In this case, I'm afraid that's not an option because unicode/ucol.h pulls in unicode/unistr.h, which in turn pulls in unicode/std_string.h and <string>. On Windows, <string> pulls in <xlocale> with tolower, toupper, etc. They're all replaced with WTF_Please_use_ASCIICType_instead_of_ctype_see_comment_in_ASCIICType_h leading to compile errors (multiple declarations).

One work around the problem is to append "_foo" to WTF_Please_use_ASCIICType_instead_of_ctype_see_comment_in_ASCIICType_h for 'foo' #defined in DisallowCType.h

So, they'll look like:

#define tolower WTF_Please_use_ASCIICType_instead_of_ctype_see_comment_in_ASCIICType_h_tolower
#define toupper WTF_Please_use_ASCIICType_instead_of_ctype_see_comment_in_ASCIICType_h_toupper

[Jungshik Shin]

Oops. I forgot to mention that ICU 4.2.x began to use std::string where it's supported. Chromium has  used ICU 3.8 and I'm about to upgrade it to ICU 4.2.1. I guess Safari will eventually move to ICU 4.2.x or later (Win Safari uses ICU 4.0, right?) even on OS X (it's almost certain that Mac OS 10.7 - after snow leopard - will have at least ICU 4.2.x). 

If the approach mentioned in the previous comment is acceptable, I'll make a patch.

[Jungshik Shin]

Created attachment 35126 [details]
patch : prepend WTF_...ASCIIType_h with 'isXXX' / 'toXXX'

[Eric Seidel (no email)]

I'm totally confused by this fix.  Why?

[Jungshik Shin]

Let me try to clarify. 

CollatorICU.cpp (in Javascript/wtf/unicode/icu) includes ICU's i18n/unicode/ucol.h. Beginning with ICU 4.2, ucol.h pulls in  C++ standard <string> (not directly but via a chain of includes). 

<string> in turn pulls in <xlocale> (again indirectly). <xlocale> in Visual Studio has the following at line 1386

        _Elem __CLR_OR_THIS_CALL toupper(_Elem _Ch) const
                {       // convert element to upper case
                return (do_toupper(_Ch));
                }

At line 1375, it has 

        _Elem __CLR_OR_THIS_CALL tolower(_Elem _Ch) const
                {       // convert element to upper case
                return (do_toupper(_Ch));
                }


Because DisallowCType.h defines tolower and toupper (and other ctype macros/functions) to WTF_Please_use_ASCIICType_instead_of_ctype_see_comment_in_ASCIICType_h, the  above two end up being identical and leading to the errors in http://paste.lisp.org/display/81781  

toupper and tolower show up multiple times in 
<xlocale> (in "Microsoft Visual Studio 8\VC\include") so that there are multiple errors as shown in http://paste.lisp.org/display/81781

[Jungshik Shin]

(In reply to comment #4)

> At line 1375, it has 
> 
>         _Elem __CLR_OR_THIS_CALL tolower(_Elem _Ch) const
>                 {       // convert element to upper case
>                 return (do_toupper(_Ch));
>                 }

Ooops. The above should read

        _Elem __CLR_OR_THIS_CALL tolower(_Elem _Ch) const
                {       // convert element to lower case
                return (do_tolower(_Ch));
                }

[Darin Adler]

Comment on attachment 35126 [details]
patch : prepend WTF_...ASCIIType_h with 'isXXX' / 'toXXX'

This seems like an OK way to sidestep the build failure. Not a great solution, but something that will do no harm.

r=me

[Jungshik Shin]

Thank you for r+. Just checked in ( http://trac.webkit.org/changeset/47516 )