[LLVMdev] [PATCH] Circular Buffered Debug Stream
dag at cray.com
Wed Dec 16 11:35:45 PST 2009
On Wednesday 16 December 2009 13:19, Chris Lattner wrote:
> + 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?
Well, you need it with a new'd array. :)
It's signed because running loops with signed induction variables is better
than with unsiged induction variables. In this case it's moot because
::write_impl is passed a size_t anyway.
It's just learned habit by me to make anything having to do with induction
I'll make it unsigned.
> What does DelayOutput do? Even reading the code, I don't really understand
> what it is doing.
Originally it told the stream whether to buffer. It's superfluous now so I'll
> 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.
More information about the llvm-dev