In this case moving implementation of `Singleton<T>::getInstance()` into a .cpp file would prevent compiler from instantiation of the method body when it sees `Singleton<Foo>::getInstance()`. In this case `Singleton<Foo>::getInstance()` must be instantiated in some source file either explicitly or implicitly. If inline implementation for `Singleton<T>::getInstance()` is provided in the header, where `Singleton<T>` is defined, the method body is instantiated implicitly when it is required. But the static field `instance` referenced in the method still must be instantiated in some source file explicitly or implicitly. So from viewpoint of convenience, moving the implementation of template `Singleton<T>::getInstance()` into source file does not look as more inflexible solution.
Probably it is more convenient to put the template for the static member to header file too: template <typename T> T *Singleton<T>::instance = nullptr; In this case both the method and corresponding static member are instantiated implicitly by compiler, no warning occurs. Can it be an acceptable solution? If there is intention to limit `Singleton` to some selected types, explicit instantiation declaration may help. Adding the declarations like: extern template Foo *Singleton<Foo>::instance; prevents from warnings. Or you think that the message should propose moving static member definition into header file as well? Thanks, --Serge 2016-05-19 9:15 GMT+06:00 Sean Silva <chisophu...@gmail.com>: > > > On Thu, Apr 21, 2016 at 12:44 AM, Serge Pavlov <sepavl...@gmail.com> > wrote: > >> Let me demonstrate the problem using excerpt from v8 sources: >> >> ------ lithium.h ---------------------------------------------------- >> template <int kOperandKind, int kNumCachedOperands> >> struct LSubKindOperand { >> static int* Create(int index) { return &cache[index]; } >> static void SetUpCache(); >> static int* cache; >> }; >> >> struct LOperand { >> static void SetUpCaches(); >> }; >> >> #define LITHIUM_OPERAND_LIST(V) \ >> V(DoubleRegister, 1, 16) >> >> #define LITHIUM_TYPEDEF_SUBKIND_OPERAND_CLASS(name, type, number) \ >> typedef LSubKindOperand<type, number> L##name; >> LITHIUM_OPERAND_LIST(LITHIUM_TYPEDEF_SUBKIND_OPERAND_CLASS) >> /* Expands to: typedef LSubKindOperand<1, 16> LDoubleRegister; */ >> #undef LITHIUM_TYPEDEF_SUBKIND_OPERAND_CLASS >> >> ------ lithium.cc---------------------------------------------------- >> #include "lithium.h" >> >> template<int kOperandKind, int kNumCachedOperands> >> int* LSubKindOperand<kOperandKind, kNumCachedOperands>::cache = 0; >> >> template<int kOperandKind, int kNumCachedOperands> >> void LSubKindOperand<kOperandKind, kNumCachedOperands>::SetUpCache() { >> cache = new int[kNumCachedOperands]; >> } >> >> void LOperand::SetUpCaches() { >> #define LITHIUM_OPERAND_SETUP(name, type, number) L##name::SetUpCache(); >> LITHIUM_OPERAND_LIST(LITHIUM_OPERAND_SETUP) >> /* Expands to: LDoubleRegister::SetUpCache(); */ >> #undef LITHIUM_OPERAND_SETUP >> } >> >> ------ lithium-x64.cc ----------------------------------------------- >> #include "lithium.h" >> >> int* GetNextSpillSlot(int kind) { >> return LSubKindOperand<1,16>::Create(0); >> } >> >> int main(int argc, char *argv[]) { >> return 0; >> } >> >> ------ build.sh ----------------------------------------------------- >> g++ lithium.cc lithium-x64.cc >> --------------------------------------------------------------------- >> >> When compiler generates code for 'GetNextSpillSlot' it implicitly >> instantiates the method 'Create'. It can do it as definition of the >> template entity 'LSubKindOperand::Create' is available from lithium.h. Then >> it attempts to instantiate static field 'LSubKindOperand::cache', as it is >> used in the body of just instantiated method 'Create'. But definition of >> this template entity is not available while compiling lithium-x64.cc. >> Failure to implicitly instantiate a template is not an error, but this >> template must be instantiated somewhere else otherwise linker founds >> undefined references. >> >> Indeed, 'LSubKindOperand<1,16>::cache' is instantiated while compiling >> lithium.cc. Method 'LOperand::SetUpCaches' is not a template, it references >> 'LSubKindOperand::SetUpCache', which in turn uses 'LSubKindOperand::cache'. >> Definitions of these templates are available in lithium.cc, compiler >> instantiates them and the program builds successfully. This solution is >> fragile however, if lithium-x64.cc referenced >> 'LSubKindOperand<1,1>::Create', the build would break because in lithium.cc >> this specialization is not instantiated. >> >> Putting templates definitions into source files rather than headers is >> similar to moving forward declarations into source files. It makes sense if >> user wants to encapsulate some functionality. In this example he should >> also move the definition 'LSubKindOperand::Create' to source file, just for >> the sake of encapsulation. This would prevent compiler from implicit >> instantiation of this method and thus from instantiation of the static >> field as well. >> >> If encapsulation is not an aim, moving template definition into header >> seems to be more natural solution. It would allow for compiler to >> instantiate all entities implicitly. >> >> Static variables of template classes are a source of confusion for some >> reason. Probably people tend to confuse them with static member template >> specializations. Anyway this type of errors is too widespread, the >> aforementioned https://llvm.org/bugs/show_bug.cgi?id=24425 is just one >> example. The change introduced in r266719 attempts to provide user with >> some assistance in these cases. >> >> I don't know details about Singleton<T>, but only existence of static >> fields should not produce the warning. There must be a reference to the >> field in some method defined inline. Solution could be similar: either the >> method that references the static field should be encapsulated into source >> file or the static field template should be exposed in header. >> > The use case of Singleton<T> I gave wasn't complete. It was more like this: > > template <typename T> > struct Singleton { > ... other stuff ... > static T *getInstance() { return Singleton<T>::instance; } > private: > static T *instance; > }; > > Making the method private to a .cpp file would defeat the entire purpose > of having the class. > > -- Sean Silva > > >> 2016-04-21 5:36 GMT+06:00 Sean Silva <chisophu...@gmail.com>: >> >>> >>> >>> On Tue, Apr 19, 2016 at 7:28 AM, Nico Weber via cfe-commits < >>> cfe-commits@lists.llvm.org> wrote: >>> >>>> (sorry, accidentally sent this mid-mail) >>>> >>>> ../../v8/src/crankshaft/lithium.h:322:45: error: instantiation of >>>> variable >>>> 'v8::internal::LSubKindOperand<v8::internal::LOperand::Kind::DOUBLE_STACK_SLOT, >>>> 128>::cache' required here, but no definition is available >>>> [-Werror,-Wundefined-var-template] >>>> if (index < kNumCachedOperands) return &cache[index]; >>>> ^ >>>> ../../v8/src/crankshaft/x64/lithium-x64.cc:334:30: note: in >>>> instantiation of member function >>>> 'v8::internal::LSubKindOperand<v8::internal::LOperand::Kind::DOUBLE_STACK_SLOT, >>>> 128>::Create' requested here >>>> return LDoubleStackSlot::Create(index, zone()); >>>> ^ >>>> ../../v8/src/crankshaft/lithium.h:335:27: note: forward declaration of >>>> template entity is here >>>> static LSubKindOperand* cache; >>>> ^ >>>> ../../v8/src/crankshaft/lithium.h:322:45: note: add an explicit >>>> instantiation declaration to suppress this warning if >>>> 'v8::internal::LSubKindOperand<v8::internal::LOperand::Kind::DOUBLE_STACK_SLOT, >>>> 128>::cache' is explicitly instantiated in another translation unit >>>> if (index < kNumCachedOperands) return &cache[index]; >>>> ^ >>>> >>>> I don't understand what this warning wants to tell me, or what it wants >>>> me to do. What is this warning good for? Neither warning text nor commit >>>> message explain that. >>>> >>> >>> Yes, the diagnostics here can be substantially improved I think. >>> >> >> Do you have any thoughts how the diagnostics could be improved? Any ideas >> are welcome. >> >> Thanks, >> --Serge >> > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits