<div dir="rtl"><div dir="ltr">The first time I used such combination it took some digging how to clear the class for re-use, since it requires two operations on two different classes in the correct order of course. It's not hard, but the class user should not be required to dig into the implementation just to clear(reset) the class to its initial state.</div>
<div dir="ltr"><br></div><div dir="ltr">With raw_svector_ostream, if I got it right, clear() should be:</div><div dir="ltr"><br></div><div dir="ltr"><div dir="ltr"> SmallString<256> OutName;</div><div dir="ltr">
llvm::raw_svector_ostream Out(OutName);</div>
<div>...</div><div> // Clear Out for re-use.</div><div> OutName.clear();</div><div> Out.resync();</div><div><br></div><div>Whatever class we end up with - StringBuilder / raw_string_ostream and raw_svector_ostream they should have that clear() function.</div>
<div><br></div><div>Yaron</div><div><br></div><div><br></div><div><br></div></div></div><div class="gmail_extra"><br><br><div class="gmail_quote"><div dir="ltr">2014-06-12 20:32 GMT+03:00 Philip Reames <span dir="ltr"><<a href="mailto:listmail@philipreames.com" target="_blank">listmail@philipreames.com</a>></span>:</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
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. <br><span class="HOEnZb"><font color="#888888">
<br>
Philip</font></span><div><div class="h5"><br>
<br>
<div>On 06/12/2014 09:21 AM, James Molloy
wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">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.
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>Alp's suggestion seems to fit that usecase, make it
simpler, and make it more efficient. So I'm all for it.</div>
<div><br>
</div>
<div>James</div>
</div>
<div class="gmail_extra"><br>
<br>
<div class="gmail_quote">
On 12 June 2014 16:58, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">
<div class="gmail_extra"><br>
<div class="gmail_quote">
<div>On Thu, Jun 12, 2014 at 4:52 PM, Alp
Toker <span dir="ltr"><<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span>
wrote:
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
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".<br>
<br>
So to put reset() or seeking operations inside the
primitive ostream subclasses would be quite a
layering violation.</blockquote>
<div><br>
</div>
</div>
<div>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. ;]</div>
<div><br>
</div>
<div>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.</div>
<div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>
<br>
</div>
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.<br>
</blockquote>
<div><br>
</div>
</div>
<div>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.</div>
<div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
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.</blockquote>
</div>
</div>
<br>
</div>
<div class="gmail_extra">We disagree here, restating
things seems unlikely to progress anything.</div>
</div>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br>
</blockquote>
</div>
<br>
</div>
<br>
<fieldset></fieldset>
<br>
<pre>_______________________________________________
llvm-commits mailing list
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a>
</pre>
</blockquote>
<br>
</div></div></div>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>