title: How to allow a std::unique_ptr to access a class's private destructor or implement a C++ factory class with a private destructor?
body: <p>I'm quite far into the development of a game using SDL, OpenGL and C++ and am looking for ways to optimize the way the game switches between GLSL shaders for lots of different objects of different types. This is much more of a C++ question than an OpenGL question. However, I still want to provide as much context as I can, as I feel some justification is needed as to why the proposed Shader class I need, needs to be created / deleted the way that it is.</p>
  <p>The first four sections are my justifications, journey &amp; attempts leading up to this point, however my question can likely be answered by just the final section alone and I've intentionally written it as a bit of a tldr.</p>
  <h2>The necessity for a Shader class:</h2>
  <p>I've seen many implementations online of OpenGL shaders being created, compiled and deleted all in the same function when game objects are created during gameplay. This has proven to be inefficient and far too slow in particular sections of my game. Thus, I've required a system that creates and compiles shaders during a load-time and then intermittently uses/swaps between them during game-time before being deleted later.</p>
  <p>This has lead to the creation of a class(<code>Shader</code>) that manages OpenGL shaders. Each instance of the class should manage one unique OpenGL shader each and contains some complex behavior around the shader type, where it's loaded in from, where it's used, the uniform variables it takes, etc.</p>
  <p>With this said, the most important role of this class is to store the <code>GLuint</code> variable <code>id</code> that is returned from <code>glCreateShader()</code>, and manage all OpenGL calls that relate to the OpenGL shader with this <code>id</code>. I understand that this is effectively futile given the global nature of OpenGL(as anywhere in the program could technically call <code>glDeleteShader()</code> with the matching <code>id</code> and break the class), however for the purposes of intentionally encapsulating all OpenGL calls to very specific areas throughout the entire codebase this system will <em>drastically</em> reduce code-complexity.</p>
  <h2>Where the problems start...</h2>
  <p>The most &quot;automatic&quot; way to manage this <code>GLuint id</code>, would be to invoke <code>glCreateShader()</code> on the object's construction and <code>glDeleteShader()</code> on the object's destruction. This guarantees(within OpenGL limits) that the OpenGL shader will exist for the entire lifetime of the C++ Shader object and eliminates the need to call some <code>void createShader()</code> and <code>deleteShader()</code> functions.</p>
  <p>This is all well and good, however problems soon arise when considering what happens if this object is copied. What if a copy of this object is destructed? That means that <code>glDeleteShader()</code> will be called and effectively break all copies of the shader object.</p>
  <p>What about simple mistakes like accidentally invoking <code>std::vector::push_back()</code> in a vector of Shaders? Various <code>std::vector</code> methods can invoke the constructor / copy constructor / destructor of their type, which can result in the same problem as above.</p>
  <p>Okay then... how about we do create some <code>void createShader()</code> and <code>deleteShader()</code> methods even if it's messy? Unfortunately this just defers the above problem, as once again <em>any</em> calls that modify the OpenGL shader will desynchronize / outright break all copies of a shader class with the same <code>id</code>. I've limited the OpenGL calls to <code>glCreateShader()</code> and <code>glDeleteShader()</code> in this example to keep things simple, however I should note that there are many other OpenGL calls in the class that would make creating various instance/static variables that keep track of instance copies far too complicated to justify doing it this way.</p>
  <p>The last point I want to make before jumping into the class design below is that for a project as large as a raw C++, OpenGL and SDL Game, I'd prefer if any potential OpenGL mistakes I make generate compiler errors versus graphical issues that are harder to track down. This can be reflected in the class design below.</p>
  <h2>The first version of the <code>Shader</code> class:</h2>
  <p>It is for the above reasons that I have elected to:</p>
  <ul>
  <li>Make the constructor <code>private</code>.</li>
  <li>Provide a public <code>static create</code> function that returns a pointer to a new Shader object in place of a constructor.</li>
  <li>Make the copy constructor <code>private</code>.</li>
  <li>Make the <code>operator=</code> <code>private</code> (Although this might not be necessary).</li>
  <li>Make the destructor private.</li>
  <li>Put calls to <code>glCreateShader()</code> in the constructor and <code>glDeleteShader()</code> in the destructor, to have OpenGL shaders exist for the lifetime of this object.</li>
  <li>As the <code>create</code> function invokes the <code>new</code> keyword(and returns the pointer to it), the place with the outside call to <code>Shader::create()</code> must then invoke <code>delete</code> manually (more on this in a second).</li>
  </ul>
  <p>To my understanding, the first two bullet points utilize a factory pattern and will generate a compiler error should a non-pointer type of the class be attempted to be created. The third, fourth and fifth bullet points then prevent the object from being copied. The seventh bullet point then ensures that the OpenGL Shader will exist for the same lifetime of the C++ Shader object.</p>
  <h2>Smart Pointers and the main problem:</h2>
  <p>The only thing I'm not a huge fan of in the above, is the <code>new</code>/<code>delete</code> calls. They also make the <code>glDeleteShader()</code> calls in the destructor of the object feel inappropriate given the encapsulation that the class is trying to achieve. Given this, I opted to:</p>
  <ul>
  <li>change the <code>create</code> function to return a <code>std::unique_ptr</code> of the <code>Shader</code> type instead of a <code>Shader</code> pointer.</li>
  </ul>
  <p>The <code>create</code> function then looked like this:</p>
  <pre><code>std::unique_ptr&lt;Shader&gt; Shader::create() {
      return std::make_unique&lt;Shader&gt;();
  }
  </code></pre>
  <p>But then a new problem arose... <code>std::make_unique</code> unfortunately requires that the <em>constructor</em> is <code>public</code>, which interferes with the necessities described in the previous section. Fortunately, I found a solution by changing it to:</p>
  <pre><code>std::unique_ptr&lt;Shader&gt; Shader::create() {
      return std::unique_ptr&lt;Shader&gt;(new Shader());
  }
  </code></pre>
  <p>But... now <code>std::unique_ptr</code> requires that the <em>destructor</em> is public! This is... better but unfortunately, this means that the destructor can be manually called outside of the class, which in turn means the <code>glDeleteShader()</code> function can be called from outside the class.</p>
  <pre><code>Shader* p = Shader::create();
  p-&gt;~Shader(); // Even though it would be hard to do this intentionally, I don't want to be able to do this.
  delete p;
  </code></pre>
  <h2>The final class:</h2>
  <p>For the sake of simplicity, I have removed the majority of instance variables, function/constructor arguments &amp; other attributes but here's what the final proposed class (mostly)looks like:</p>
  <pre><code>class GLSLShader {
  
  public:
      ~GLSLShader() { // OpenGL delete calls for id }; // want to make this private.
  
      static std::unique_ptr&lt;GLSLShader&gt; create() { return std::unique_ptr&lt;GLSLShader&gt;(new GLSLShader()); };
  
  private:
      GLSLShader() { // OpenGL create calls for id };
  
      GLSLShader(const GLSLShader&amp; glslShader);
      GLSLShader&amp; operator=(const GLSLShader&amp;);
  
      GLuint id;
  
  };
  </code></pre>
  <p>I'm happy with everything in this class, aside from the fact that the destructor is public. I've put this design to the test and the performance increase is very noticeable. Even though I can't imagine I'd ever accidentally manually call the destructor on a <code>Shader</code> object, I don't like that it is publicly exposed. I also feel that I might accidentally miss something, like the <code>std::vector::push_back</code> consideration in the second section.</p>
  <p>I've found two potential solutions to this problem. I'd like some advice on these or other solutions.</p>
  <ol>
  <li><p>Make <code>std::unique_ptr</code> or <code>std::make_unique</code> a <code>friend</code> of the <code>Shader</code> class. I've been reading threads such as <a href="https://stackoverflow.com/questions/33905030/how-to-make-stdmake-unique-a-friend-of-my-class">this one</a>, however this is to make the constructor accessible, rather than the destructor. I also don't quite understand the downsides / extra considerations needed with making <code>std::unique_ptr</code> or <code>std::make_unique</code> a <code>friend</code> (The top answer to that thread + comments)?</p>
  </li>
  <li><p>Not use smart pointers at all. Is there perhaps a way to have my <code>static create()</code> function return a raw pointer(using the <code>new</code> keyword), that is automatically deleted inside the class / when the <code>Shader</code> goes out of scope and the destructor is called?</p>
  </li>
  </ol>
  <p>Thank you very much for your time.</p>