Eli Bendersky
2014-Jul-18 20:35 UTC
[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>
Eric Christopher
2014-Jul-18 23:30 UTC
[LLVMdev] [cfe-dev] Use of Smart Pointers in LLVM Projects
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 >
Lang Hames
2014-Jul-18 23:39 UTC
[LLVMdev] [cfe-dev] Use of Smart Pointers in LLVM Projects
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>