r207087 - Comment to XML conversion: use unique_ptr for SimpleFormatContext

David Blaikie dblaikie at gmail.com
Thu Apr 24 08:17:15 PDT 2014


On Thu, Apr 24, 2014 at 7:27 AM, Dmitri Gribenko <gribozavr at gmail.com> wrote:
> On Thu, Apr 24, 2014 at 3:16 PM, David Blaikie <dblaikie at gmail.com> wrote:
>> On Thu, Apr 24, 2014 at 12:52 AM, Dmitri Gribenko <gribozavr at gmail.com> wrote:
>>> Author: gribozavr
>>> Date: Thu Apr 24 02:52:31 2014
>>> New Revision: 207087
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=207087&view=rev
>>> Log:
>>> Comment to XML conversion: use unique_ptr for SimpleFormatContext
>>
>> Any reason this couldn't just be an Optional<SimpleFormatContext>?
>
> It would not be much different from unique_ptr...

It'd avoid an extra memory allocation and makes it clear that this
isn't a polymorphic object, etc. Minimal contract/functionality
required.

>
>> Judging by the comments this might not even need to be optional - it
>> just needs a way to shrink/dispose of some buffers? That should be
>> achievable without having to destroy an object... but I haven't looked
>> in detail at where the side effects are. I guess it's the underlying
>> SourceManager that needs some way to reset.
>
> Seems like removing a file would involve a dance around SourceManager
> and FileManager, which up to now (as far as I see) did not expect any
> files to be removed.  I don't think it is worth the complexity,
> re-creating the object once in a while seems reasonable to me.

Hrm. OK - seems a bit of a pity to me, though.



More information about the cfe-commits mailing list