Bug 88307 - Rename UTF8.h/cpp to WTF_UTF8.h/cpp to avoid clashes with ICU's utf8.h
Summary: Rename UTF8.h/cpp to WTF_UTF8.h/cpp to avoid clashes with ICU's utf8.h
Status: RESOLVED DUPLICATE of bug 70913
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 420+
Hardware: All All
: P1 Blocker
Assignee: Csaba Osztrogonác
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks: 88300
  Show dependency treegraph
 
Reported: 2012-06-05 01:19 PDT by Csaba Osztrogonác
Modified: 2012-06-13 08:32 PDT (History)
5 users (show)

See Also:


Attachments
proposed patch (57.74 KB, patch)
2012-06-05 08:39 PDT, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2012-06-05 01:19:04 PDT
- U8_MAX_LENGHT doesn't exist in Source/WebCore/platform/text/TextCodecUTF8.h
- U8_APPEND_UNSAFE doesn't exist in Source/WebCore/platform/text/TextCodecUTF8.cpp

It must be a missing include problem, because they are defined in ICU somewhere.
Comment 1 Csaba Osztrogonác 2012-06-05 07:24:16 PDT
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
Comment 2 Csaba Osztrogonác 2012-06-05 08:06:33 PDT
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?
Comment 3 Csaba Osztrogonác 2012-06-05 08:11:05 PDT
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 ...
Comment 4 Csaba Osztrogonác 2012-06-05 08:19:05 PDT
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. :-/
Comment 5 Csaba Osztrogonác 2012-06-05 08:39:46 PDT
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 :) )
Comment 6 WebKit Review Bot 2012-06-05 08:42:07 PDT
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.
Comment 7 Raphael Kubo da Costa (:rakuco) 2012-06-07 11:40:40 PDT
Should bug 70913 be marked as a duplicate of this one now?
Comment 8 Balazs Kelemen 2012-06-07 14:03:12 PDT
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.
Comment 9 Csaba Osztrogonác 2012-06-08 00:20:52 PDT
(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 ***