Use std::not_fn instead of lambda in URLParser
Created attachment 437657 [details] Patch
Created attachment 437662 [details] Patch
Created attachment 437678 [details] Patch
Comment on attachment 437678 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437678&action=review > Source/WTF/wtf/text/StringCommon.h:539 > +template<typename CharacterType, std::enable_if_t<std::is_integral<CharacterType>::value>* = nullptr> You can use std::is_integral_v. > Source/WTF/wtf/text/StringImpl.h:596 > +template<typename CodeUnitMatchFunction, std::enable_if_t<std::is_invocable_r<bool, CodeUnitMatchFunction, UChar>::value>* = nullptr> UChar → LChar You can use is_invocable_r_v.
Comment on attachment 437678 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437678&action=review >> Source/WTF/wtf/text/StringImpl.h:596 >> +template<typename CodeUnitMatchFunction, std::enable_if_t<std::is_invocable_r<bool, CodeUnitMatchFunction, UChar>::value>* = nullptr> > > UChar → LChar > You can use is_invocable_r_v. UChar is correct here because CodeUnitMatchFunction used to be bool (*)(UChar) and it will be called with UChar. I like is_invocable_r_v though.
Created attachment 437831 [details] Patch
Comment on attachment 437831 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437831&action=review > Source/WTF/wtf/text/WTFString.h:173 > size_t find(CodeUnitMatchFunction matchFunction, unsigned start = 0) const { return m_impl ? m_impl->find(matchFunction, start) : notFound; } Does this copy the matchFunction object? Should you use CodeUnitMatchFunction&& and std::forward here? You don't templatize StringImpl::find(CodeUnitMatchFunction, unsigned index). Why can you call it with matchFunction? Does this convert matchFunction to bool (*)(UChar)? If so, why do you need to templatize this method?
Comment on attachment 437831 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437831&action=review > Source/WTF/wtf/text/StringImpl.h:659 > +template<typename CodeUnitMatchFunction, std::enable_if_t<std::is_invocable_r_v<bool, CodeUnitMatchFunction, UChar>>*> Now both 'find' are template functions. How about unifying them by templatize LChar and UChar?
Comment on attachment 437831 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437831&action=review > Tools/TestWebKitAPI/Tests/WebKitCocoa/EventAttribution.mm:78 > +static TestNavigationDelegate *delegateAllowingAllTLS() These changes seems came from other bug.
Oh I see. LChar is indeed correct in the one you suggested because it will be called with LChar.
Created attachment 437848 [details] Patch
Comment on attachment 437848 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437848&action=review > Source/WebCore/ChangeLog:8 > + Add a few explicit template parameters needed since we are passing a template parameter now instead of a function pointer. You don't modify String::find in this patch. You don't need this change. > Source/WTF/wtf/text/StringImpl.h:647 > +inline size_t find(const CodeUnit* characters, unsigned length, CodeUnitMatchFunction matchFunction, unsigned index) I think it should be CodeUnitMatchFunction&&. Otherwide the callable object is coped.
Comment on attachment 437848 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437848&action=review >> Source/WTF/wtf/text/StringImpl.h:647 >> +inline size_t find(const CodeUnit* characters, unsigned length, CodeUnitMatchFunction matchFunction, unsigned index) > > I think it should be CodeUnitMatchFunction&&. Otherwide the callable object is coped. It's so-called Universal Reference.
Created attachment 437877 [details] Patch
Comment on attachment 437877 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437877&action=review > Source/WTF/wtf/text/StringImpl.h:647 > +inline size_t find(const CodeUnit* characters, unsigned length, CodeUnitMatchFunction&& matchFunction, unsigned index) 'find' functions in StringCommon.h and StringImpl.h are similar. I think they should be in the same file. > Source/WebCore/html/DOMTokenList.cpp:47 > + return token.find(isHTMLSpace<UChar>) != notFound; You don't modify String::find in this patch. Why do you need this change?
It used to be able to infer <UChar> because it was being passed as a bool(*)(UChar) but now it can't because it's a templatized invocable.
Comment on attachment 437877 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437877&action=review >> Source/WebCore/html/DOMTokenList.cpp:47 >> + return token.find(isHTMLSpace<UChar>) != notFound; > > You don't modify String::find in this patch. Why do you need this change? I confirmed that this patch can compile without this change.
Comment on attachment 437877 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437877&action=review > Source/WTF/wtf/URLParser.cpp:2710 > + return segment.find(std::not_fn(isASCIIHexDigit<UChar>), 2) == notFound; find == notFound is harder to read than !contains; can we add/use contains instead?
Created attachment 437916 [details] Patch
Committed r282303 (241575@main): <https://commits.webkit.org/241575@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 437916 [details].
<rdar://problem/82995709>