
Modernize C++: Sink Functions - joebaf
http://www.bfilipek.com/2017/02/modernize-sink-functions.html
======
ndh2
I didn't see any valuable information here besides "Use unique_ptr instead of
raw pointers". The rest is noise, and a contrived example. It seems like a
profile post written to impress recruiters.

~~~
Zarathust
a shared_ptr would probably also be a better choice if the ptr should go
through several nodes of the pipeline

~~~
ajross
A shared_ptr is a reference counted allocator, which has nontrivial runtime
overhead. It's not what you want when you just want to pass down a reference
to some object allocated as part of an enclosing stack frame.

The problem in the article is that the code has a firm RAII promise that the
higher level object _will_ be destroyed correctly (i.e. never before any
called functions use it). But the author wants to add safety by ensuring that
the called functions aren't able to store or copy the pointer or otherwise
muck with that allocation promise. And the chosen syntax with unique_ptr works
well enough I guess (though it will break if you need the same object used
across two disjoint subtrees of the calll tree).

Personally? If this is really what you want then Rust is probably a better
proposition as it doesn't require extra syntax for this case. Is it really
what you want? Dunno. A simple application of "const" to a few places can get
you maybe 70% of the way to the goal here in designs like this.

~~~
adrianratnapala
> though it will break if you need the same object used across two disjoint
> subtrees of the calll tree

Yes, while I prefer to use plain values or unique_ptrs when I can, I will use
C++ references (or plain pointers) in this sort of situation. Basically in C++
I think an of old-style pointers/refs as Rust-like "borrowed" references.

This is an example of how the Rust safety model is really about compiler
enforcement of good practices from C and C++ (as opposed to, say, GC which is
a distinct practice). I appreciate what Rust is doing, but it is also true
that good coding conventions in C++ will get you 90% of the way there.

~~~
duneroadrunner
> good coding conventions in C++ will get you 90% of the way there

For those that need it, SaferCPlusPlus conventions will get you even further.

> Basically in C++ I think an of old-style pointers/refs as Rust-like
> "borrowed" references.

They key safety feature of Rust references is that they have "scope lifetime"
and their target is guaranteed to be alive for the duration of that scope
lifetime. You can use SaferCPlusPlus' "scope pointers"[1] to achieve this in
C++.

Scope pointers can be explicitly converted to raw pointers, so they can be
used with existing "legacy" functions. But you get even more benefit when you
change your function interfaces to support scope pointers directly[2]. For
example, it would prevent the function from (inappropriately) putting a copy
of the scope pointer into "long term storage" (like Rust does).

[1] shameless plug: [https://github.com/duneroadrunner/SaferCPlusPlus#scope-
point...](https://github.com/duneroadrunner/SaferCPlusPlus#scope-pointers)

[2] [https://github.com/duneroadrunner/SaferCPlusPlus#safely-
pass...](https://github.com/duneroadrunner/SaferCPlusPlus#safely-passing-
parameters-by-reference)

------
smitherfield
I'd think of smart pointers as function parameters as a code smell.[0] Heap
allocations should be managed with an RAII class (of which smart pointers may
be an internal implementation detail), and "sink functions" should take an
rvalue reference (which semantically means "take ownership of this
parameter").

[0] Whether a resource is allocated on the heap or elsewhere, or, whether a
heap-allocated resource has owned or reference-counted semantics, are
implementation details that should be abstracted away from your top-level
APIs, so you have the flexibility to change them later.

~~~
vvanders
Honestly, everyone reaches for shared_ptr way too quickly/easily. In the
majority of cases it's a sign that you don't have a clear owner of a component
and you've just deferred the decision to whatever (non-)deterministic behavior
your code has.

I've found using unqiue_ptr + proper move semantics tend to lead to cleaner,
single-owner architectures that are much more sane to maintain/scale.

~~~
smitherfield
Agreed; probably upwards of 90% of the uses of shared_ptr I've seen (and 70%
or so with unique_ptr) are by people who clearly don't understand the C/C++
memory model and are trying to write Java in C++, and half the remainder are
by people who've lost track of ownership and lifetimes and paper over it by
randomly sprinkling smart pointers throughout the codebase until it sort of
works.

I'm not against smart pointers where they make sense, it's just whenever I see
them I start bracing myself for the horrors that seem to inevitably follow.

~~~
vvanders
Yeah, it's also one of my big gripes about Java or any managed language. You
never know if some jerk library grabbed a reference to your object when you
passed it to a function and decides to leak it via holding a ref.

It's also why I really like that Rust decided to go RAII/single-ownership and
add a ton of friction(via RefCell) to make sure that you _really_ want shared
ownership. It forces you to get your house in order up-front which I've found
almost always leads to a better design overall.

~~~
smitherfield
RAII really is a brilliant design, it both (mostly) solves the problem of
manual memory management and is easier (for my brain at least) to reason about
than deterministic (shared_ptr, Obj-C/Swift) or nondeterministic (managed
langs) garbage collection schemes. Once I grokked it I've never looked at
managed langs the same way again.

In C++'s defense it's not like the language or the compilers encourage you to
use shared_ptr, but it's also true (mainly for legacy C-interop cruft) it
doesn't do enough to nudge you to write code in a proper RAII style, nor to
enforce correctness WRT ownership.

I like that Rust forces you to explicitly opt out of RAII and/or single-owner
semantics. (Those are the "defaults" in C++ too, but too easy to implicitly or
accidentally opt out of). On the other hand you can write Java in any
language, and from the little Rust I've read, newcomers from managed langs
will often daisy-chain enough smart pointers in a row to put any 13-year-old
who just downloaded Unreal Engine to shame.

The last time I took a serious look at Rust I thought it took things too far
in the other direction; you weren't able to use two (nonoverlapping!) data
members of the same struct in a function, and the suggested workaround was to
wrap it in a smart pointer. Has that issue been fixed now?

~~~
vvanders
> The last time I took a serious look at Rust I thought it took things too far
> in the other direction; you weren't able to use two (nonoverlapping!) data
> members of the same struct in a function, and the suggested workaround was
> to wrap it in a smart pointer. Has that issue been fixed now?

I think that's largely been addressed. Are you talking about doing something
like this?

[https://play.rust-
lang.org/?gist=cf5835d28946df6c5093d57b7ca...](https://play.rust-
lang.org/?gist=cf5835d28946df6c5093d57b7ca0aba1&version=stable)

~~~
smitherfield
It's long enough ago I can't remember exactly what I was trying to do except
that it was trivially correct, but yeah I think that was it. Anyway, I'm glad
that's been solved; looks like I've got some new weekend plans! :)

~~~
Sharlin
Closures still borrow whole structs from the captured scope when they could
only borrow the fields they actually need:

[https://play.rust-
lang.org/?gist=26b2efa859589a8172cad982f0d...](https://play.rust-
lang.org/?gist=26b2efa859589a8172cad982f0dfb89b&version=stable)

This is especially frustrating when it's self that is being captured. There's
an accepted RFC for fixing this but I'm not sure when it's going to be
implemented.

~~~
vvanders
You can work-around that by capturing the fields you need individually. It's a
little awkward but does let you express what you're trying to do there:

[https://play.rust-
lang.org/?gist=2463ff559d7f96f916d246272b1...](https://play.rust-
lang.org/?gist=2463ff559d7f96f916d246272b19ff68&version=stable)

Agreed that it's not the best default behavior though. Generally I tend to try
to keep unrelated things in separate structs for this reason(with the nice
side benefit of keeping concerns separate).

------
halayli
Relevant:

[https://herbsutter.com/2013/06/05/gotw-91-solution-smart-
poi...](https://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-
parameters/)

------
dpc_pw
Isn't that just Rust's move semantics + `Box`?

Instead of improving a detail, you can just improve everything and move to
Rust... :>

