Introduce StringBuilder utility around raw_string_ostream

[David Blaikie]

On Thu, Jun 12, 2014 at 1:25 PM, Alp Toker <alp at nuanti.com> 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?

I'm not sure I follow. std::stringstream has internal storage but that
doesn't require other things like std::fstream to have internal
storage. That doesn't seem like a non-sequitur.

> 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,

I don't see how either is more or less safe. In one case we have two
types, one with internal storage and one without. The other we have
one type with the dynamic choice between internal storage and not. In
both situations we can still write the same bugs.

> 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.
>
>
> --
> http://www.nuanti.com
> the browser experts
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits