Introduce StringBuilder utility around raw_string_ostream

[Yaron Keren]

The first time I used such combination it took some digging how to clear
the class for re-use, since it requires two operations on two different
classes in the correct order of course. It's not hard, but the class user
should not be required to dig into the implementation just to clear(reset)
the class to its initial state.

With raw_svector_ostream, if I got it right, clear() should be:

    SmallString<256> OutName;
    llvm::raw_svector_ostream Out(OutName);
...
    // Clear Out for re-use.
    OutName.clear();
    Out.resync();

Whatever class we end up with - StringBuilder / raw_string_ostream and
raw_svector_ostream they should have that clear() function.

Yaron





2014-06-12 20:32 GMT+03:00 Philip Reames <listmail at philipreames.com>:

>  I agree with James.  My experience with current raw_string_ostream code
> is that it's almost always wrong the first time.  I don't care what we end
> up calling it, but having a simple interface which is hard to screw up
> would be very helpful.  Simple guidelines for code reviews would also be
> good.
>
> Philip
>
>
> On 06/12/2014 09:21 AM, James Molloy wrote:
>
> FWIW, I like Alp's suggestion. There are a bunch of ways to do string
> building in LLVM (Twine, raw_string_ostream, std::stringstream). I'd
> appreciate a single mechanism for making strings.
>
>  And the important thing I as a user would love to see is checking. Am I
> performing unsafe operations? Will my call result in references to
> deallocated storage? My experience with Twine has been a succession of
> "bugger, that was a reference to a temporary object. Add a .str() here"
> until it works.
>
>  Alp's suggestion seems to fit that usecase, make it simpler, and make it
> more efficient. So I'm all for it.
>
>  James
>
>
>  On 12 June 2014 16:58, Chandler Carruth <chandlerc at google.com> wrote:
>
>>
>>  On Thu, Jun 12, 2014 at 4:52 PM, Alp Toker <alp at nuanti.com> wrote:
>>>
>>> The existing stream classes are really light append-only non-seekable
>>> streams. reset() is a higher-level feature that makes onerous assumptions
>>> about storage, both "what it can do" and "what we're allowed to do with it".
>>>
>>> So to put reset() or seeking operations inside the primitive ostream
>>> subclasses would be quite a layering violation.
>>
>>
>>  I dunno. raw_string_ostream has a 'str' method that returns a
>> std::string reference. I don't know what we could possibly do to this class
>> that would more tightly couple it to the underlying storage. ;]
>>
>>  But I'm not asking you to add this functionality, just saying that if
>> this is a problem in practice there seems to be an easy solution.
>>
>>
>>>
>>>  I can't see how that'd be done without making a muddle of the ostream
>>> primitive classes. Adding optional storage to a "simple adaptor class"
>>> strikes me as a poor idea because it'd no longer be usable be a thin
>>> interface that's easily inheritable.
>>>
>>
>>  I don't see why raw_string_ostream is a thin interface or is easily
>> inheritable. I think it probably should be declared as final. It is a very
>> specific, concrete wrapper around a std::string and a raw_ostream. Same
>> goes for the svector variant.
>>
>>
>>>
>>> At 25 LoC the proposed implementation is still pretty tight even after
>>> adding the stack allocation facility. And it doesn't hack up ostream
>>> classes, and fixes various points of use, all clear pluses to me.
>>
>>
>>  We disagree here, restating things seems unlikely to progress anything.
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>
>
> _______________________________________________
> llvm-commits mailing listllvm-commits at cs.uiuc.eduhttp://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140612/cbae1a10/attachment.html>