Bug 177566 – Support concatenating StringView with other string types

• Component: Web Template Framework
• Status: RESOLVED FIXED

[Daniel Bates]

We should add StringOperators support for StringViews so that stringView + string works. Currently only makeString() is smart enough to concatenate a passed StringView with other string types.

[Daniel Bates]

Created attachment 322014 [details]
Patch and unit tests

[Sam Weinig]

I think it is confusing that we have these two methods of concatenation, operator+ and makeString(). I have been using makeString() under the assumption that only it allowed for a gaurenteed single allocation, no matter how many parameters were passed. It would be good if we could pick one and stick to it.

[Daniel Bates]

(In reply to Sam Weinig from comment #2)
> I think it is confusing that we have these two methods of concatenation,
> operator+ and makeString(). I have been using makeString() under the
> assumption that only it allowed for a gaurenteed single allocation, no
> matter how many parameters were passed. It would be good if we could pick
> one and stick to it.

I agree we should standardize on one way to concatenate a string. StringOperators is syntactic sugar on top of makeString(). I like the syntactic sugar and this motivated me to write this patch. Should I email webkit-dev? How do you suggest that we proceed?

[Daniel Bates]

For completeness, StringOperators was added in the patch for bug #58420 landed in <https://trac.webkit.org/changeset/86330>. From the get-go it was implemented in terms of tryMakeString().

[Darin Adler]

Comment on attachment 322014 [details]
Patch and unit tests

View in context: https://bugs.webkit.org/attachment.cgi?id=322014&action=review

> Source/WTF/ChangeLog:18
> +        (WTF::StringTypeAdapter<StringView>::StringTypeAdapter<StringView>): Moved from StringView.h.

Why?

[Daniel Bates]

(In reply to Darin Adler from comment #5)
> Comment on attachment 322014 [details]
> Patch and unit tests
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=322014&action=review
> 
> > Source/WTF/ChangeLog:18
> > +        (WTF::StringTypeAdapter<StringView>::StringTypeAdapter<StringView>): Moved from StringView.h.
> 
> Why?

Will revert.

It has been a while since I wrote this patch. I suspect the reason I moved this class from file Source/WTF/wtf/text/StringView.h to file StringConcatenate.h was to group it with all the other StringTypeAdapter classes defined in file StringConcatenate.h.

[Daniel Bates]

Seeing some of Darin's recent String::format() changes reminded me of this patch. I find the string concatenation functions convenient to use and more aesthetically pleasing than using the variadic function makeString(). I am going to land this patch.

[Daniel Bates]

Created attachment 357467 [details]
To Land

[Daniel Bates]

Committed r239282: <https://trac.webkit.org/changeset/239282>

[Radar WebKit Bug Importer]

<rdar://problem/46786243>