[llvm-commits] Twine/StringRef enhancements & usage

[David Blaikie]

Here's some more Twine-ification of APIs. I realize this seems like a
somewhat random assortment - I haven't, for example, fully changed
some types to Twine (so there might be some functions in a type that
take Twine, others that take StringRef still), but it does seem to be
stable & I didn't want to just keep piling on more code.

API changes:
* Twine
  * appendTo(std::string&)
    Works like "toVector" but on std::strings. (I'd like to rename
toVector to appendTo to make it clear it's appending, not overwriting
- I added test cases to expose this existing deliberate design because
when I 'fixed' it by clearing the vector in toVector I broke a bunch
of stuff that was relying on it being appending)
  * assignTo(std::string&)
    clear() + appendTo
* StringRef
  * StringRef(const char (&)[N])
    an array constructor so it doesn't have to use strlen for string
literals. For some reason this doesn't work in GCC (try making the
StringRef(const char*) ctor explicit & GCC starts complaining about
how it can't convert the const char* in 'return "foo";' into a
StringRef). This is a little tricky - it doesn't include the trailing
null character if one is present. This is to maintain compatibility
with the existing implicit conversion from string arrays via the
(const char*) ctor.
  * assignTo(std::string&)
    the same idea as Twine::appendTo(std::string&). This is more
efficient than using str = strRef; because it doesn't produce an extra
temporary buffer to return through. ctor initialization (std::string
str = strRef;) is still efficient due to RVO.
  * booleanTest()/SafeBool<T>
    I've made StringRef boolean testable so it can be used as a drop
in replacement when null tests are required
* TwineString - a type that makes retrieving/manipulating a Twine
value a little simpler. Yes, it's a little dodgy that it derives from
StringRef but it's 'good enough' to make Twines the default string
arguments - any time you see a StringRef or std::string argument there
should be a specific explanation.

One current problem I have is that SmallString<T> is convertible to
StringRef and StringRef to Twine, but that seems to be insufficient to
implicitly convert from SmallString<T> to Twine - so I've added a few
explicit Twine(smallstr) calls around. If there's a better way to
approach this (ultimately we could add a SmallString option to Twine,
but it'd be sort of nice not to have to do that either) I'm all ears.

Aside: Twine::toNullTerminatedStringRef scares me. A lot. It relies on
UB to leave the null character beyond the end of the allocated buffer.
Could we fix this to return a StringRef that doesn't include the null
character (though I'm not sure this bit should matter, really - code
asking for the null terminated StringRef is probably doing it to pass
to a C API that's not going to care about the StringRef's concept of
length), but leave the character itself in the buffer? (& I was
thinking we could check if the Twine was a non-empty StringRef
already, test the last character for null, then return a StringRef
over that rather than using a new buffer - though this wouldn't work
for my current StringRef(const char(&)[N]) ctor, since the length
won't include the null character).

Thanks for your help,

- David
-------------- next part --------------
A non-text attachment was scrubbed...
Name: twine_clang.diff
Type: application/octet-stream
Size: 1262 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110728/f90ebda3/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: twine.diff
Type: application/octet-stream
Size: 70174 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110728/f90ebda3/attachment-0001.obj>