Bug 189693 - Rename <wtf/unicode/UTF8.h> to <wtf/unicode/UTF8Conversion.h> in order to avoid conflicting with ICU's unicode/utf8.h
Summary: Rename <wtf/unicode/UTF8.h> to <wtf/unicode/UTF8Conversion.h> in order to avo...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords: InRadar
Depends on:
Blocks: 171618
  Show dependency treegraph
 
Reported: 2018-09-18 00:00 PDT by Fujii Hironori
Modified: 2018-11-04 21:54 PST (History)
4 users (show)

See Also:


Attachments
Patch (58.12 KB, patch)
2018-11-01 00:15 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2018-09-18 00:00:27 PDT
[Win][Clang] warning: #include resolved using non-portable Microsoft search rules as: ..\..\Source\WTF\wtf/unicode/utf8.h

While doing Bug 171618, a lot of following compilation warnings are reported.

> [1033/6181] Building CXX object Source\WTF\wtf\CMakeFiles\WTF.dir\ParkingLot.cpp.obj
> In file included from ..\..\Source\WTF\wtf\ParkingLot.cpp:31:
> In file included from ..\..\Source\WTF\wtf/StringPrintStream.h:31:
> In file included from ..\..\Source\WTF\wtf/text/WTFString.h:32:
> In file included from ..\..\Source\WTF\wtf/text/StringImpl.h:27:
> In file included from ..\..\WebKitLibraries\win\include\unicode/ustring.h:21:
> In file included from ..\..\WebKitLibraries\win\include\unicode/utypes.h:44:
> ..\..\WebKitLibraries\win\include\unicode/utf.h(217,10):  warning: #include resolved using non-portable Microsoft search rules as: ..\..\Source\WTF\wtf/unicode/utf8.h [-Wmicrosoft-include]
> #include "unicode/utf8.h"
>          ^
> In file included from ..\..\Source\WTF\wtf\ParkingLot.cpp:31:
> In file included from ..\..\Source\WTF\wtf/StringPrintStream.h:31:
> In file included from ..\..\Source\WTF\wtf/text/WTFString.h:32:
> In file included from ..\..\Source\WTF\wtf/text/StringImpl.h:27:
> In file included from ..\..\WebKitLibraries\win\include\unicode/ustring.h:21:
> In file included from ..\..\WebKitLibraries\win\include\unicode/utypes.h:44:
> In file included from ..\..\WebKitLibraries\win\include\unicode/utf.h:221:
> ..\..\WebKitLibraries\win\include\unicode/utf_old.h(166,10):  warning: #include resolved using non-portable Microsoft search rules as: ..\..\Source\WTF\wtf/unicode/utf8.h [-Wmicrosoft-include]
> #include "unicode/utf8.h"
>          ^
> 2 warnings generated.


See Also:

Bug 70913 – using extrernal ICU library on case unsensitive drives will not work
Bug 88307 – Rename UTF8.h/cpp to WTF_UTF8.h/cpp to avoid clashes with ICU's utf8.h
Comment 1 Fujii Hironori 2018-09-18 00:17:20 PDT
https://msdn.microsoft.com/en-us/library/36k2cdd4.aspx

> For include files that are specified as #include "path-spec",
> directory searching begins with the directory of the parent file
> and then proceeds through the directories of any grandparent
> files. That is, searching begins relative to the directory that
> contains the source file that contains the #include directive
> that's being processed. If there is no grandparent file and the
> file has not been found, the search continues as if the file name
> were enclosed in angle brackets.
Comment 2 Don Olmstead 2018-09-25 15:08:32 PDT
Isn't the larger problem that Source/WTF/wtf is being used as an include directory?
Comment 3 Fujii Hironori 2018-09-25 18:14:47 PDT
(In reply to Don Olmstead from comment #2)
> Isn't the larger problem that Source/WTF/wtf is being used as an include
> directory?

It is another problem should be fixed. But, it is not the main problem.

The main problem is the MSVC quirks (Comment 1).
It happens in following conditions:

1. ICU's header unicode/utf.h has '#include "unicode/utf8.h"' not '<unicode/utf8.h>'
2. ParkingLot.cpp indirectly includes unicode/utf.h
3. There is unicode/UTF8.h in WTF relative to the grand-grand-... parent ParkingLot.cpp
Comment 4 Fujii Hironori 2018-10-30 18:47:06 PDT
This is not a Clang-cl warning problem, but also MSVC builds have this issue.

Here is the build log of compiling ParkingLot.cpp with /showIncludes in WebKit WinCairo port with MSVC.
https://gist.github.com/fujii/a4a72fd3581e6b38962706b60e77d75c

> 1>Note: including file:      C:\webkit\ga\WebKitLibraries\win\include\unicode/utf.h
> 1>Note: including file:       c:\webkit\ga\source\wtf\wtf\unicode/utf8.h

wtf/unicode/UTF8.h is included by unicode/utf.h.
Comment 5 Fujii Hironori 2018-11-01 00:15:53 PDT
Created attachment 353584 [details]
Patch
Comment 6 Build Bot 2018-11-01 00:18:24 PDT
Attachment 353584 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:28:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:29:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.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]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:122:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:221:  One line control clauses should not use braces.  [whitespace/braces] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:234:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:234:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:235:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:236:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:247:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:254:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:256:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:256:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:257:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:257:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:258:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:262:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:262:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:263:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:264:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:265:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:266:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:270:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:288:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:288:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:289:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:290:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:291:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:292:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:330:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.cpp:414:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.h:45:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.h:45:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.h:46:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.h:47:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.h:48:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.h:53:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.h:66:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.h:70:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/WTF/wtf/unicode/UTF8Conversion.h:74:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Total errors found: 41 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Yusuke Suzuki 2018-11-01 07:38:02 PDT
Comment on attachment 353584 [details]
Patch

r=me
Comment 8 Fujii Hironori 2018-11-01 18:14:13 PDT
Comment on attachment 353584 [details]
Patch

Clearing flags on attachment: 353584

Committed r237714: <https://trac.webkit.org/changeset/237714>
Comment 9 Fujii Hironori 2018-11-01 18:14:16 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2018-11-01 18:15:24 PDT
<rdar://problem/45749635>