RESOLVED FIXED 192742
Replace many uses of String::format with more type-safe alternatives 
https://bugs.webkit.org/show_bug.cgi?id=192742
Summary Replace many uses of String::format with more type-safe alternatives
Darin Adler
Reported 2018-12-15 10:10:51 PST
Replace many uses of String::format with more type-safe alternatives
Attachments
Patch (70.99 KB, patch)
2018-12-15 12:18 PST, Darin Adler 		no flags
DetailsFormatted DiffDiff
Patch (134.93 KB, patch)
2018-12-15 14:58 PST, Darin Adler 		no flags
DetailsFormatted DiffDiff
Patch (131.51 KB, patch)
2019-01-26 11:29 PST, Darin Adler 		no flags
DetailsFormatted DiffDiff
Patch (132.90 KB, patch)
2019-01-27 16:30 PST, Darin Adler 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Darin Adler
Comment 1 2018-12-15 12:18:35 PST Comment hidden (obsolete)
EWS Watchlist
Comment 2 2018-12-15 12:23:54 PST Comment hidden (obsolete)
Darin Adler
Comment 3 2018-12-15 14:58:42 PST
Created attachment 357406 [details] Patch
Mark Lam
Comment 4 2018-12-15 15:48:19 PST
Comment on attachment 357406 [details] Patch r=me
Darin Adler
Comment 5 2018-12-15 16:09:39 PST
Committed r239254: <https://trac.webkit.org/changeset/239254>
Radar WebKit Bug Importer
Comment 6 2018-12-15 16:10:31 PST
<rdar://problem/46757369>
Matt Lewis
Comment 7 2018-12-17 10:39:23 PST
This looks to have broken the windows 10 debug build: https://build.webkit.org/builders/Apple%20Win%2010%20Debug%20%28Build%29/builds/1011/steps/compile-webkit/logs/stdio https://build.webkit.org/builders/Apple%20Win%2010%20Debug%20%28Build%29/builds/997 error: wtf\text\stringconcatenate.h(314): error C2027: use of undefined type 'WTF::StringTypeAdapter<uint64_t,void>' revision before builds, but this one doesn't
Matt Lewis
Comment 8 2018-12-17 10:45:25 PST
Reverted r239254 for reason: This broke the Windows 10 Debug build Committed r239273: <https://trac.webkit.org/changeset/239273>
Darin Adler
Comment 9 2018-12-17 11:22:43 PST
That error can be fixed by adding an include of StringConcatenateNumbers.h to whatever source file failed to build, too bad we had to roll it out instead.
Michael Catanzaro
Comment 10 2018-12-17 14:15:18 PST
This is closely-related to bug #188191.
Darin Adler
Comment 11 2019-01-25 15:22:51 PST
I don’t know where to add the include because the above comment doesn’t show enough context for me to know which file failed to compile.
Ryan Haddad
Comment 12 2019-01-25 15:30:15 PST
(In reply to Darin Adler from comment #11) > I don’t know where to add the include because the above comment doesn’t show > enough context for me to know which file failed to compile. It looks like the log from win-ews is still accessible: https://webkit-queues.webkit.org/results/10421570 C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\DerivedSources\ForwardingHeaders\wtf/text/StringConcatenate.h(314): error C2027: use of undefined type 'WTF::StringTypeAdapter<uint64_t,void>' (compiling source file C:\cygwin\home\buildbot\WebKit\Source\WebCore\PAL\pal\FileSizeFormatter.cpp) [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebCore\PAL\pal\PAL.vcxproj] C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\DerivedSources\ForwardingHeaders\wtf/text/StringConcatenate.h(314): note: see declaration of 'WTF::StringTypeAdapter<uint64_t,void>' (compiling source file C:\cygwin\home\buildbot\WebKit\Source\WebCore\PAL\pal\FileSizeFormatter.cpp) C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\DerivedSources\ForwardingHeaders\wtf/text/StringConcatenate.h(327): note: see reference to function template instantiation 'WTF::String WTF::tryMakeString<uint64_t,const char*>(uint64_t,const char *)' being compiled (compiling source file C:\cygwin\home\buildbot\WebKit\Source\WebCore\PAL\pal\FileSizeFormatter.cpp) C:\cygwin\home\buildbot\WebKit\Source\WebCore\PAL\pal\FileSizeFormatter.cpp(36): note: see reference to function template instantiation 'WTF::String WTF::makeString<uint64_t,const char*>(uint64_t,const char *)' being compiled C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\DerivedSources\ForwardingHeaders\wtf/text/StringConcatenate.h(314): error C2672: 'tryMakeStringFromAdapters': no matching overloaded function found (compiling source file C:\cygwin\home\buildbot\WebKit\Source\WebCore\PAL\pal\FileSizeFormatter.cpp) [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebCore\PAL\pal\PAL.vcxproj]
Darin Adler
Comment 13 2019-01-26 11:15:32 PST
Thanks. Looks like it’s FileSizeFormatter.cpp.
Darin Adler
Comment 14 2019-01-26 11:29:18 PST Comment hidden (obsolete)
Darin Adler
Comment 15 2019-01-27 16:30:36 PST
Created attachment 360311 [details] Patch
Darin Adler
Comment 16 2019-01-27 17:56:35 PST
Committed r240557: <https://trac.webkit.org/changeset/240557>
Michael Catanzaro
Comment 17 2019-01-27 19:02:32 PST
It broke the WPE/GTK debug build. I'm not sure why: In file included from DerivedSources/ForwardingHeaders/wtf/text/AtomicString.h:364, from DerivedSources/ForwardingHeaders/wtf/text/WTFString.h:667, from ../../Source/WebCore/platform/network/CacheValidation.h:33, from ../../Source/WebCore/loader/cache/CachedResource.h:26, from ../../Source/WebCore/loader/cache/CachedImage.h:25, from ../../Source/WebCore/rendering/RenderImageResource.h:28, from ../../Source/WebCore/rendering/RenderImageResource.cpp:29, from DerivedSources/WebCore/unified-sources/UnifiedSource-043dd90b-8.cpp:1: DerivedSources/ForwardingHeaders/wtf/text/StringConcatenate.h: In instantiation of ‘WTF::String WTF::tryMakeString(StringTypes ...) [with StringTypes = {const char*, int}]’: DerivedSources/ForwardingHeaders/wtf/text/StringConcatenate.h:327:34: required from ‘WTF::String WTF::makeString(StringTypes ...) [with StringTypes = {const char*, int}]’ ../../Source/WebCore/rendering/RenderLayerCompositor.cpp:1281:84: required from here DerivedSources/ForwardingHeaders/wtf/text/StringConcatenate.h:314:38: error: invalid use of incomplete type ‘class WTF::StringTypeAdapter<int, void>’ return tryMakeStringFromAdapters(StringTypeAdapter<StringTypes>(strings)...); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from DerivedSources/ForwardingHeaders/wtf/Ref.h:30, from DerivedSources/ForwardingHeaders/wtf/RefPtr.h:28, from DerivedSources/ForwardingHeaders/wtf/HashFunctions.h:26, from DerivedSources/ForwardingHeaders/pal/SessionID.h:28, from ../../Source/WebCore/platform/network/CacheValidation.h:28, from ../../Source/WebCore/loader/cache/CachedResource.h:26, from ../../Source/WebCore/loader/cache/CachedImage.h:25, from ../../Source/WebCore/rendering/RenderImageResource.h:28, from ../../Source/WebCore/rendering/RenderImageResource.cpp:29, from DerivedSources/WebCore/unified-sources/UnifiedSource-043dd90b-8.cpp:1: DerivedSources/ForwardingHeaders/wtf/Forward.h:61:43: note: declaration of ‘class WTF::StringTypeAdapter<int, void>’ template<typename, typename = void> class StringTypeAdapter; ^~~~~~~~~~~~~~~~~ I don't see why and templates hurt my head, so I'm going to revert just the one line change in RenderLayerCompositor.cpp. I'll also sneak in a fix for an improper string format warning that was added to this file in the past couple days. Strange coincidence that it's in the same file.
Michael Catanzaro
Comment 18 2019-01-27 19:03:35 PST
Committed r240558: <https://trac.webkit.org/changeset/240558>
Darin Adler
Comment 19 2019-01-28 09:06:50 PST
(In reply to Michael Catanzaro from comment #17) > I don't see why Most likely reason is the need for an include of <wtf/text/StringConcatenateNumbers.h> in RenderLayerCompositor.cpp. Would you be willing to try that and land it if it works?
Michael Catanzaro
Comment 20 2019-01-28 09:09:50 PST
OK. Why isn't that included from StringConcatenate.h :(
Michael Catanzaro
Comment 21 2019-01-28 11:09:45 PST
Committed r240590: <https://trac.webkit.org/changeset/240590>
Darin Adler
Comment 22 2019-01-28 14:45:06 PST
(In reply to Michael Catanzaro from comment #20) > Why isn't that included from StringConcatenate.h :( If we prove it’s negligible slowdown to build time then we could do that to make it easier to get including right.
Note You need to log in before you can comment on or make changes to this bug.