Move the code for creating a UCharIterator from a StringView from CollatorICU.cpp to StringView.h/.cpp so that future patches that will use ucol_strcollIter (including Bug 147604) can use it.
Created attachment 266722 [details] Patch
Comment on attachment 266722 [details] Patch Why should this be a member function instead of just a function that takes a StringView? It’s a little bit dangerous to create one of these UCharIterator objects for general use, because the iterator only good as long as the characters pointer to by the StringView are good. The functions we pass to the UCharIterator don’t do the CHECK_STRINGVIEW_LIFETIME checking.
(In reply to comment #2) > Comment on attachment 266722 [details] > Patch > > Why should this be a member function instead of just a function that takes a > StringView? > > It’s a little bit dangerous to create one of these UCharIterator objects for > general use, because the iterator only good as long as the characters > pointer to by the StringView are good. The functions we pass to the > UCharIterator don’t do the CHECK_STRINGVIEW_LIFETIME checking. Could you please advise me on how to go about this? Should I just change it to a function that takes a StringView while keeping it in StringView.h, or should I not do this in general? If the latter, is there a way to avoid duplicate code?
(In reply to comment #3) > Could you please advise me on how to go about this? Should I just change it > to a function that takes a StringView while keeping it in StringView.h, or > should I not do this in general? If the latter, is there a way to avoid > duplicate code? As a first step, I’d suggest changing it to a function that takes a StringView and putting the function in Collator.h. Later we can move it somewhere else if we want to use UCharIterator for things other than collation. The Collator abstraction is not super-important since we are OK with WebKit using ICU directly. It was created by someone who was trying to change WebKit to work with multiple international libraries, but that’s not really a goal for the project now. Anyway, I think having UCharIterator createUCharIterator(StringView) in Collator.h would be a fine way to share it with other code.
Created attachment 267286 [details] Patch
Comment on attachment 267286 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=267286&action=review > Source/WTF/wtf/unicode/Collator.h:71 > +UCharIterator createIterator(StringView); Might later find that we need a more specific name like createUCharIterator but lets start with this.
Comment on attachment 267286 [details] Patch Clearing flags on attachment: 267286 Committed r194041: <http://trac.webkit.org/changeset/194041>
All reviewed patches have been landed. Closing bug.