Introduce StringBuilder utility around raw_string_ostream

Philip Reames listmail at philipreames.com
Thu Jun 12 10:32:07 PDT 2014


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 
> <mailto:chandlerc at google.com>> wrote:
>
>
>     On Thu, Jun 12, 2014 at 4:52 PM, Alp Toker <alp at nuanti.com
>     <mailto: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 <mailto:llvm-commits at cs.uiuc.edu>
>     http://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/2633d9f9/attachment.html>


More information about the llvm-commits mailing list