0

I am using CRTP to create a counter class, similar to Object Counter

Additionally, classes that derive from this counter also should not be destructible.

It will look something like this

template <typename DERIVED_CLASS, std::size_t size = 0>
class Counter{
  private:
    ~Counter() = delete;
  protected:
    std::vector<DERIVED_CLASS*> created;
    Counter(){}
  public:
  //More stuff later
};

class A : public Counter<A>{
  public:
    A(){std::cout << "A created" << std::endl;}
    //More stuff later
};

A a;

int main(){
  /* All should be invalid
  A b;
  A c{a};
  A* pA = new A;
  */



  return 0;
}

The creation of a should be the only permitted usage for creating/deleting objects of these types. They should not be copyable, and should last for the entirety of the program life, therefore should not be destructed.

However, I get the the following errors and diagnostic messages

base class 'Counter<A>' has private destructor

destructor of 'A' is implicitly deleted because base class 'Counter<A>' has a deleted destructor

~Counter' has been explicitly marked deleted here

attempt to use a deleted function

Edit:

Just clarifying a few things.

Of course the object will be destructed at program exit. This is desired behaviour. It only should not be destructible by the user or during program life.

Secondly, multiple objects can be created. This is not a create once kind of thing. Many can be created, but not copied or deleted.

Edit: Code for @joerbrech's comment

template <typename DERIVED_CLASS, std::size_t size = 0>
class Counter{
  private:
    Counter(Counter const &) = delete;
    Counter& operator=(Counter const &) = delete;

  protected:
    ~Counter(){}
    Counter(){}

  public:
    template <typename... DERIVED_ARGS>
    static DERIVED_CLASS& create(DERIVED_ARGS... args){
      DERIVED_CLASS* pDerived = new DERIVED_CLASS(args...);
      //Save the pointer into a static collection
      return *pDerived;
    }
  //More stuff later
};


class A: public Counter<A>{
  using Base = Counter<A>;
  friend Base;
  A(int x){}
  //More stuff later
};

A& a = A::create(1);

int main(){
  A& b = A::create(2); //Works (bad)
  // A c(3); //Fails (good)
  // A* pA = new A(4); //Fails (good)

  (void)b; //To silence unused variable warning

  return 0;
}
Nathan29006781
  • 173
  • 1
  • 9
  • 1
    When the program ends, after the `main` function have returned, the global object `a` will be destructed. That's just how C++ works. – Some programmer dude Dec 13 '22 at 01:30
  • 2
    Just because `a` is declared in global scope doesn't prevent it from being destructed at program exit. Especially since it has a data member that has a destructor. So, your example makes no sense. If you truly want to block creation/destruction, then make `A` be a *singleton* class with `private` (not `deleted`) constructors and destructor, and then create the single instance via a `public` factory where you want it to be created. – Remy Lebeau Dec 13 '22 at 01:32
  • 1
    Perhaps what you're looking for is some kind of *singleton*, or some similar "create once" pattern? – Some programmer dude Dec 13 '22 at 01:33
  • Updated to clarify – Nathan29006781 Dec 13 '22 at 01:56
  • 2
    You don't need to `delete` the destructor of `Counter`, nor do you need to make it `private`. The object will then be destroyed as the program exits normally. It will not be destroyed before that. If a programmer chooses to explicitly destroy it before that, then the programmer is responsible for any consequences (e.g. undefined behaviour, should the object be destroyed a second time). To paraphrase Bjarne Stroustrup, you're trying to prevent actions by a Machiavellian programmer - whereas C++ features are tools to prevent problems due to Murphy, not to Machiavelli. – Peter Dec 13 '22 at 02:08
  • 1
    Aren't you just trying to create a static variable? If you desperatly want to enforce that all instances are static, you could make the constructor private, delete the copy and move ctors and have a friend function or static member function as a "factory" *(in need for a better term)* that adds a new instance to a suitable static collection and returns a reference/pointer/iterator to the created element. – joergbrech Dec 13 '22 at 04:13
  • @joergbrech, I'll probably end up with something like your solution, but I don't really like this since it involves me explicitly using `new` in the factory method, but since the variable lasts for program life, it should be ok. It also doesn't prevent me from creating an instance like `b` – Nathan29006781 Dec 13 '22 at 15:37
  • 1
    Since the constructor is private, you won't be able to create an instance like `b`. And you don't have to explicitly use `new`, you can allocate elements any way you like in the factory. By emplacing, or using smart pointers... – joergbrech Dec 13 '22 at 15:43
  • @joergbrech I'll probably use smart pointers instead of `new`, but for this example, new is easier. I can still create `b` the same way I created `a`. See the question for the code since it won't fit the comment – Nathan29006781 Dec 13 '22 at 15:59
  • 1
    With my solution this would not be possible. You would be forces to use the factory method, since ctors are private and copy and move ctors are deleted. I can try to write up an answer if you like that hopefully clears up some confusion, it just might take me some time. I am a bit busy right now... – joergbrech Dec 13 '22 at 16:03
  • @joergbrech Thanks! Whenever you're free, I'd appreciate that. I posted the code that allows me to make `b`. It does give a potential memory leak warning, but it's not enforced. – Nathan29006781 Dec 13 '22 at 16:07
  • `DERIVED_CLASS* pDerived = new DERIVED_CLASS(args...);`: If you write static before that line, you will create a static pointer that exists until the program exits, even if the function is called in main. But the pointer is always the same for every call: That's why you would need a static collection that you emplace upon new instances with every call if you want more than one instance. This is awkward to say the least, but if that is important to you, go for it...Because of `new` the destructor is never called. Memory is freed by the OS, which is bad practice. You should prefer normal RAII. – joergbrech Dec 13 '22 at 16:16
  • @Nathan29006781 having written some code (see my answer) I don't find the solution with a static collection super awkward, I think it's fine: In the end, it is exactly the singleton pattern, where the collection is the singleton, not its elements. It's up to you to decide if the safeguard against non-static use is worth the added complexity or not. – joergbrech Dec 14 '22 at 19:31

1 Answers1

1

You cannot delete a destructor. If you want your variable to not be destroyed before the program ends, you should declare it as static. If your class is only meant to every be used as global or static instances, I would just document this: If a user of your library uses it without the static keyword, they use your library wrong.

In addition, note that if your goal is to have the object counter work, you don't have to make sure that the derived classes aren't destroyed before the program ends. Since the data members objects_created and objects_alive are static in the example in your link, these variables will start to exist before the first instantiation of the derived class and cease to exist no earlier than at the end of your program. https://godbolt.org/z/14zMhv993

If you want to implement safeguards that make using the derived classes as non-static instances impossible, we can copy from the singleton pattern. Let's forget for a moment that you want to allow more than one instance of your class and implement the singleton pattern.

#include <iostream>
#include <cassert>

template <typename DERIVED_CLASS, std::size_t size = 0>
class Counter{

  protected:
    ~Counter(){ std::cout << "Counter dtor\n"; }
    Counter(){ std::cout << "Counter ctor\n"; }

  public:
    template <typename... DERIVED_ARGS>
    static DERIVED_CLASS& create(DERIVED_ARGS... args){
      static DERIVED_CLASS derived{args...};
      return derived;
    }

    //Counter is non-copyable
    Counter(Counter const &) = delete;
    Counter& operator=(Counter const &) = delete;

  //More stuff later
};


class A: public Counter<A>{
  using Base = Counter<A>;
  friend Base;
  A(int x){}
  //More stuff later
};

Now of every derived class, e.g. A there can only ever be one instance. The inner static variable derived in Counter::create will only be instantiated once in the first call to that function.

Note also, that the destructor gets called when the program exits:

int main(){

    {
        A& b = A::create(2);

        // no new instance will be created at a second call
        assert(&b == &A::create(42));
    }

    std::cout << "dtor will be called after this\n";

    return 0;
}

Output:

Counter ctor
dtor will be called after this
Counter dtor

https://godbolt.org/z/xezKhx1fE

If you need more than one instance, you can create a static collection in create and just emplace/insert into the collection with every function call. You have to use a collection, that makes sure that references to elements remain valid. This is true for std::unordered_map for example, but nor for std::vector, since std::vector has contiguous data storage and might re-allocate.

#include <unordered_map>
#include <memory>
#include <iostream>
#include <cassert>

template <typename DERIVED_CLASS, std::size_t size = 0>
class Counter{

  protected:
    Counter(){ }

  public:

    using Map = std::unordered_map<int, DERIVED_CLASS>;

    static Map& get_map() {
        static Map map;
        return map;
    }

    template <typename... DERIVED_ARGS>
    static DERIVED_CLASS& create(DERIVED_ARGS&&... args){

        Map& map = get_map();
        static int idx = 0;
      
        map.insert(
            std::make_pair(
                idx, 
                DERIVED_CLASS{std::forward<DERIVED_ARGS>(args)...}
            )
        );

        return map.at(idx++);
    }

    virtual ~Counter(){ }


    //Counter is non-copyable
    Counter(Counter const &) = delete;
    Counter& operator=(Counter const &) = delete;

    // it must be moveable though to insert into map
    Counter(Counter&&) = default;
    Counter& operator=(Counter&&) = default;

  //More stuff later
};


class A: public Counter<A>{
  using Base = Counter<A>;
  friend Base;
  A(int x){}
  //More stuff later
};

int main(){

    A& a = A::create(2);
    A& b = A::create(42);

    assert(&a == &A::get_map().at(0));
    assert(&b == &A::get_map().at(1));
    assert(&a != &b);

    return 0;
}

https://godbolt.org/z/qTPdbvsoh

The destructor of your Counter instances are called in the destructor call of the static map. The static map will be destructed when the program exits. If you want to make sure noone ever tinkers with your static map, you can make get_map private.

Addendum: Of course you can create A or Counter instances on the heap, but then prefer smart_pointers: This guarantees that the memory is freed when it should and does not need to be cleaned up by the operating system when your program exits.

joergbrech
  • 2,056
  • 1
  • 5
  • 17