[LLVMdev] [PATCH] Circular Buffered Debug Stream

[David Greene]

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.

Ok.

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

Ok.

> 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 
variables signed.

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
remove it.

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

Ok.

> 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.

All right.

                              -Dave