3

While reading through a code-base, I spotted a class that addresses the common "requiring objects on heap" problem by having a protected destructor. The class is also abstract, allowing a single instance creation method through a static function.

You can see a reduced version of the pattern below. I have a question regarding the return type of ::Make:

 // header file --------------------------------------
 namespace ooo {

 class Object {
 public:
   Object() { std::cout << "Constructor call\n"; }
   virtual void F() = 0;

   static std::unique_ptr<Object, void (*)(Object *)> Make();

 protected:
   virtual ~Object() { std::cout << "Destructor call\n"; }
 };

 } // namespace ooo

 // implementation file ----------------------------
 namespace {

 class ObjectImpl : public ooo::Object {
 public:
   void F() override { std::cout << "doing stuff\n"; }
   ~ObjectImpl() override { std::cout << "destructor of derived\n"; }
 };

 } // namespace

 namespace ooo {

 std::unique_ptr<Object, void (*)(Object *)> Object::Make() {
   return std::unique_ptr<Object, void (*)(Object *)>{
       new ObjectImpl, [](Object *e) { delete e; }}; 
 }

 } // namespace ooo

 void Use() {
   // The design of class Object, should prevent users from doing the following:
   //  
   // 1. Explicit allocation - creating instance in free store:
   // std::unique_ptr<Envelope> ptr{new Envelope}; // compilation error due to
   // private destructor.
   //  
   // 2. Plain automatic instance:
   // Envelope e; // Compilation error due to private destructor.
   //  
   // 3. Plain free store instance:
   // auto ee = new ooo::Object; // You can't have an instance of abstract class.

   // This is the use pattern we want to enable
   auto e = ooo::Object::Make();
 }

 int main() {
   std::cout << "Entering example\n\n";
   Use();

   return 0;
 }

As you can see, ::Make() returns a uniqe pointer, whose type has to state explicitly (we can't use automatic return types because users are only expected to include the header).

Since the type of the deleter is a pointer to a function, and we use a lambda as a deleter, won't the unique pointer capture a pointer to a dying object at the end?

As a side note, I believe the lambda is used because due to scope (it's declared inside the static function) it has access to the protected destructor.

Jarod42
  • 203,559
  • 14
  • 181
  • 302
Lorah Attkins
  • 5,331
  • 3
  • 29
  • 63
  • 2
    lambda doesn't capture anything (in `[](Object *e) { delete e; }`)... – Jarod42 Jan 22 '20 at 14:06
  • 1
    TL;DR of the dupe: Yor're fine. The compiler basically makes a global function that does the same thing the lambda does and the pointer points to that function. – NathanOliver Jan 22 '20 at 14:09
  • @NathanOliver: I don't think this should be closed as a dupe. That "dupe" is significantly more obscure in wording of the question and answer. It takes a good bit of transformation and reading between the lines to get to the answer to this question. – metal Jan 22 '20 at 14:27
  • @Jarod42 I'm not at all concerned what the lambda captures; as a side note it wouldn't convert to a function pointer if it did capture anything. My concern (and ok, I admit using the word capture is a bit wrong here) is **what the unique pointer captures**, i.e. what the unique pointer's deleter member is going to be. Maybe I'd have described it better if I said: **Is there a problem with the thing that sinks inside the unique pointer**? Judging by the certainty of comments, I should still be fine right? – Lorah Attkins Jan 22 '20 at 15:08
  • @LorahAttkins: There is no issue with the deleter. It is purely an access thing. No one but an `Object` itself or a subclass is allowed at compile-time to delete an `Object`. The lambda in the context written has that access. If someone were to call `unique_ptr::get_deleter()`, they could potentially circumvent the protection here and delete an `Object` manually using it, but you can only keep honest people honest. :-) If you were to move the lambda *outside* of the `Make()` function, it no longer has access and [fails at compile-time](http://coliru.stacked-crooked.com/a/5b7618be06b045b2). – metal Jan 22 '20 at 18:20

1 Answers1

2

There will be no dangling reference. The lambda capture is specified by what's in [] that starts the lambda. Here, it is empty, meaning it does not capture anything. This lambda is thus equivalent to a free-standing (or static member) function.

The code could have been written like this to get the same effect, but with less concision:

 // static
 void Object::Destroy( Object* e ) { delete e; }

 std::unique_ptr<Object, void (*)(Object *)> Object::Make() {
   return std::unique_ptr<Object, void (*)(Object *)>{
       new ObjectImpl, Destroy }; 
 }

See it live on Coliru.

While a user of the class who is determined to do things the wrong way can find a way to abuse the system (say, by calling unique_ptr::get_deleter()), the attempt here is to follow Scott Meyers' dictum from Effective C++: "Make interfaces easy to use correctly and hard to use incorrectly."

metal
  • 6,202
  • 1
  • 34
  • 49