r206986 - Replacing a naked pointer with a unique_ptr. No functional changes intended.

Delesley Hutchins delesley at google.com
Wed Apr 23 10:58:24 PDT 2014


Yes, Prev needs to be a pointer.  And it's non-owning, so IMHO, a raw
pointer is just fine.

CallCtx does not need to be dynamically allocated, so long as you put
in a move assignment operator.
The copy is probably marginally cheaper than the dynamic
(de)allocation, it's just more code to write.  :-)

  -DeLesley


On Wed, Apr 23, 2014 at 10:23 AM, David Blaikie <dblaikie at gmail.com> wrote:
> On Wed, Apr 23, 2014 at 10:18 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
>> On Wed, Apr 23, 2014 at 1:08 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>> On Wed, Apr 23, 2014 at 7:26 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
>>>> Author: aaronballman
>>>> Date: Wed Apr 23 09:26:59 2014
>>>> New Revision: 206986
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=206986&view=rev
>>>> Log:
>>>> Replacing a naked pointer with a unique_ptr. No functional changes intended.
>>>
>>> Any particular reason this needs to be dynamically allocated (it
>>> doesn't appear to be polymorphic - but perhaps it will be in the
>>> future?)? Would Optional<CallingContext> suffice?
>>
>> I think it would be possible to do -- CallingContext::Prev would have
>> to change to be Optional<CallingContext> Prev,
>
> Well, that wouldn't work (then you'd have an infinitely recursive type
> - optional has inline storage, so if a T has an Optional<T> then T has
> to be infinitely large), but it doesn't need to - it can remain as a
> non-owning pointer. (we could add a utility to Optional<T> to retrieve
> a T*, null if the Optional has no current value, if it doesn't already
> have such an operation)
>
>>but I don't see the
>> extra copies being overly expensive. I'll let DeLesley make the call
>> since this is in perf-sensitive code.
>>
>> ~Aaron



-- 
DeLesley Hutchins | Software Engineer | delesley at google.com | 505-206-0315



More information about the cfe-commits mailing list