Summary: | Finish auditing call sites of upper() and lower(), eliminate many, and rename the functions | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Darin Adler <darin> | ||||||||||||
Component: | WebCore Misc. | Assignee: | Darin Adler <darin> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | achristensen, andersca, benjamin, cdumez, commit-queue, kling, ossy, peavo, rniwa, sam | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Darin Adler
2016-02-04 19:13:21 PST
Created attachment 270747 [details]
Patch
Attachment 270747 [details] did not pass style-queue:
ERROR: Source/WebKit/win/Plugins/PluginPackage.h:43: Code inside a namespace should not be indented. [whitespace/indent] [4]
ERROR: Source/WebKit/mac/Plugins/WebBasePluginPackage.mm:223: Missing space around : in range-based for statement [whitespace/colon] [4]
ERROR: Source/WebCore/platform/text/LocaleToScriptMappingDefault.cpp:167: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebCore/platform/text/LocaleToScriptMappingDefault.cpp:394: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 4 in 54 files
If any of these errors are false positives, please file a bug against check-webkit-style.
This patch wraps up upper and lower. After this, will return to equalIgnoringCase, contains, startsWith, endsWith. Then, done with case, will move on to the various cases of whitespace checking and stripping that should be using isHTMLSpace rather than Unicode concept of whitespace. Two of the style complaints are false positives due to check-webkit-style not understanding our formatting for lambdas (bug already filed long ago). One is a false positive due to the script not understanding of an Objective-C modern for loop that just happens to have a colon somewhere in the expression on the right side of the "in" (no bug filed yet). The fourth is not technically a false positive: a request to change the indenting of an entire header file which I won't do in this patch. Created attachment 270748 [details]
Patch
Attachment 270748 [details] did not pass style-queue:
ERROR: Source/WebKit/win/Plugins/PluginPackage.h:43: Code inside a namespace should not be indented. [whitespace/indent] [4]
ERROR: Source/WebKit/mac/Plugins/WebBasePluginPackage.mm:223: Missing space around : in range-based for statement [whitespace/colon] [4]
ERROR: Source/WebCore/platform/text/LocaleToScriptMappingDefault.cpp:167: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebCore/platform/text/LocaleToScriptMappingDefault.cpp:394: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 4 in 55 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 270749 [details]
Patch
Attachment 270749 [details] did not pass style-queue:
ERROR: Source/WebKit/win/Plugins/PluginPackage.h:43: Code inside a namespace should not be indented. [whitespace/indent] [4]
ERROR: Source/WebKit/mac/Plugins/WebBasePluginPackage.mm:223: Missing space around : in range-based for statement [whitespace/colon] [4]
ERROR: Source/WebCore/platform/text/LocaleToScriptMappingDefault.cpp:167: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebCore/platform/text/LocaleToScriptMappingDefault.cpp:394: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 4 in 55 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 270796 [details]
Patch
Attachment 270796 [details] did not pass style-queue:
ERROR: Source/WebKit/win/Plugins/PluginPackage.h:43: Code inside a namespace should not be indented. [whitespace/indent] [4]
ERROR: Source/WebKit/mac/Plugins/WebBasePluginPackage.mm:223: Missing space around : in range-based for statement [whitespace/colon] [4]
ERROR: Source/WebCore/platform/text/LocaleToScriptMappingDefault.cpp:167: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebCore/platform/text/LocaleToScriptMappingDefault.cpp:394: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 4 in 55 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 270798 [details]
Patch
Attachment 270798 [details] did not pass style-queue:
ERROR: Source/WebKit/win/Plugins/PluginPackage.h:43: Code inside a namespace should not be indented. [whitespace/indent] [4]
ERROR: Source/WebKit/mac/Plugins/WebBasePluginPackage.mm:223: Missing space around : in range-based for statement [whitespace/colon] [4]
ERROR: Source/WebCore/platform/text/LocaleToScriptMappingDefault.cpp:167: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebCore/platform/text/LocaleToScriptMappingDefault.cpp:394: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 4 in 55 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 270798 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270798&action=review > Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:250 > + DEPRECATED_DEFINE_STATIC_LOCAL(HostsSet, hosts, ()); As long as you are touching this, might be nice to upgrade it to use NeverDestroyed (In reply to comment #13) > > Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:250 > > + DEPRECATED_DEFINE_STATIC_LOCAL(HostsSet, hosts, ()); > > As long as you are touching this, might be nice to upgrade it to use > NeverDestroyed I had done that in my original patch. And the build failed on GTK and EFL because of something wrong with the includes, so I undid it and rolled it back for now. Committed r196223: <http://trac.webkit.org/changeset/196223> It broke the WinCairo build. (In reply to comment #16) > It broke the WinCairo build. Not surprising since there is no EWS for it. Could you point me to the specific breakage to make it faster to fix it? (In reply to comment #17) > (In reply to comment #16) > > It broke the WinCairo build. > > Not surprising since there is no EWS for it. Could you point me to the > specific breakage to make it faster to fix it? https://build.webkit.org/builders/WinCairo%2064-Bit%20Release/builds/53680 Otherwise the WinCairo bot is completely broken now due to bug153876 That is a mysterious failure. It’s compiling WTFString.cpp and unable to find some functions from StringImpl.h that are clearly present. Maybe the WinCairo build system has some dependency problem and it’s trying to build with an old StringImpl.h before those functions were added. Pretty sure that problem is a bug with the build system in WinCairo. I won’t be able to fix it. Someone who is actively maintaining that port can fix it and I will be happy to advise. Note that the error message cites a forward declaration of StringImpl in PrintStream.h, not the real definition of StringImpl from StringImpl.h. |