Bug 214320 – Don't include <wtf/text/WTFString.h> in SVGParserUtilities.h

• Component: SVG
• Status: RESOLVED FIXED

[Fujii Hironori]

Don't include <wtf/text/WTFString.h> in SVGParserUtilities.h

See https://lists.webkit.org/pipermail/webkit-dev/2020-July/031274.html

r264332 added #include <wtf/text/WTFString.h> in SVGParserUtilities.h.

[Fujii Hironori]

Created attachment 404280 [details]
WIP patch

[Fujii Hironori]

Here are the compilation errros of the WIP patch
> perl .\Tools\Scripts\build-webkit --wincairo --debug --no-unified-builds


[5077/6395] Building CXX object Source\WebCore\CMakeFiles\WebCore.dir\svg\SVGParserUtilities.cpp.obj
FAILED: Source/WebCore/CMakeFiles/WebCore.dir/svg/SVGParserUtilities.cpp.obj 
C:\PROGRA~1\LLVM\bin\clang-cl.exe  /nologo -TP -DANGLE_ENABLE_D3D11 -DANGLE_ENABLE_D3D9 -DANGLE_ENABLE_ESSL -DANGLE_ENABLE_HLSL -DBUILDING_WITH_CMAKE=1 -DBUILDING_WebCore -DEGL_EGLEXT_PROTOTYPES -DEGL_EGL_PROTOTYPES=1 -DGL_API="" -DGL_APICALL="" -DGL_GLES_PROTOTYPES=1 -DGL_GLEXT_PROTOTYPES -DHAVE_CONFIG_H=1 -DNOCRYPT -DNOMINMAX -DSTATICALLY_LINKED_WITH_PAL=1 -DUNICODE -DWINVER=0x601 -DWTF_PLATFORM_WIN_CAIRO=1 -D_CRT_SECURE_NO_WARNINGS -D_HAS_EXCEPTIONS=0 -D_UNICODE -D_WIN32_WINNT=0x601 -D_WINDOWS -D_WINSOCKAPI_="" -D__STDC_CONSTANT_MACROS -IDerivedSources\ForwardingHeaders -I. -IWebCore\DerivedSources -I..\..\Source\WebCore -I..\..\Source\WebCore\Modules\airplay -I..\..\Source\WebCore\Modules\applepay -I..\..\Source\WebCore\Modules\applepay\paymentrequest -I..\..\Source\WebCore\Modules\applicationmanifest -I..\..\Source\WebCore\Modules\async-clipboard -I..\..\Source\WebCore\Modules\beacon -I..\..\Source\WebCore\Modules\cache -I..\..\Source\WebCore\Modules\credentialmanagement -I..\..\Source\WebCore\Modules\encryptedmedia -I..\..\Source\WebCore\Modules\encryptedmedia\legacy -I..\..\Source\WebCore\Modules\entriesapi -I..\..\Source\WebCore\Modules\fetch -I..\..\Source\WebCore\Modules\geolocation -I..\..\Source\WebCore\Modules\highlight -I..\..\Source\WebCore\Modules\indexeddb -I..\..\Source\WebCore\Modules\indexeddb\client -I..\..\Source\WebCore\Modules\indexeddb\server -I..\..\Source\WebCore\Modules\indexeddb\shared -I..\..\Source\WebCore\Modules\mediacapabilities -I..\..\Source\WebCore\Modules\mediacontrols -I..\..\Source\WebCore\Modules\mediarecorder -I..\..\Source\WebCore\Modules\mediasession -I..\..\Source\WebCore\Modules\mediasource -I..\..\Source\WebCore\Modules\mediastream -I..\..\Source\WebCore\Modules\mediastream\libwebrtc -I..\..\Source\WebCore\Modules\navigatorcontentutils -I..\..\Source\WebCore\Modules\notifications -I..\..\Source\WebCore\Modules\paymentrequest -I..\..\Source\WebCore\Modules\pictureinpicture -I..\..\Source\WebCore\Modules\plugins -I..\..\Source\WebCore\Modules\quota -I..\..\Source\WebCore\Modules\remoteplayback -I..\..\Source\WebCore\Modules\speech -I..\..\Source\WebCore\Modules\streams -I..\..\Source\WebCore\Modules\webaudio -I..\..\Source\WebCore\Modules\webauthn -I..\..\Source\WebCore\Modules\webauthn\cbor -I..\..\Source\WebCore\Modules\webauthn\fido -I..\..\Source\WebCore\Modules\webdatabase -I..\..\Source\WebCore\Modules\webdriver -I..\..\Source\WebCore\Modules\webgpu -I..\..\Source\WebCore\Modules\webgpu\WHLSL -I..\..\Source\WebCore\Modules\webgpu\WHLSL\AST -I..\..\Source\WebCore\Modules\websockets -I..\..\Source\WebCore\Modules\webxr -I..\..\Source\WebCore\accessibility -I..\..\Source\WebCore\accessibility\isolatedtree -I..\..\Source\WebCore\animation -I..\..\Source\WebCore\bindings -I..\..\Source\WebCore\bindings\js -I..\..\Source\WebCore\bridge -I..\..\Source\WebCore\bridge\c -I..\..\Source\WebCore\bridge\jsc -I..\..\Source\WebCore\contentextensions -I..\..\Source\WebCore\crypto -I..\..\Source\WebCore\crypto\algorithms -I..\..\Source\WebCore\crypto\keys -I..\..\Source\WebCore\crypto\parameters -I..\..\Source\WebCore\css -I..\..\Source\WebCore\css\parser -I..\..\Source\WebCore\css\typedom -I..\..\Source\WebCore\cssjit -I..\..\Source\WebCore\dom -I..\..\Source\WebCore\dom\messageports -I..\..\Source\WebCore\domjit -I..\..\Source\WebCore\editing -I..\..\Source\WebCore\fileapi -I..\..\Source\WebCore\history -I..\..\Source\WebCore\html -I..\..\Source\WebCore\html\canvas -I..\..\Source\WebCore\html\forms -I..\..\Source\WebCore\html\parser -I..\..\Source\WebCore\html\shadow -I..\..\Source\WebCore\html\track -I..\..\Source\WebCore\inspector -I..\..\Source\WebCore\inspector\agents -I..\..\Source\WebCore\inspector\agents\page -I..\..\Source\WebCore\inspector\agents\worker -I..\..\Source\WebCore\layout -I..\..\Source\WebCore\layout\blockformatting -I..\..\Source\WebCore\layout\blockformatting\tablewrapper -I..\..\Source\WebCore\layout\displaytree -I..\..\Source\WebCore\layout\floats -I..\..\Source\WebCore\layout\inlineformatting -I..\..\Source\WebCore\layout\inlineformatting\text -I..\..\Source\WebCore\layout\integration -I..\..\Source\WebCore\layout\invalidation -I..\..\Source\WebCore\layout\layouttree -I..\..\Source\WebCore\layout\tableformatting -I..\..\Source\WebCore\loader -I..\..\Source\WebCore\loader\appcache -I..\..\Source\WebCore\loader\archive -I..\..\Source\WebCore\loader\archive\mhtml -I..\..\Source\WebCore\loader\cache -I..\..\Source\WebCore\loader\icon -I..\..\Source\WebCore\mathml -I..\..\Source\WebCore\page -I..\..\Source\WebCore\page\animation -I..\..\Source\WebCore\page\csp -I..\..\Source\WebCore\page\scrolling -I..\..\Source\WebCore\platform -I..\..\Source\WebCore\platform\animation -I..\..\Source\WebCore\platform\audio -I..\..\Source\WebCore\platform\encryptedmedia -I..\..\Source\WebCore\platform\gamepad -I..\..\Source\WebCore\platform\graphics -I..\..\Source\WebCore\platform\graphics\cpu\arm -I..\..\Source\WebCore\platform\graphics\cpu\arm\filters -I..\..\Source\WebCore\platform\graphics\displaylists -I..\..\Source\WebCore\platform\graphics\filters -I..\..\Source\WebCore\platform\graphics\iso -I..\..\Source\WebCore\platform\graphics\opentype -I..\..\Source\WebCore\platform\graphics\transforms -I..\..\Source\WebCore\platform\mediacapabilities -I..\..\Source\WebCore\platform\mediarecorder -I..\..\Source\WebCore\platform\mediasession -I..\..\Source\WebCore\platform\mediastream -I..\..\Source\WebCore\platform\mediastream\libwebrtc -I..\..\Source\WebCore\platform\mock -I..\..\Source\WebCore\platform\mock\mediasource -I..\..\Source\WebCore\platform\network -I..\..\Source\WebCore\platform\sql -I..\..\Source\WebCore\platform\text -I..\..\Source\WebCore\platform\xr -I..\..\Source\WebCore\plugins -I..\..\Source\WebCore\rendering -I..\..\Source\WebCore\rendering\line -I..\..\Source\WebCore\rendering\mathml -I..\..\Source\WebCore\rendering\shapes -I..\..\Source\WebCore\rendering\style -I..\..\Source\WebCore\rendering\svg -I..\..\Source\WebCore\rendering\updating -I..\..\Source\WebCore\replay -I..\..\Source\WebCore\storage -I..\..\Source\WebCore\style -I..\..\Source\WebCore\svg -I..\..\Source\WebCore\svg\animation -I..\..\Source\WebCore\svg\graphics -I..\..\Source\WebCore\svg\graphics\filters -I..\..\Source\WebCore\svg\properties -I..\..\Source\WebCore\websockets -I..\..\Source\WebCore\workers -I..\..\Source\WebCore\workers\service -I..\..\Source\WebCore\workers\service\context -I..\..\Source\WebCore\workers\service\server -I..\..\Source\WebCore\worklets -I..\..\Source\WebCore\xml -I..\..\Source\WebCore\xml\parser -I..\..\Source\WebCore\Modules\gamepad -I..\..\Source\WebCore\platform\graphics\gpu -I..\include\private -I..\include\private\JavaScriptCore -I..\..\Source\WebCore\accessibility\win -I..\..\Source\WebCore\page\win -I..\..\Source\WebCore\platform\graphics\egl -I..\..\Source\WebCore\platform\graphics\opengl -I..\..\Source\WebCore\platform\graphics\win -I..\..\Source\WebCore\platform\network\win -I..\..\Source\WebCore\platform\win -I..\..\Source\WebCore\platform\cf -I..\..\Source\WebCore\platform\graphics\cairo -I..\..\Source\WebCore\platform\network\curl -I..\..\Source\WebCore\platform\image-decoders -I..\..\Source\WebCore\platform\image-decoders\bmp -I..\..\Source\WebCore\platform\image-decoders\gif -I..\..\Source\WebCore\platform\image-decoders\ico -I..\..\Source\WebCore\platform\image-decoders\jpeg -I..\..\Source\WebCore\platform\image-decoders\jpeg2000 -I..\..\Source\WebCore\platform\image-decoders\png -I..\..\Source\WebCore\platform\image-decoders\webp -I..\..\Source\WebCore\platform\graphics\texmap -I..\..\Source\WebCore\platform\graphics\nicosia -I..\..\Source\WebCore\loader\archive\cf -IJavaScriptCore\Headers -IJavaScriptCore\PrivateHeaders -IWTF\Headers -IPAL\Headers -IANGLE\Headers -I..\..\WebKitLibraries\win\include -I..\..\WebKitLibraries\win\include\cairo /W4 -fdiagnostics-color=always -fcolor-diagnostics -Wno-noexcept-type -Wno-parentheses-equality -Qunused-arguments -Wwrite-strings -Wundef -Wpointer-arith -Wmissing-format-attribute -Wformat-security -Wcast-align -Wno-unknown-argument -Wno-nonportable-include-path -Wno-unknown-pragmas -Wno-macro-redefined -Wno-undef /DWIN32 /D_WINDOWS  /GR- /EHsc- -fno-strict-aliasing /MD /Zi /Ob0 /Od /RTC1   /bigobj /wd4018 /wd4068 /wd4099 /wd4100 /wd4127 /wd4138 /wd4146 /wd4180 /wd4189 /wd4201 /wd4206 /wd4244 /wd4251 /wd4267 /wd4275 /wd4288 /wd4291 /wd4305 /wd4309 /wd4344 /wd4355 /wd4389 /wd4396 /wd4456 /wd4457 /wd4458 /wd4459 /wd4481 /wd4503 /wd4505 /wd4510 /wd4512 /wd4530 /wd4610 /wd4611 /wd4646 /wd4702 /wd4706 /wd4722 /wd4800 /wd4819 /wd4951 /wd4952 /wd4996 /wd6011 /wd6031 /wd6211 /wd6246 /wd6255 /wd6387 /wd4091 /Zi /GS /EHa- /EHc- /EHs- /fp:except- /analyze- /bigobj /utf-8 /validate-charset -fmsc-version=1911 -std:c++17 /Yu"WebCorePrefix.h" /FI"WebCorePrefix.h" /Fp"C:/home/webkit/gb/WebKitBuild/Debug/Source/WebCore/WebCore_SOURCES/WebCorePrefix.pch" /showIncludes /FoSource\WebCore\CMakeFiles\WebCore.dir\svg\SVGParserUtilities.cpp.obj /FdSource\WebCore\WebCore.pdb -c ..\..\Source\WebCore\svg\SVGParserUtilities.cpp
In file included from ..\..\Source\WebCore\svg\SVGParserUtilities.cpp:26:
In file included from ..\..\Source\WebCore\dom\Document.h:30:
In file included from ..\..\Source\WebCore\css\CSSRegisteredCustomProperty.h:28:
WTF\Headers\wtf/text/WTFString.h(445,19): error: explicit specialization of 'WTF::DefaultHash<WTF::String>' after instantiation
template<> struct DefaultHash<String> { using Hash = StringHash; };
                  ^~~~~~~~~~~~~~~~~~~
WTF\Headers\wtf/Forward.h(89,49): note: implicit instantiation first required here
template<typename ValueArg, typename = typename DefaultHash<ValueArg>::Hash, typename = HashTraits<ValueArg>> class HashSet;
                                                ^
In file included from ..\..\Source\WebCore\svg\SVGParserUtilities.cpp:26:
In file included from ..\..\Source\WebCore\dom\Document.h:31:
In file included from ..\..\Source\WebCore\html\CanvasBase.h:28:
In file included from ..\..\Source\WebCore\platform\graphics/IntSize.h:29:
In file included from WTF\Headers\wtf/JSONValues.h:35:
In file included from WTF\Headers\wtf/HashMap.h:25:
WTF\Headers\wtf/HashTable.h(692,17): error: type 'void' cannot be used prior to '::' because it has no members
            if (HashFunctions::safeToCompareToEmptyOrDeleted) {
                ^
WTF\Headers\wtf/HashTable.h(661,16): note: in instantiation of function template specialization 'WTF::HashTable<WTF::String, WTF::KeyValuePair<WTF::String, WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> > >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::String, WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> > > >, void, WTF::HashMap<WTF::String, WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> >, void, WTF::HashTraits<WTF::String>, WTF::HashTraits<WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> > > >::KeyValuePairTraits, WTF::HashTraits<WTF::String> >::inlineLookup<WTF::IdentityHashTranslator<WTF::HashMap<WTF::String, WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> >, void, WTF::HashTraits<WTF::String>, WTF::HashTraits<WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> > > >::KeyValuePairTraits, void>, WTF::String>' requested here
        return inlineLookup<HashTranslator>(key);
               ^
WTF\Headers\wtf/HashTable.h(1070,28): note: in instantiation of function template specialization 'WTF::HashTable<WTF::String, WTF::KeyValuePair<WTF::String, WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> > >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::String, WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> > > >, void, WTF::HashMap<WTF::String, WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> >, void, WTF::HashTraits<WTF::String>, WTF::HashTraits<WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> > > >::KeyValuePairTraits, WTF::HashTraits<WTF::String> >::lookup<WTF::IdentityHashTranslator<WTF::HashMap<WTF::String, WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> >, void, WTF::HashTraits<WTF::String>, WTF::HashTraits<WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> > > >::KeyValuePairTraits, void>, WTF::String>' requested here
        ValueType* entry = lookup<HashTranslator>(key);
                           ^
WTF\Headers\wtf/HashTable.h(475,52): note: in instantiation of function template specialization 'WTF::HashTable<WTF::String, WTF::KeyValuePair<WTF::String, WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> > >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::String, WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> > > >, void, WTF::HashMap<WTF::String, WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> >, void, WTF::HashTraits<WTF::String>, WTF::HashTraits<WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> > > >::KeyValuePairTraits, WTF::HashTraits<WTF::String> >::find<WTF::IdentityHashTranslator<WTF::HashMap<WTF::String, WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> >, void, WTF::HashTraits<WTF::String>, WTF::HashTraits<WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> > > >::KeyValuePairTraits, void>, WTF::String>' requested here
        iterator find(const KeyType& key) { return find<IdentityTranslatorType>(key); }
                                                   ^
WTF\Headers\wtf/HashMap.h(286,19): note: in instantiation of member function 'WTF::HashTable<WTF::String, WTF::KeyValuePair<WTF::String, WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> > >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::String, WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> > > >, void, WTF::HashMap<WTF::String, WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> >, void, WTF::HashTraits<WTF::String>, WTF::HashTraits<WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> > > >::KeyValuePairTraits, WTF::HashTraits<WTF::String> >::find' requested here
    return m_impl.find(key);
                  ^
WTF\Headers\wtf/JSONValues.h(354,18): note: in instantiation of member function 'WTF::HashMap<WTF::String, WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> >, void, WTF::HashTraits<WTF::String>, WTF::HashTraits<WTF::RefPtr<WTF::JSONImpl::Value, WTF::DumbPtrTraits<WTF::JSONImpl::Value> > > >::find' requested here
    return m_map.find(name);
                 ^
In file included from ..\..\Source\WebCore\svg\SVGParserUtilities.cpp:26:
In file included from ..\..\Source\WebCore\dom\Document.h:31:
In file included from ..\..\Source\WebCore\html\CanvasBase.h:28:
In file included from ..\..\Source\WebCore\platform\graphics/IntSize.h:29:
In file included from WTF\Headers\wtf/JSONValues.h:35:
In file included from WTF\Headers\wtf/HashMap.h:25:

[Fujii Hironori]

> WTF\Headers\wtf/text/WTFString.h(445,19): error: explicit specialization of
> 'WTF::DefaultHash<WTF::String>' after instantiation
> template<> struct DefaultHash<String> { using Hash = StringHash; };
>                   ^~~~~~~~~~~~~~~~~~~
> WTF\Headers\wtf/Forward.h(89,49): note: implicit instantiation first
> required here
> template<typename ValueArg, typename = typename DefaultHash<ValueArg>::Hash,
> typename = HashTraits<ValueArg>> class HashSet;
>                                                 ^

a forward template specialization declaration of DefaultHash<String> has to be in Forward.h.

[Fujii Hironori]

Created attachment 404288 [details]
WIP patch

[Fujii Hironori]

Created attachment 404289 [details]
compilation error

Still no luck with "perl .\Tools\Scripts\build-webkit --wincairo --debug --no-unified-builds".

How can I fix it?

[Darin Adler]

Seems like we do need to include WTFString.h unless we are willing to write HashSet<String, StringHash> rather than just HashSet<String>.

[Darin Adler]

Another option is to put the DefaultHash specialization for String into Forward.h instead of WTFString.h, which seems to fit the philosophy of Forward.h pretty well.

[Said Abou-Hallawa]

(In reply to Fujii Hironori from comment #5)
> Created attachment 404289 [details]
> compilation error
> 
> Still no luck with "perl .\Tools\Scripts\build-webkit --wincairo --debug
> --no-unified-builds".
> 
> How can I fix it?

Your two patches passed on wincairo bots. Where is the "--no-unified-builds" option used? What are you are trying to fix?

[Fujii Hironori]

(In reply to Said Abou-Hallawa from comment #8)
> Your two patches passed on wincairo bots. Where is the "--no-unified-builds"
> option used? What are you are trying to fix?

I also wonder if we should care non-unified builds. Let's discuss in webkit-dev.
https://lists.webkit.org/pipermail/webkit-dev/2020-July/031280.html

[Fujii Hironori]

I think DefaultHash<T>::Hash is unnecessarily nested, and this seems the problem.
But, it is too complicated. I'm not sure.

(In reply to Darin Adler from comment #7)
> Another option is to put the DefaultHash specialization for String into
> Forward.h instead of WTFString.h, which seems to fit the philosophy of
> Forward.h pretty well.

It sounds nice.

[Fujii Hironori]

(In reply to Fujii Hironori from comment #10)
> I think DefaultHash<T>::Hash is unnecessarily nested, and this seems the
> problem.

r264488 fixed the problem. My WIP patch can compile now.

[Fujii Hironori]

Created attachment 404516 [details]
Patch

[Fujii Hironori]

Comment on attachment 404516 [details]
Patch

Clearing flags on attachment: 404516

Committed r264533: <https://trac.webkit.org/changeset/264533>

[Fujii Hironori]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/65743754>