WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED DUPLICATE of
bug 70913
88307
Rename UTF8.h/cpp to WTF_UTF8.h/cpp to avoid clashes with ICU's utf8.h
https://bugs.webkit.org/show_bug.cgi?id=88307
Summary
Rename UTF8.h/cpp to WTF_UTF8.h/cpp to avoid clashes with ICU's utf8.h
Csaba Osztrogonác
Reported
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.
Attachments
proposed patch
(57.74 KB, patch)
2012-06-05 08:39 PDT
,
Csaba Osztrogonác
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Csaba Osztrogonác
Comment 1
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
Csaba Osztrogonác
Comment 2
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?
Csaba Osztrogonác
Comment 3
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 ...
Csaba Osztrogonác
Comment 4
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. :-/
Csaba Osztrogonác
Comment 5
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 :) )
WebKit Review Bot
Comment 6
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.
Raphael Kubo da Costa (:rakuco)
Comment 7
2012-06-07 11:40:40 PDT
Should
bug 70913
be marked as a duplicate of this one now?
Balazs Kelemen
Comment 8
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.
Csaba Osztrogonác
Comment 9
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
***
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug