Bug 88307

Here is the exact error log: f:\webkit\source\webcore\platform\text\TextCodecUTF8.h(50) : error C2065: 'U8_MAX_LENGTH' : undeclared identifier f:\WebKit\Source\WebCore\platform\text\TextCodecUTF8.cpp(311) : error C3861: 'U8_APPEND_UNSAFE': identifier not found

U8_MAX_LENGTH is 4 on my Linux machine and defined by /usr/include/unicode/utf8.h. Which is included by /usr/include/unicode/utf.h <--- .... <--- /usr/include/unicode/uchar.h <--- Source/WTF/wtf/unicode/icu/UnicodeIcu.h <--- Source/WTF/wtf/unicode/Unicode.h <--- Source/WebCore/platform/text/TextCodec.h Pffff ... I think I got it after preprocessing the source ... Here is a part of ICU/include/unicode/utf.h: [snip] /* include the utfXX.h ------------------------------------------------------ */ #if !U_NO_DEFAULT_INCLUDE_UTF_HEADERS #include "unicode/utf8.h" ----> HERE is the problem !!! #include "unicode/utf16.h" [end] Unfortunately MSVC includes WebKit/Source/WTF/wtf/unicode/utf8.h here instead of ICU's unicode/utf8.h and it caused all of these problem ... Here I have no idea how can we fix it properly ... How I hate filename clashing ... Any idea?

My collegue pointed out that Source/WTF/wtf/unicode/UTF8.h is uppercase, but ICU/include/utf.8 is lowercase ... So it only clashes on Windows ...

What do you think about renaming UTF8.h/cpp to WTF_UTF8.h/cpp ? (UTF8.h guards itself with WTF_UTF8_h ifdef guard right now.) I have no better idea. :-/

Created attachment 145806 [details] proposed patch Patch for EWS bots. Let's see if renaming works or not. (I bet changes in Mac project files are wrong, because I didn't use XCode :) )

Attachment 145806 [details] did not pass style-queue: Source/WTF/wtf/unicode/WTF_UTF8.cpp:65: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/WTF/wtf/unicode/WTF_UTF8.cpp:122: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WTF/wtf/unicode/WTF_UTF8.cpp:220: One line control clauses should not use braces. [whitespace/braces] [4] Source/WTF/wtf/unicode/WTF_UTF8.cpp:233: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] Source/WTF/wtf/unicode/WTF_UTF8.cpp:233: More than one command on the same line [whitespace/newline] [4] Source/WTF/wtf/unicode/WTF_UTF8.cpp:234: More than one command on the same line [whitespace/newline] [4] Source/WTF/wtf/unicode/WTF_UTF8.cpp:235: More than one command on the same line [whitespace/newline] [4] Source/WTF/wtf/unicode/WTF_UTF8.cpp:246: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WTF/wtf/unicode/WTF_UTF8.cpp:253: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] Source/WTF/wtf/unicode/WTF_UTF8.cpp:255: More than one command on the same line in if [whitespace/parens] [4] Source/WTF/wtf/unicode/WTF_UTF8.cpp:256: More than one command on the same line in if [whitespace/parens] [4] Source/WTF/wtf/unicode/WTF_UTF8.cpp:257: More than one command on the same line in if [whitespace/parens] [4] Source/WTF/wtf/unicode/WTF_UTF8.cpp:261: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] Source/WTF/wtf/unicode/WTF_UTF8.cpp:261: More than one command on the same line in if [whitespace/parens] [4] Source/WTF/wtf/unicode/WTF_UTF8.cpp:262: More than one command on the same line in if [whitespace/parens] [4] Source/WTF/wtf/unicode/WTF_UTF8.cpp:263: More than one command on the same line in if [whitespace/parens] [4] Source/WTF/wtf/unicode/WTF_UTF8.cpp:264: More than one command on the same line in if [whitespace/parens] [4] Source/WTF/wtf/unicode/WTF_UTF8.cpp:265: More than one command on the same line in if [whitespace/parens] [4] Source/WTF/wtf/unicode/WTF_UTF8.cpp:268: More than one command on the same line in if [whitespace/parens] [4] Source/WTF/wtf/unicode/WTF_UTF8.cpp:287: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] Source/WTF/wtf/unicode/WTF_UTF8.cpp:287: More than one command on the same line [whitespace/newline] [4] Source/WTF/wtf/unicode/WTF_UTF8.cpp:288: More than one command on the same line [whitespace/newline] [4] Source/WTF/wtf/unicode/WTF_UTF8.cpp:289: More than one command on the same line [whitespace/newline] [4] Source/WTF/wtf/unicode/WTF_UTF8.cpp:290: More than one command on the same line [whitespace/newline] [4] Source/WTF/wtf/unicode/WTF_UTF8.cpp:291: More than one command on the same line [whitespace/newline] [4] Source/WTF/wtf/unicode/WTF_UTF8.cpp:328: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] Source/WTF/wtf/unicode/WTF_UTF8.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/WTF/wtf/unicode/WTF_UTF8.h:45: One space before end of line comments [whitespace/comments] [5] Source/WTF/wtf/unicode/WTF_UTF8.h:46: One space before end of line comments [whitespace/comments] [5] Source/WTF/wtf/unicode/WTF_UTF8.h:47: One space before end of line comments [whitespace/comments] [5] Source/WTF/wtf/unicode/WTF_UTF8.h:48: One space before end of line comments [whitespace/comments] [5] Source/WTF/wtf/unicode/WTF_UTF8.h:53: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WTF/wtf/unicode/WTF_UTF8.h:79: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/text/String.cpp:27: Alphabetical sorting problem. [build/include_order] [4] SFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSClassRef.cpp',..." exit_code: 1 ource/WTF/wtf/text/WTFString.cpp:33: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 35 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.

Should bug 70913 be marked as a duplicate of this one now?

Comment on attachment 145806 [details] proposed patch I don't like WTF_UTF8.h. This is the same problem as with WTFString.h which does not have underscore, and generally our naming convention does not prefer underscores. I would name it to WTFUTF8.h. A sed on the diff file should be enough :) Furthermore, I would add a comment to the header explaining why has this file such a name.

(In reply to comment #8) > (From update of attachment 145806 [details]) > I don't like WTF_UTF8.h. This is the same problem as with WTFString.h which does not have underscore, and generally our naming convention does not prefer underscores. I would name it to WTFUTF8.h. A sed on the diff file should be enough :) Furthermore, I would add a comment to the header explaining why has this file such a name. Good point. I didn't think about the proper name when I uploaded the patch. First I wanted to test if the EWS bots like it or not. (In reply to comment #7) > Should bug 70913 be marked as a duplicate of this one now? Good to know if there is a bug about it. :) It does what Balázs suggested. :) It is the elder one, so this one should be duplicated. *** This bug has been marked as a duplicate of bug 70913 ***