Introduce StringBuilder utility around raw_string_ostream

Chandler Carruth chandlerc at google.com
Thu Jun 12 06:54:55 PDT 2014


How about an even simpler approach.

Put a std::string inside raw_string_ostream, and a default constructor
which binds the reference to its internal std::string. I'm moderately
confident that our optimizers will remove the "dead" std::string that is
left in the class by users that want to leverage external storage.

This has the advantage of not introducing any new types or patterns, merely
removing the requirement to pass in a std::string to provide storage by
using internal storage.

That's getting far enough down in the simplicity (for folks learning about
the APIs) to make me quite happy.


On Thu, Jun 12, 2014 at 2:39 PM, Alp Toker <alp at nuanti.com> wrote:

>
> On 12/06/2014 14:17, Yaron Keren wrote:
>
>> This looks like a very useful class. It should work for the similar
>> SmallString pattern, right?
>>
>>   SmallString<256> OutName;
>>   llvm::raw_svector_ostream Out(OutName);
>> cast<ItaniumMangleContext>(CGM.getCXXABI().getMangleContext()).mangleCXXVTT(RD,
>> Out);
>>   Out.flush();
>>   StringRef Name = OutName.str();
>>
>> ->
>>
>>   StringBuilder Out;
>> cast<ItaniumMangleContext>(CGM.getCXXABI().getMangleContext()).mangleCXXVTT(RD,
>> Out);
>>   StringRef Name = Out.str();
>>
>
> Yeah, that'd work. But for cases like the above we should first provide a
> SmallString<> backed specialization that permits StringBuilder<256> Out;
>
> The initial patch I posted is aimed more at the std::string-backed cases.
> A stack-allocating specialization should follow nicely from that.
>
> Alp.
>
>
>
>> Yaron
>>
>>
>>
>> 2014-06-12 8:01 GMT+03:00 Alp Toker <alp at nuanti.com <mailto:
>> alp at nuanti.com>>:
>>
>>
>>
>>     On 12/06/2014 07:41, David Blaikie wrote:
>>
>>         (would find this a little easier to review if it were in Phab)
>>
>>         One off-the-cuff thought: If it's not already documented, you
>>         might
>>         want to put a comment in raw_string_ostream's ctor that
>>         declares that
>>         it /really shouldn't/ touch the string (since you're passing in an
>>         object in the derived class that won't itself be constructed until
>>         after the raw_string_ostream's ctor has finished). Or, better
>>         yet, if
>>         there's a way to build a raw_string_ostream without a string, then
>>         attach it later (in the constructor of StringBuilder) that
>>         might be
>>         good.
>>
>>
>>     Sure, I'll add a comment. (But I don't think it's a big concern to
>>     justify any major shuffle -- the ctor is right there inline to see
>>     a few lines above, and it seems unlikely a stream adapter would
>>     ever touch its storage unless written to.)
>>
>>
>>         (insert various bikeshedding about the name - though I'm not
>>         really
>>         sure raw_ostream ever needed to adopt the C++ standard library
>>         naming
>>         convention in the first place, but I haven't looked closely)
>>
>>
>>     So, I thought about that..
>>
>>     Having tried raw_string_builder_ostream, string_builder_ostream,
>>     raw_string_builder, I ended up with StringBuilder as the only one
>>     that looked beautiful in use.
>>
>>     Hard to say why, but it's probably because it has a dual role as
>>     storage and streaming interface unlike the ostream adapter
>>     classes, making the distinction significant.
>>
>>     Alp.
>>
>>
>>
>>         On Wed, Jun 11, 2014 at 9:25 PM, Alp Toker <alp at nuanti.com
>>         <mailto:alp at nuanti.com>> wrote:
>>
>>             Replaces the pattern:
>>
>>                std::string buf;
>>                raw_string_ostream os(buf);
>>                ...
>>                os.flush();
>>                use(buf);
>>
>>             with:
>>                StringBuilder os;
>>                ...
>>                use (os.str());
>>
>>             Benefits:
>>
>>               * Provides an inherently safe and opaque interface for
>>             building std::string
>>             instances
>>               * Opens up the possibility of changing the underlying
>>             storage in future.
>>
>>             Patch also converts various uses, some of which were
>>             accessing the storage
>>             without flushing.
>>
>>             Alp.
>>
>>             --
>>             http://www.nuanti.com
>>             the browser experts
>>
>>
>>             _______________________________________________
>>             llvm-commits mailing list
>>             llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>>
>>             http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>>     --     http://www.nuanti.com
>>     the browser experts
>>
>>     _______________________________________________
>>     llvm-commits mailing list
>>     llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>>     http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>>
> --
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140612/e888159d/attachment.html>


More information about the llvm-commits mailing list