Introduce StringBuilder utility around raw_string_ostream
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
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.
> 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
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits