Mostly sounds good.

On Dec 18, 2007, at 10:51 AM, Darin Adler wrote:

The WebCore::String class needs some work. I have some plans to improve it. Here's an outline of what I have in mind.

- eliminate all non-const functions from StringImpl; fixes tricky sharing semantics - change all uses of const StringImpl* to just plain StringImpl* because there will no longer be any real difference between these types - then, once RefCounted is made thread-safe, then we could remove the copy() function altogether, since the only reason to use it would be thread safety

- change functions that return StringImpl* to return PassRefPtr<StringImpl> instead, if they sometimes make a new object; makes it harder to accidentally leak - change functions that take StringImpl* to take PassRefPtr<StringImpl> instead if they take ownership; can reduce reference count churn - make a release() function on String that returns a PassRefPtr<StringImpl>; can reduce reference count churn

- eliminate functions that return a StringImpl* from StringImpl, because it's to easy to misunderstand and think these modify the string in place - eliminate functions that return a String from String -- also easy to use wrong for the same reasons

Where would we put functions that take a string and return a modified one? Seems like we'll still want those, if our string type becomes truly immutable.

- change TextStream into a class named StringBuilder for clients that are building up strings (name inspired by Java); it will have get() and release() functions that will return PassRefPtr<StringImpl>

I kind of like the += syntax for a StringBuilder better than the << syntax, but I guess that's harder to use if there's no operator+. It seems like the obvious implementation techniques are making a Vector<UChar> and appending to it, or keeping a Vector<StringImpl>, neither of those would play well with a non-releasing get().

- add functions that take a PassRefPtr<StringImpl> and return a PassRefPtr<StringImpl> for many string operations, since those can optimize the "only one reference" case and re-use the same character buffer; change the String class to use those as appropriate - rename the String::impl() function to String::get() to match other RefPtr classes - consider eliminating String and using RefPtr<StringImpl> at most or all call sites - consider changing call sites that take a const String& to instead take StringImpl* or PassRefPtr<StringImpl>

- rename StringImpl to SharedString
- consider renaming WebCore::String to StringRefPtr since it's a lightweight wrapper around a RefPtr; one benefit is that it would allow us to eliminate the header name PlatformString.h

If you did both of those things, a better name for SharedString might be String.

These plans are still a bit rough. Please let me know what you think. Depending on what feedback I get, I might make a Wiki page about this.

Some of this I will do soon. Some of it is longer-term.

    -- Darin

webkit-dev mailing list

webkit-dev mailing list

Reply via email to