Introduce StringBuilder utility around raw_string_ostream

[Philip Reames]

Taking a step back, here's the features I've heard that are desired out 
of whatever we end up creating:
- An easy clear function which clears the underlying storage.  This 
prevents bugs like:
std::string Str;
raw_string_ostream Stream;
Stream << "some text"; <-- not yet flushed!
Str.clear();
Stream << "something"; <-- flushes *both* writes to the String.

- Desire to avoid virtual dispatch on write_impl (and thus buffer in the 
base class).
- Desire to preserve a streaming interface for streaming clients.

- A version of raw_string_stream which owns it's own storage.  (i.e. can 
be used as a single stack object.)  This prevents bugs like:
std::string Str;
raw_string_ostream Stream;
Stream << "some text";
use( Str ); <-- not yet flushed

Is there anything else that the proposed StringBuilder adds?

For the first, we can and should add a clear function to the 
raw_string_ostream class.  This seems like a good idea regardless.

Do we agree the last is desirable?  Regardless of how that ends up being 
implemented?

Philip



On 06/12/2014 01:25 PM, Alp Toker wrote:
>
> On 12/06/2014 22:37, Chandler Carruth wrote:
>> On Thu, Jun 12, 2014 at 8:26 PM, Alp Toker <alp at nuanti.com 
>> <mailto:alp at nuanti.com>> wrote:
>>
>>
>>     On 12/06/2014 21:31, Chandler Carruth wrote:
>>
>>
>>         On Thu, Jun 12, 2014 at 5:21 PM, James Molloy
>>         <james at jamesmolloy.co.uk <mailto:james at jamesmolloy.co.uk>
>>         <mailto:james at jamesmolloy.co.uk
>>         <mailto:james at jamesmolloy.co.uk>>> 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.
>>
>>
>>         But that's not what we would get.
>>
>>
>>     I think you're taking an extreme and isolated view on this one
>>     Chandler.
>>
>>
>> Dave independently wrote almost the exact same thing.
>>
>>     What James describes is exactly what we get from the proposed patch.
>>
>>
>> We disagree here.
>>
>>     I've gone ahead and produced examples of invalid string access,
>>     and demonstrated how the patch fixes each of these:
>>
>>
>> Yes, two bugs in debugging code. Both of which would also be avoided 
>> by the simpler suggestion I already made.
>
> I don't really agree.
>
>> I continue to believe the simpler suggestion could be made to work 
>> equally well with stack based storage.
>
> What would happen if every ostream subclass provided integrated 
> storage? You put an std::string in raw_string_ostream .. but what do 
> you put in a raw_fd_ostream? This non sequitur highlights the logical 
> shortcoming of your simpler suggestion.
>
> These are streaming interfaces -- the only alternative I see you've 
> suggested so far is to plug arbitrary storage and provide a weak, 
> not-really-safe synthesis of what I've proposed, adding inappropriate 
> complexity to raw_string_ostream with no clear benefit.
>
>>
>>
>>         , and it's going to be quite far from replacing the use cases
>>         of Twine.
>>
>>
>>     The latest iteration of the patch is actually a very worthwhile
>>     Twine alternative if you're hitting string lifetime issues with 
>> that.
>>
>>     Anyway, shouldn't we be working to make the LLVM interfaces safer
>>     and more efficient for developers?
>>
>>
>> We are. Part of that is code review. Currently, I don't see any 
>> strong consensus that your proposal is the right tradeoff, and an 
>> alternative has been proposed in code review that hasn't really been 
>> explored.
>
> Okay. What evidence do you have for that claim? I'm lacking context.
>
> Alp.
>
>