[cfe-dev] Use of Smart Pointers in LLVM Projects

[Alp Toker]

On 18/07/2014 02:21, David Blaikie wrote:
> Are people OK with/prefer the use of owning smart pointers in APIs?

I think smart pointers are great to use for storage, or as struct 
members where there's actually a clear need for ownership. Just by 
virtue of getting rid of destructors containing single delete 
expressions this is already a win.

In many cases there shouldn't be a need for smart pointers at all, 
because the object is owned by a central facility's smart pointer so 
it's safe to just pass around the pointer in ephemeral contexts like 
stack-based instances. In such context raw pointers and references 
aren't scary -- they're just fine.

(All the above goes equally for uniquely owned pointers, shared pointers 
and intrusive refcounted pointers.)

> Are there places where you've found them to be too noisy/burdensome
> and would rather use raw pointers or some other abstraction?

A clear place that smart pointers *shouldn't* be used are 'create' 
functions.

There's already a strong convention in the thousands of 'create' 
functions across the codebase and they have the clear, well-defined 
behaviour of returning a new pointer.

We all know what to expect from these functions, much like new/delete in 
C++ and malloc/free. They can be easily adopted by std::shared_ptr or 
std::unique_ptr as needed, so there's no benefit in enforcing smart 
pointer usage at creation.

Apart from the complexity and visual clutter, it's pretty annoying to 
have to decide whether to get(), release() or somehow else acquire the 
object you just created. The result of using smart pointers as returns 
isn't pretty -- you end up with awkward code like 
createFoo().get()->func() or other.reset(createFoo().release()).

Transfer of new pointers from 'create' functions and usage is already 
trivially clear so we shouldn't change these.


> Would you
> prefer pre-commit review of such changes to adequately consider
> alternatives (such as?)?

The clear-cut two use-cases above are reasonable for post-commit review, 
certainly in the modules I'm helping to maintain.

The creation functions shouldn't be changed because of the reasons given 
above, and I think it makes sense to put that in policy rather than 
making the decision on a per-module basis.

Thanks for evaluating this David!

-- 
http://www.nuanti.com
the browser experts