[LLVMdev] [PATCH] Circular Buffered Debug Stream

Chris Lattner clattner at apple.com
Wed Dec 16 11:19:56 PST 2009


On Dec 16, 2009, at 8:12 AM, David Greene wrote:
>>> 
>>> What, specifically, do you want reviewed before I start checking in?
>>> I'll be happy to prepare a patch that shows just that.
>> 
>> I want a patch that does one thing (e.g. implements dbgs(), 
> 
> Ok, let's start with that.  Here's the patch to add the circular raw_ostream.

This idea of the patch makes sense to me, but here are a few questions:

+    int BufferSize;
+    std::vector<char> BufferArray;
+    bool DelayOutput;
+    std::vector<char>::iterator Cur;

Please doxygenify these.

Instead of using a std::vector for BufferArray, please just new[] an array since it is fixed size.

Why is BufferSize needed with the vector? Isn't it always the size of BufferArray?  Also, why is it signed?


What does DelayOutput do?  Even reading the code, I don't really understand what it is doing.


+    circular_raw_ostream(raw_ostream &Stream, int BufferSize = -1,
+                         bool Delete = false) 
+        : raw_ostream(/*unbuffered*/true), TheStream(0), DeleteStream(false),
+            BufferArray(BufferSize < 0 ? 8192 : BufferSize),

Please make BufferSize an 'unsigned' and default it to 8192.  Please use PRESERVE_STREAM instead of 'false'.



> Do you want another patch that modifies Debug.h and then one more patch that
> shows client use?

Sure, you don't need to convert everything over, but once the general approach looks fine you can do the rest without review.

-Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20091216/4fdc4387/attachment.html>


More information about the llvm-dev mailing list