10

We're using the curiously recurring template pattern to implement singletons. However, with recent Clang versions we're getting a -Wundefined-var-template warning. The suggested fix to which is to add an "explicit instantiation declaration".

I attempted to do this, but then I get errors about "explicit specialization after instantiation" in the compilation unit where the definition of the singleton template class member variable is.

What's the appropriate construct to fix the issue highlighted by this warning?

Simplified Details (much of the logic has been removed, to make a MCVE):

SingletonBase.hh:

template < class T > class SingletonBase {
public:
  static T * get_instance() {
    if ( ! instance_ ) {
      instance_ = T::create_singleton_instance();
    }
    return instance_;
  }
private:
  static T * instance_;
};

Singleton.hh:

#include "SingletonBase.hh"

class Singleton : public SingletonBase< Singleton > {
  friend class SingletonBase< Singleton >;
public:
  int do_stuff(int v) { return v+2; }
private:
  static Singleton * create_singleton_instance() {
    return new Singleton;
  }
};

Singleton.cc:

#include "Singleton.hh"
template <> Singleton * SingletonBase< Singleton >::instance_( nullptr );

When we compile with a recent version of clang (3.9.0; but not with clang 3.7), we get an warning when compiling files other than Singleton.cc. (with -std=c++11 and -Werror)

In file included from OtherFile.cc:2:
In file included from ./Singleton.hh:2:
./SingletonBase.hh:5:16: warning: instantiation of variable 'SingletonBase<Singleton>::instance_' required here, but no definition is available [-Wundefined-var-template]
        if ( ! instance_ ) {
               ^
OtherFile.cc:5:25: note: in instantiation of member function 'SingletonBase<Singleton>::get_instance' requested here
      return Singleton::get_instance()->do_stuff(4);
                        ^
./SingletonBase.hh:11:18: note: forward declaration of template entity  is here
       static T * instance_;
             ^

./SingletonBase.hh:5:16: note: add an explicit instantiation declaration to suppress this warning if 'SingletonBase<Singleton>::instance_' is explicitly instantiated in another translation unit
        if ( ! instance_ ) {
               ^
1 error generated.

I added the following line to the end of Singleton.hh, as it's what I'm lead to believe the explicit instantiation declaration syntax should be. 

extern template Singleton* SingletonBase< class Singleton >::instance_;

While that fixes issues with compiling OtherFile.cc, it results in a new error when compiling Singleton.cc

Singleton.cc:3:57: error: explicit specialization of 'instance_' after instantiation
    template <> Singleton * SingletonBase< Singleton >::instance_( nullptr );
                                                    ^
./Singleton.hh:14:66: note: explicit instantiation first required here
    extern template Singleton* SingletonBase< class Singleton >::instance_;
                                                             ^
1 error generated.

What should I be doing here to fix these warnings/errors? Is there a more appropriate syntax for the explicit instantiation declaration that I'm not understanding?

Community
  • 1
  • 1
R.M.
  • 3,461
  • 1
  • 21
  • 41
  • 2
    But why oh why? You created a static member var and not lock, thus it's not thread-safe. You could just put that static in `get_instance()`, spare the if, have it thread-safe. Your base class doesn't ensure that this is a singleton, yet it requires a create_instance function. I can, at any point, create two 'Singleton' instances w/o even touching your interface. You cannot destroy-recreate it. You can have base & descendant singletons at the same time. You're leaking and never call the destructor. The fix? Delete these sources and start over. (No offense, we can guide you to have a proper one) – lorro Sep 09 '16 at 22:57
  • @Iorro Keep in mind that I've ripped out most of the details (including the thread-safety bits) to give you a MCVE. If you have a Singleton implementation you consider "best practices", feel free to link it in a comment, but my original question still stands. – R.M. Sep 09 '16 at 23:26
  • This is _very_ important, I suggest you add a comment on that in the main question section, otherwise people will try to fix these, not the compiler error. Does the member variable -> function static variable fix work for you, or do you have reasons to keep it a member? – lorro Sep 10 '16 at 17:22

3 Answers3

5

The simplest fix is to define instance_ in SingletonBase.hh:

template < class T > class SingletonBase {
public:
  static T * get_instance() {
    if ( ! instance_ ) {
      instance_ = T::create_singleton_instance();
    }
    return instance_;
  }
private:
  static T * instance_;
};

template <typename T>
T* SingletonBase<T>::instance_ = nullptr;

However, I don't see the point of SingletonBase if you are going to rely on T::create_singleton_instance() to create the instance. You might as well implement get_instance() in the derived class.

Using a CRTP to implement the singleton pattern makes sense only if the base class can construct an instance of the derived class using the default constructor.

template < class T > class SingletonBase {
   public:
      static T& get_instance() {
         static T instance_;
         return instance_;
      }
   private:
};

Further reading: How to implement multithread safe singleton in C++11 without using <mutex>

Community
  • 1
  • 1
R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • Unfortunately, this is not always an option. If the singleton is used in multiple translation units and in multiple dynamic libraries, the instance is created multiple times. – Alejandro Mar 10 '22 at 10:55
2

I would recommend this implementation of a singleton instead:

template<class T>
struct SingletonBase {
    static T& get_instance() {
        static T instance;
        return instance;
    }
};

It's thread safe and remove your warning.

If you want, you can keep your create_singleton_instance:

template<class T>
struct SingletonBase {
    static T& get_instance() {
        static T instance{T::create_singleton_instance()};
        return instance;
    }
};

And changing the function implementation to:

static SomeClass create_singleton_instance() {
    return {};
}
Guillaume Racicot
  • 39,621
  • 9
  • 77
  • 141
  • .. except that it's not a _single_ton. You have not warranted a single instance at this point. – lorro Sep 10 '16 at 17:24
  • 1
    Then juste make the constructor private and friend the parent. – Guillaume Racicot Sep 10 '16 at 18:35
  • The proposed solution breaks something awful on Microsoft Windows 8 and below with Visual Studio 2015 and below. Microsoft does not provide ["magic statics"](https://msdn.microsoft.com/en-us/library/hh567368.aspx) a.k.a. N2660 [Dynamic Initialization and Destruction with Concurrency](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2660.htm). Changing compilers does not fix it. The problem is with the runtime and the platform. Its a core language feature that took almost a decade to arrive. Microsoft weaponized the defect and use it to force people to upgrade to Windows 10 to get it. – jww Aug 16 '17 at 14:33
  • 1
    @jww that's unfortunate. However, I won't write a suboptimal solution because a platform has defects and lack of packport. This is still the best solution for a singleton and will likely stay like that. However, I'd write a note to OP if it was tagged with `msvc` or something like that. As it's tagged `clang++`, there is not known problem with this idiom. – Guillaume Racicot Aug 16 '17 at 15:26
1

Apparently, the explicit instantiation declaration should be of the form

template <> Singleton * SingletonBase< Singleton >::instance_;
skimo
  • 11
  • 1