llvm dev - Jul 2014 - [LLVMdev] [cfe-dev] Use of Smart Pointers in LLVM Projects

On Fri, Jul 18, 2014 at 12:06 PM, Jordan Rose <jordan_rose at apple.com>
wrote:
> I don't have much to add here besides +1. I think using std::unique_ptr
> even for create* functions/methods is the right way to go.

+1 smart pointers here are a win in terms of safety and self-documentation.
I don't see why create* factories should be treated differently.

Eli



> Reid's point about an abstraction penalty is interesting, but I
don't
> think we do ownership transfers often enough to actually see a performance
> hit. (Of course, in the non-transferring case we'd just pass the
pointer,
> not a 'const std::unique_ptr &' or anything.)
>
> Jordan
>
>
> On Jul 17, 2014, at 16:21 , David Blaikie <dblaikie at gmail.com>
wrote:
>
> > There seems to be some uncertainty about the use of smart pointers
> > (previously OwningPtr, now std::unique_ptr and std::shared_ptr
> > predominantly) in the LLVM project as a whole, so here's a thread
to
> > discuss/clarify/etc the project preferences/direction with regard to
> > smart pointer usage.
> >
> > For some context, see discussions in LLVM r212403 and Clang r213307.
> >
> > The basic question here seems to be whether smart pointer ownership is
> > a direction we want to take the LLVM project in general.
> >
> > I've seen others contribute and have myself contributed many
patches
> > moving towards smart pointer ownership (both in the pre-C++11 days of
> > OwningPtr, and much moreso in the post-C++11 world with
> > std::unique_ptr and std::shared_ptr being usable inside containers, as
> > return values, etc, allowing many more opportunities).
> >
> > std::unique_ptr's been used in LLD as far back as r153620.
> > std::unique_ptr appeared in LLVM shortly after the C++11 switch with
> > Ahmed's work to migrate the project from OwningPtr to
std::unique_ptr
> > (starting with r202609 and ending with r211259). Originally OwningPtr
> > was added in r45261.
> > Something in the order of 60 changes across clang and LLVM mention
> > unique_ptr in their subject and migrate various APIs to use unique_ptr
> > for ownership. Many of which remove uses of explicit delete or helpers
> > like DeleteContainerPointers (and removing explicit dtors in many of
> > those cases).
> >
> > Are people OK with/prefer the use of owning smart pointers in APIs?
> > Are there places where you've found them to be too
noisy/burdensome
> > and would rather use raw pointers or some other abstraction? Would you
> > prefer pre-commit review of such changes to adequately consider
> > alternatives (such as?)?
> >
> > Thanks,
> > - David
> > _______________________________________________
> > cfe-dev mailing list
> > cfe-dev at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140718/8ee25883/attachment.html>

Sounds like we've got sufficient amount of momentum here for Dave to
go ahead and recommit. Any last objections?

-eric

On Fri, Jul 18, 2014 at 1:35 PM, Eli Bendersky <eliben at google.com>
wrote:
>
>
>
> On Fri, Jul 18, 2014 at 12:06 PM, Jordan Rose <jordan_rose at
apple.com> wrote:
>>
>> I don't have much to add here besides +1. I think using
std::unique_ptr
>> even for create* functions/methods is the right way to go.
>
>
> +1 smart pointers here are a win in terms of safety and self-documentation.
> I don't see why create* factories should be treated differently.
>
> Eli
>
>
>
>>
>> Reid's point about an abstraction penalty is interesting, but I
don't
>> think we do ownership transfers often enough to actually see a
performance
>> hit. (Of course, in the non-transferring case we'd just pass the
pointer,
>> not a 'const std::unique_ptr &' or anything.)
>>
>> Jordan
>>
>>
>> On Jul 17, 2014, at 16:21 , David Blaikie <dblaikie at gmail.com>
wrote:
>>
>> > There seems to be some uncertainty about the use of smart pointers
>> > (previously OwningPtr, now std::unique_ptr and std::shared_ptr
>> > predominantly) in the LLVM project as a whole, so here's a
thread to
>> > discuss/clarify/etc the project preferences/direction with regard
to
>> > smart pointer usage.
>> >
>> > For some context, see discussions in LLVM r212403 and Clang
r213307.
>> >
>> > The basic question here seems to be whether smart pointer
ownership is
>> > a direction we want to take the LLVM project in general.
>> >
>> > I've seen others contribute and have myself contributed many
patches
>> > moving towards smart pointer ownership (both in the pre-C++11 days
of
>> > OwningPtr, and much moreso in the post-C++11 world with
>> > std::unique_ptr and std::shared_ptr being usable inside
containers, as
>> > return values, etc, allowing many more opportunities).
>> >
>> > std::unique_ptr's been used in LLD as far back as r153620.
>> > std::unique_ptr appeared in LLVM shortly after the C++11 switch
with
>> > Ahmed's work to migrate the project from OwningPtr to
std::unique_ptr
>> > (starting with r202609 and ending with r211259). Originally
OwningPtr
>> > was added in r45261.
>> > Something in the order of 60 changes across clang and LLVM mention
>> > unique_ptr in their subject and migrate various APIs to use
unique_ptr
>> > for ownership. Many of which remove uses of explicit delete or
helpers
>> > like DeleteContainerPointers (and removing explicit dtors in many
of
>> > those cases).
>> >
>> > Are people OK with/prefer the use of owning smart pointers in
APIs?
>> > Are there places where you've found them to be too
noisy/burdensome
>> > and would rather use raw pointers or some other abstraction? Would
you
>> > prefer pre-commit review of such changes to adequately consider
>> > alternatives (such as?)?
>> >
>> > Thanks,
>> > - David
>> > _______________________________________________
>> > cfe-dev mailing list
>> > cfe-dev at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>>
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>

No objections here. I'd be glad to see this go in. :)

- Lang.


On Fri, Jul 18, 2014 at 4:30 PM, Eric Christopher <echristo at gmail.com>
wrote:
> Sounds like we've got sufficient amount of momentum here for Dave to
> go ahead and recommit. Any last objections?
>
> -eric
>
> On Fri, Jul 18, 2014 at 1:35 PM, Eli Bendersky <eliben at google.com>
wrote:
> >
> >
> >
> > On Fri, Jul 18, 2014 at 12:06 PM, Jordan Rose <jordan_rose at
apple.com>
> wrote:
> >>
> >> I don't have much to add here besides +1. I think using
std::unique_ptr
> >> even for create* functions/methods is the right way to go.
> >
> >
> > +1 smart pointers here are a win in terms of safety and
> self-documentation.
> > I don't see why create* factories should be treated differently.
> >
> > Eli
> >
> >
> >
> >>
> >> Reid's point about an abstraction penalty is interesting, but
I don't
> >> think we do ownership transfers often enough to actually see a
> performance
> >> hit. (Of course, in the non-transferring case we'd just pass
the
> pointer,
> >> not a 'const std::unique_ptr &' or anything.)
> >>
> >> Jordan
> >>
> >>
> >> On Jul 17, 2014, at 16:21 , David Blaikie <dblaikie at
gmail.com> wrote:
> >>
> >> > There seems to be some uncertainty about the use of smart
pointers
> >> > (previously OwningPtr, now std::unique_ptr and
std::shared_ptr
> >> > predominantly) in the LLVM project as a whole, so here's
a thread to
> >> > discuss/clarify/etc the project preferences/direction with
regard to
> >> > smart pointer usage.
> >> >
> >> > For some context, see discussions in LLVM r212403 and Clang
r213307.
> >> >
> >> > The basic question here seems to be whether smart pointer
ownership is
> >> > a direction we want to take the LLVM project in general.
> >> >
> >> > I've seen others contribute and have myself contributed
many patches
> >> > moving towards smart pointer ownership (both in the pre-C++11
days of
> >> > OwningPtr, and much moreso in the post-C++11 world with
> >> > std::unique_ptr and std::shared_ptr being usable inside
containers, as
> >> > return values, etc, allowing many more opportunities).
> >> >
> >> > std::unique_ptr's been used in LLD as far back as
r153620.
> >> > std::unique_ptr appeared in LLVM shortly after the C++11
switch with
> >> > Ahmed's work to migrate the project from OwningPtr to
std::unique_ptr
> >> > (starting with r202609 and ending with r211259). Originally
OwningPtr
> >> > was added in r45261.
> >> > Something in the order of 60 changes across clang and LLVM
mention
> >> > unique_ptr in their subject and migrate various APIs to use
unique_ptr
> >> > for ownership. Many of which remove uses of explicit delete
or helpers
> >> > like DeleteContainerPointers (and removing explicit dtors in
many of
> >> > those cases).
> >> >
> >> > Are people OK with/prefer the use of owning smart pointers in
APIs?
> >> > Are there places where you've found them to be too
noisy/burdensome
> >> > and would rather use raw pointers or some other abstraction?
Would you
> >> > prefer pre-commit review of such changes to adequately
consider
> >> > alternatives (such as?)?
> >> >
> >> > Thanks,
> >> > - David
> >> > _______________________________________________
> >> > cfe-dev mailing list
> >> > cfe-dev at cs.uiuc.edu
> >> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
> >>
> >> _______________________________________________
> >> cfe-dev mailing list
> >> cfe-dev at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
> >
> >
> >
> > _______________________________________________
> > cfe-dev mailing list
> > cfe-dev at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
> >
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140718/60249da6/attachment.html>