Re: r266719 - Warn if function or variable cannot be implicitly instantiated

2016-08-09 Thread Serge Pavlov via cfe-commits
Whether enable this warning or not should be determined by user feedback.
The warning was implemented to help users in complicated cases that arise
in module-enabled builds. It seems however that it makes troubles for other
users. Based on feedback on this list I feel that it is better to make this
message off by default. It still should be useful for people who use
explicit instantiation, but this use case is rare, I think.

Should someone approve turning the message off by default, or discussion
here is enough?


Thanks,
--Serge

2016-08-09 20:44 GMT+07:00 Nico Weber :

> With 3.9 coming around, I'd like to suggest that we pull this warning from
> 3.9 and maybe trunk. It sounds like Sean found it only possible to deploy
> this warning since they already had a workaround for a different compiler
> bug (!). In Chromium, we can't enable this warning since one of our
> (admittedly dubiously designed) template classes in a foundational library
> requires people to have an explicit instantiation of their downstream
> classes in a client cc file. Fixing this warning would mean giving the h
> file in the foundational library forward declarations of all clients of the
> template.
>
> The motivation for this warning was PR24425, which is something that's
> only relevant with modules enabled. Maybe the warning should only fire with
> modules. But as-is, the warning warns about something that isn't really a
> problem in practice, and it's difficult to fix (and as said, fixing it has
> few benefits). I don't think it's at the bar we have for clang warnings.
>
> Is there anyone who has deployed this warning successfully on a larger
> codebase? Examples of real bugs it found?
>
> On Fri, May 20, 2016 at 12:14 AM, Sean Silva 
> wrote:
>
>>
>>
>> On Thu, May 19, 2016 at 12:13 PM, Serge Pavlov 
>> wrote:
>>
>>> In this case moving implementation of `Singleton::getInstance()` into
>>> a .cpp file would prevent compiler from instantiation of the method body
>>> when it sees `Singleton::getInstance()`. In this case
>>> `Singleton::getInstance()` must be instantiated in some source
>>> file either explicitly or implicitly. If inline implementation for
>>> `Singleton::getInstance()` is provided in the header, where
>>> `Singleton` 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::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 
>>> T *Singleton::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?
>>>
>>
>> I think that is what makes the most sense in this scenario (and it
>> simplifies things for clients of the library).
>> Unfortunately, for the library that was producing this warning they
>> already required clients to provide explicit instantiations in a .cpp file
>> (they had a macro like `DEFINE_SINGLETON_INSTANCE(T)` which a user would
>> place in a .cpp file to instantiate Singleton::instance for their type).
>>
>> So fixing this warning like this would have a compatibility concern.
>>
>> Thankfully, it seems like some compilers (not clang) have a bug in that
>> they will emit Singleton::instance any time that Singleton is
>> instantiated unless they have already seen an explicit instantiation
>> declaration of Singleton::instance in a header, so this library
>> already had (under an #if) explicit instantiations for
>> Singleton::instance for all classes that needed it in order to work
>> around this compiler bug. So in the end I fixed this by just enabling those
>> ifdefs so that clang would see the Singleton::instance explicitly
>> declared in the header.
>> (This is sort of a strange coincidence, but it worked out nicely)
>>
>>
>>
>>>
>>> If there is intention to limit `Singleton` to some selected types,
>>> explicit instantiation declaration may help. Adding the declarations like:
>>>
>>> extern template Foo *Singleton::instance;
>>>
>>>
>>> prevents from warnings.
>>>
>>> Or you think that the message should propose moving static member
>>> definition into header file as well?
>>>
>>
>> I'm not sure, but I think that the current diagnostics are not very
>> actionable. Hopefully google will lead users to your description in this
>> thread. I wish we had something like https://doc.rust-lang.org
>> /error-index.html where we could officially provide a more in-depth
>> explanation similar to your posts in this thread.
>>
>> -- Sean Silva
>>
>>
>>>
>>>
>>> Thanks,
>>> --Serge
>>>
>>> 2016-05-19 9:15 GMT+06:00 Sean 

Re: r266719 - Warn if function or variable cannot be implicitly instantiated

2016-08-09 Thread Nico Weber via cfe-commits
With 3.9 coming around, I'd like to suggest that we pull this warning from
3.9 and maybe trunk. It sounds like Sean found it only possible to deploy
this warning since they already had a workaround for a different compiler
bug (!). In Chromium, we can't enable this warning since one of our
(admittedly dubiously designed) template classes in a foundational library
requires people to have an explicit instantiation of their downstream
classes in a client cc file. Fixing this warning would mean giving the h
file in the foundational library forward declarations of all clients of the
template.

The motivation for this warning was PR24425, which is something that's only
relevant with modules enabled. Maybe the warning should only fire with
modules. But as-is, the warning warns about something that isn't really a
problem in practice, and it's difficult to fix (and as said, fixing it has
few benefits). I don't think it's at the bar we have for clang warnings.

Is there anyone who has deployed this warning successfully on a larger
codebase? Examples of real bugs it found?

On Fri, May 20, 2016 at 12:14 AM, Sean Silva  wrote:

>
>
> On Thu, May 19, 2016 at 12:13 PM, Serge Pavlov 
> wrote:
>
>> In this case moving implementation of `Singleton::getInstance()` into
>> a .cpp file would prevent compiler from instantiation of the method body
>> when it sees `Singleton::getInstance()`. In this case
>> `Singleton::getInstance()` must be instantiated in some source file
>> either explicitly or implicitly. If inline implementation for
>> `Singleton::getInstance()` is provided in the header, where
>> `Singleton` 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::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 
>> T *Singleton::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?
>>
>
> I think that is what makes the most sense in this scenario (and it
> simplifies things for clients of the library).
> Unfortunately, for the library that was producing this warning they
> already required clients to provide explicit instantiations in a .cpp file
> (they had a macro like `DEFINE_SINGLETON_INSTANCE(T)` which a user would
> place in a .cpp file to instantiate Singleton::instance for their type).
>
> So fixing this warning like this would have a compatibility concern.
>
> Thankfully, it seems like some compilers (not clang) have a bug in that
> they will emit Singleton::instance any time that Singleton is
> instantiated unless they have already seen an explicit instantiation
> declaration of Singleton::instance in a header, so this library
> already had (under an #if) explicit instantiations for
> Singleton::instance for all classes that needed it in order to work
> around this compiler bug. So in the end I fixed this by just enabling those
> ifdefs so that clang would see the Singleton::instance explicitly
> declared in the header.
> (This is sort of a strange coincidence, but it worked out nicely)
>
>
>
>>
>> If there is intention to limit `Singleton` to some selected types,
>> explicit instantiation declaration may help. Adding the declarations like:
>>
>> extern template Foo *Singleton::instance;
>>
>>
>> prevents from warnings.
>>
>> Or you think that the message should propose moving static member
>> definition into header file as well?
>>
>
> I'm not sure, but I think that the current diagnostics are not very
> actionable. Hopefully google will lead users to your description in this
> thread. I wish we had something like https://doc.rust-lang.
> org/error-index.html where we could officially provide a more in-depth
> explanation similar to your posts in this thread.
>
> -- Sean Silva
>
>
>>
>>
>> Thanks,
>> --Serge
>>
>> 2016-05-19 9:15 GMT+06:00 Sean Silva :
>>
>>>
>>>
>>> On Thu, Apr 21, 2016 at 12:44 AM, Serge Pavlov 
>>> wrote:
>>>
 Let me demonstrate the problem using excerpt from v8 sources:

 -- lithium.h 
 template 
 struct LSubKindOperand {
   static int* Create(int index) { return [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 

Re: r266719 - Warn if function or variable cannot be implicitly instantiated

2016-05-19 Thread Sean Silva via cfe-commits
On Thu, May 19, 2016 at 12:13 PM, Serge Pavlov  wrote:

> In this case moving implementation of `Singleton::getInstance()` into a
> .cpp file would prevent compiler from instantiation of the method body when
> it sees `Singleton::getInstance()`. In this case
> `Singleton::getInstance()` must be instantiated in some source file
> either explicitly or implicitly. If inline implementation for
> `Singleton::getInstance()` is provided in the header, where
> `Singleton` 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::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 
> T *Singleton::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?
>

I think that is what makes the most sense in this scenario (and it
simplifies things for clients of the library).
Unfortunately, for the library that was producing this warning they already
required clients to provide explicit instantiations in a .cpp file (they
had a macro like `DEFINE_SINGLETON_INSTANCE(T)` which a user would place in
a .cpp file to instantiate Singleton::instance for their type).

So fixing this warning like this would have a compatibility concern.

Thankfully, it seems like some compilers (not clang) have a bug in that
they will emit Singleton::instance any time that Singleton is
instantiated unless they have already seen an explicit instantiation
declaration of Singleton::instance in a header, so this library
already had (under an #if) explicit instantiations for
Singleton::instance for all classes that needed it in order to work
around this compiler bug. So in the end I fixed this by just enabling those
ifdefs so that clang would see the Singleton::instance explicitly
declared in the header.
(This is sort of a strange coincidence, but it worked out nicely)



>
> If there is intention to limit `Singleton` to some selected types,
> explicit instantiation declaration may help. Adding the declarations like:
>
> extern template Foo *Singleton::instance;
>
>
> prevents from warnings.
>
> Or you think that the message should propose moving static member
> definition into header file as well?
>

I'm not sure, but I think that the current diagnostics are not very
actionable. Hopefully google will lead users to your description in this
thread. I wish we had something like
https://doc.rust-lang.org/error-index.html where we could officially
provide a more in-depth explanation similar to your posts in this thread.

-- Sean Silva


>
>
> Thanks,
> --Serge
>
> 2016-05-19 9:15 GMT+06:00 Sean Silva :
>
>>
>>
>> On Thu, Apr 21, 2016 at 12:44 AM, Serge Pavlov 
>> wrote:
>>
>>> Let me demonstrate the problem using excerpt from v8 sources:
>>>
>>> -- lithium.h 
>>> template 
>>> struct LSubKindOperand {
>>>   static int* Create(int index) { return [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 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* LSubKindOperand::cache = 0;
>>>
>>> template
>>> void LSubKindOperand::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 

Re: r266719 - Warn if function or variable cannot be implicitly instantiated

2016-05-19 Thread Serge Pavlov via cfe-commits
In this case moving implementation of `Singleton::getInstance()` into a
.cpp file would prevent compiler from instantiation of the method body when
it sees `Singleton::getInstance()`. In this case
`Singleton::getInstance()` must be instantiated in some source file
either explicitly or implicitly. If inline implementation for
`Singleton::getInstance()` is provided in the header, where
`Singleton` 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::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 
T *Singleton::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::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 :

>
>
> On Thu, Apr 21, 2016 at 12:44 AM, Serge Pavlov 
> wrote:
>
>> Let me demonstrate the problem using excerpt from v8 sources:
>>
>> -- lithium.h 
>> template 
>> struct LSubKindOperand {
>>   static int* Create(int index) { return [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 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* LSubKindOperand::cache = 0;
>>
>> template
>> void LSubKindOperand::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 

Re: r266719 - Warn if function or variable cannot be implicitly instantiated

2016-05-18 Thread Sean Silva via cfe-commits
On Thu, Apr 21, 2016 at 12:44 AM, Serge Pavlov  wrote:

> Let me demonstrate the problem using excerpt from v8 sources:
>
> -- lithium.h 
> template 
> struct LSubKindOperand {
>   static int* Create(int index) { return [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 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* LSubKindOperand::cache = 0;
>
> template
> void LSubKindOperand::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, 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 I gave wasn't complete. It was more like this:

template 
struct Singleton {
... other stuff ...
  static T *getInstance() { return Singleton::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 :
>
>>
>>
>> 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
>>> 

Re: r266719 - Warn if function or variable cannot be implicitly instantiated

2016-04-20 Thread Sean Silva via cfe-commits
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 128>::cache' required here, but no definition is available
> [-Werror,-Wundefined-var-template]
> if (index < kNumCachedOperands) return [index];
> ^
> ../../v8/src/crankshaft/x64/lithium-x64.cc:334:30: note: in instantiation
> of member function
> 'v8::internal::LSubKindOperand 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 128>::cache' is explicitly instantiated in another translation unit
> if (index < kNumCachedOperands) return [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.
Richard's comment https://llvm.org/bugs/show_bug.cgi?id=24425#c2 gives more
insight into the part of the standard that this comes from.

-- Sean Silva



>
> On Tue, Apr 19, 2016 at 10:27 AM, Nico Weber  wrote:
>
>> Hi Serge,
>>
>> this complains on this snippet from v8:
>>
>> template 
>> class LSubKindOperand final : public LOperand {
>>  public:
>>   static LSubKindOperand* Create(int index, Zone* zone) {
>> if (index < kNumCachedOperands) return [index];
>> return new(zone) LSubKindOperand(index);
>>   }
>>  private:
>>   static LSubKindOperand* cache;
>>   explicit LSubKindOperand(int index);
>> };
>>
>>
>>
>> On Tue, Apr 19, 2016 at 2:19 AM, Serge Pavlov via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: sepavloff
>>> Date: Tue Apr 19 01:19:52 2016
>>> New Revision: 266719
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=266719=rev
>>> Log:
>>> Warn if function or variable cannot be implicitly instantiated
>>>
>>> With this patch compiler emits warning if it tries to make implicit
>>> instantiation
>>> of a template but cannot find the template definition. The warning can
>>> be suppressed
>>> by explicit instantiation declaration or by command line options
>>> -Wundefined-var-template and -Wundefined-func-template. The
>>> implementation follows
>>> the discussion of http://reviews.llvm.org/D12326.
>>>
>>> Differential Revision: http://reviews.llvm.org/D16396
>>>
>>> Added:
>>> cfe/trunk/test/SemaTemplate/undefined-template.cpp
>>> Modified:
>>> cfe/trunk/include/clang/AST/DeclBase.h
>>> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>> cfe/trunk/include/clang/Sema/Sema.h
>>> cfe/trunk/lib/AST/DeclBase.cpp
>>> cfe/trunk/lib/Sema/SemaOverload.cpp
>>> cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>>> cfe/trunk/test/CXX/temp/temp.decls/temp.mem/p1.cpp
>>> cfe/trunk/test/OpenMP/parallel_ast_print.cpp
>>> cfe/trunk/test/OpenMP/parallel_sections_ast_print.cpp
>>> cfe/trunk/test/OpenMP/target_parallel_ast_print.cpp
>>> cfe/trunk/test/OpenMP/task_ast_print.cpp
>>> cfe/trunk/test/OpenMP/teams_ast_print.cpp
>>> cfe/trunk/test/OpenMP/threadprivate_ast_print.cpp
>>> cfe/trunk/test/SemaCXX/PR10177.cpp
>>> cfe/trunk/test/SemaCXX/undefined-internal.cpp
>>>
>>> Modified: cfe/trunk/include/clang/AST/DeclBase.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=266719=266718=266719=diff
>>>
>>> ==
>>> --- cfe/trunk/include/clang/AST/DeclBase.h (original)
>>> +++ cfe/trunk/include/clang/AST/DeclBase.h Tue Apr 19 01:19:52 2016
>>> @@ -52,6 +52,7 @@ struct PrintingPolicy;
>>>  class RecordDecl;
>>>  class Stmt;
>>>  class StoredDeclsMap;
>>> +class TemplateDecl;
>>>  class TranslationUnitDecl;
>>>  class UsingDirectiveDecl;
>>>  }
>>> @@ -905,6 +906,10 @@ public:
>>> DeclKind == FunctionTemplate;
>>>}
>>>
>>> +  /// \brief If this is a declaration that describes some template, this
>>> +  /// method returns that template declaration.
>>> +  TemplateDecl *getDescribedTemplate() const;
>>> +
>>>/// \brief Returns the function itself, or the templated function if
>>> this is a
>>>/// function template.
>>>FunctionDecl *getAsFunction() 

Re: r266719 - Warn if function or variable cannot be implicitly instantiated

2016-04-20 Thread Sean Silva via cfe-commits
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 128>::cache' required here, but no definition is available
> [-Werror,-Wundefined-var-template]
> if (index < kNumCachedOperands) return [index];
> ^
> ../../v8/src/crankshaft/x64/lithium-x64.cc:334:30: note: in instantiation
> of member function
> 'v8::internal::LSubKindOperand 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 128>::cache' is explicitly instantiated in another translation unit
> if (index < kNumCachedOperands) return [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.
>


+1

I'm seeing a similar error in an internal codebase for a Singleton class.

Something like:

template 
struct Singleton {
... other stuff ...
  T *getInstance();
private:
  static T *instance;
};

And `instance` is defined in a .cpp file somewhere.

This seems to be deliberate.

-- Sean Silva


>
> On Tue, Apr 19, 2016 at 10:27 AM, Nico Weber  wrote:
>
>> Hi Serge,
>>
>> this complains on this snippet from v8:
>>
>> template 
>> class LSubKindOperand final : public LOperand {
>>  public:
>>   static LSubKindOperand* Create(int index, Zone* zone) {
>> if (index < kNumCachedOperands) return [index];
>> return new(zone) LSubKindOperand(index);
>>   }
>>  private:
>>   static LSubKindOperand* cache;
>>   explicit LSubKindOperand(int index);
>> };
>>
>>
>>
>> On Tue, Apr 19, 2016 at 2:19 AM, Serge Pavlov via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: sepavloff
>>> Date: Tue Apr 19 01:19:52 2016
>>> New Revision: 266719
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=266719=rev
>>> Log:
>>> Warn if function or variable cannot be implicitly instantiated
>>>
>>> With this patch compiler emits warning if it tries to make implicit
>>> instantiation
>>> of a template but cannot find the template definition. The warning can
>>> be suppressed
>>> by explicit instantiation declaration or by command line options
>>> -Wundefined-var-template and -Wundefined-func-template. The
>>> implementation follows
>>> the discussion of http://reviews.llvm.org/D12326.
>>>
>>> Differential Revision: http://reviews.llvm.org/D16396
>>>
>>> Added:
>>> cfe/trunk/test/SemaTemplate/undefined-template.cpp
>>> Modified:
>>> cfe/trunk/include/clang/AST/DeclBase.h
>>> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>> cfe/trunk/include/clang/Sema/Sema.h
>>> cfe/trunk/lib/AST/DeclBase.cpp
>>> cfe/trunk/lib/Sema/SemaOverload.cpp
>>> cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>>> cfe/trunk/test/CXX/temp/temp.decls/temp.mem/p1.cpp
>>> cfe/trunk/test/OpenMP/parallel_ast_print.cpp
>>> cfe/trunk/test/OpenMP/parallel_sections_ast_print.cpp
>>> cfe/trunk/test/OpenMP/target_parallel_ast_print.cpp
>>> cfe/trunk/test/OpenMP/task_ast_print.cpp
>>> cfe/trunk/test/OpenMP/teams_ast_print.cpp
>>> cfe/trunk/test/OpenMP/threadprivate_ast_print.cpp
>>> cfe/trunk/test/SemaCXX/PR10177.cpp
>>> cfe/trunk/test/SemaCXX/undefined-internal.cpp
>>>
>>> Modified: cfe/trunk/include/clang/AST/DeclBase.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=266719=266718=266719=diff
>>>
>>> ==
>>> --- cfe/trunk/include/clang/AST/DeclBase.h (original)
>>> +++ cfe/trunk/include/clang/AST/DeclBase.h Tue Apr 19 01:19:52 2016
>>> @@ -52,6 +52,7 @@ struct PrintingPolicy;
>>>  class RecordDecl;
>>>  class Stmt;
>>>  class StoredDeclsMap;
>>> +class TemplateDecl;
>>>  class TranslationUnitDecl;
>>>  class UsingDirectiveDecl;
>>>  }
>>> @@ -905,6 +906,10 @@ public:
>>> DeclKind == FunctionTemplate;
>>>}
>>>
>>> +  /// \brief If this is a declaration that describes some template, this
>>> +  /// method returns that template declaration.
>>> +  TemplateDecl *getDescribedTemplate() const;
>>> +
>>>/// \brief Returns the function itself, or the templated function if
>>> this 

Re: r266719 - Warn if function or variable cannot be implicitly instantiated

2016-04-19 Thread Serge Pavlov via cfe-commits
Hi Nico,

This message indicates that compiler sees a reference to static member
'cache' of some specialization of template 'LSubKindOperand' (namely
'LSubKindOperand').
This is a static member, so compiler needs corresponding template
definition to instantiate it, but there is no such in the file being
compiled. So compiler cannot implicitly generate 'LSubKindOperand<...>::cache'.
It is not an error if the definition is defined somewhere in the project,
in other translation units. In the case of v8 it is defined
in src\crankshaft\lithium.cc.

The warning can help in detection of some cases, difficult to diagnose for
user, the problem described in PR24425 is one of such. If the project v8
didn't have definition in lithium.cc linker error would be seen.

I would recommend moving definition of static member template:

template
LSubKindOperand*
LSubKindOperand::cache = NULL;

from lithium.cc to lithium.h.

Thanks,
--Serge

2016-04-19 20:28 GMT+06:00 Nico Weber :

> (sorry, accidentally sent this mid-mail)
>
> ../../v8/src/crankshaft/lithium.h:322:45: error: instantiation of variable
> 'v8::internal::LSubKindOperand 128>::cache' required here, but no definition is available
> [-Werror,-Wundefined-var-template]
> if (index < kNumCachedOperands) return [index];
> ^
> ../../v8/src/crankshaft/x64/lithium-x64.cc:334:30: note: in instantiation
> of member function
> 'v8::internal::LSubKindOperand 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 128>::cache' is explicitly instantiated in another translation unit
> if (index < kNumCachedOperands) return [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.
>
> On Tue, Apr 19, 2016 at 10:27 AM, Nico Weber  wrote:
>
>> Hi Serge,
>>
>> this complains on this snippet from v8:
>>
>> template 
>> class LSubKindOperand final : public LOperand {
>>  public:
>>   static LSubKindOperand* Create(int index, Zone* zone) {
>> if (index < kNumCachedOperands) return [index];
>> return new(zone) LSubKindOperand(index);
>>   }
>>  private:
>>   static LSubKindOperand* cache;
>>   explicit LSubKindOperand(int index);
>> };
>>
>>
>>
>> On Tue, Apr 19, 2016 at 2:19 AM, Serge Pavlov via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: sepavloff
>>> Date: Tue Apr 19 01:19:52 2016
>>> New Revision: 266719
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=266719=rev
>>> Log:
>>> Warn if function or variable cannot be implicitly instantiated
>>>
>>> With this patch compiler emits warning if it tries to make implicit
>>> instantiation
>>> of a template but cannot find the template definition. The warning can
>>> be suppressed
>>> by explicit instantiation declaration or by command line options
>>> -Wundefined-var-template and -Wundefined-func-template. The
>>> implementation follows
>>> the discussion of http://reviews.llvm.org/D12326.
>>>
>>> Differential Revision: http://reviews.llvm.org/D16396
>>>
>>> Added:
>>> cfe/trunk/test/SemaTemplate/undefined-template.cpp
>>> Modified:
>>> cfe/trunk/include/clang/AST/DeclBase.h
>>> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>> cfe/trunk/include/clang/Sema/Sema.h
>>> cfe/trunk/lib/AST/DeclBase.cpp
>>> cfe/trunk/lib/Sema/SemaOverload.cpp
>>> cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>>> cfe/trunk/test/CXX/temp/temp.decls/temp.mem/p1.cpp
>>> cfe/trunk/test/OpenMP/parallel_ast_print.cpp
>>> cfe/trunk/test/OpenMP/parallel_sections_ast_print.cpp
>>> cfe/trunk/test/OpenMP/target_parallel_ast_print.cpp
>>> cfe/trunk/test/OpenMP/task_ast_print.cpp
>>> cfe/trunk/test/OpenMP/teams_ast_print.cpp
>>> cfe/trunk/test/OpenMP/threadprivate_ast_print.cpp
>>> cfe/trunk/test/SemaCXX/PR10177.cpp
>>> cfe/trunk/test/SemaCXX/undefined-internal.cpp
>>>
>>> Modified: cfe/trunk/include/clang/AST/DeclBase.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=266719=266718=266719=diff
>>>
>>> 

Re: r266719 - Warn if function or variable cannot be implicitly instantiated

2016-04-19 Thread Nico Weber via cfe-commits
(sorry, accidentally sent this mid-mail)

../../v8/src/crankshaft/lithium.h:322:45: error: instantiation of variable
'v8::internal::LSubKindOperand::cache' required here, but no definition is available
[-Werror,-Wundefined-var-template]
if (index < kNumCachedOperands) return [index];
^
../../v8/src/crankshaft/x64/lithium-x64.cc:334:30: note: in instantiation
of member function
'v8::internal::LSubKindOperand::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::cache' is explicitly instantiated in another translation unit
if (index < kNumCachedOperands) return [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.

On Tue, Apr 19, 2016 at 10:27 AM, Nico Weber  wrote:

> Hi Serge,
>
> this complains on this snippet from v8:
>
> template 
> class LSubKindOperand final : public LOperand {
>  public:
>   static LSubKindOperand* Create(int index, Zone* zone) {
> if (index < kNumCachedOperands) return [index];
> return new(zone) LSubKindOperand(index);
>   }
>  private:
>   static LSubKindOperand* cache;
>   explicit LSubKindOperand(int index);
> };
>
>
>
> On Tue, Apr 19, 2016 at 2:19 AM, Serge Pavlov via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: sepavloff
>> Date: Tue Apr 19 01:19:52 2016
>> New Revision: 266719
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=266719=rev
>> Log:
>> Warn if function or variable cannot be implicitly instantiated
>>
>> With this patch compiler emits warning if it tries to make implicit
>> instantiation
>> of a template but cannot find the template definition. The warning can be
>> suppressed
>> by explicit instantiation declaration or by command line options
>> -Wundefined-var-template and -Wundefined-func-template. The
>> implementation follows
>> the discussion of http://reviews.llvm.org/D12326.
>>
>> Differential Revision: http://reviews.llvm.org/D16396
>>
>> Added:
>> cfe/trunk/test/SemaTemplate/undefined-template.cpp
>> Modified:
>> cfe/trunk/include/clang/AST/DeclBase.h
>> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> cfe/trunk/include/clang/Sema/Sema.h
>> cfe/trunk/lib/AST/DeclBase.cpp
>> cfe/trunk/lib/Sema/SemaOverload.cpp
>> cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>> cfe/trunk/test/CXX/temp/temp.decls/temp.mem/p1.cpp
>> cfe/trunk/test/OpenMP/parallel_ast_print.cpp
>> cfe/trunk/test/OpenMP/parallel_sections_ast_print.cpp
>> cfe/trunk/test/OpenMP/target_parallel_ast_print.cpp
>> cfe/trunk/test/OpenMP/task_ast_print.cpp
>> cfe/trunk/test/OpenMP/teams_ast_print.cpp
>> cfe/trunk/test/OpenMP/threadprivate_ast_print.cpp
>> cfe/trunk/test/SemaCXX/PR10177.cpp
>> cfe/trunk/test/SemaCXX/undefined-internal.cpp
>>
>> Modified: cfe/trunk/include/clang/AST/DeclBase.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=266719=266718=266719=diff
>>
>> ==
>> --- cfe/trunk/include/clang/AST/DeclBase.h (original)
>> +++ cfe/trunk/include/clang/AST/DeclBase.h Tue Apr 19 01:19:52 2016
>> @@ -52,6 +52,7 @@ struct PrintingPolicy;
>>  class RecordDecl;
>>  class Stmt;
>>  class StoredDeclsMap;
>> +class TemplateDecl;
>>  class TranslationUnitDecl;
>>  class UsingDirectiveDecl;
>>  }
>> @@ -905,6 +906,10 @@ public:
>> DeclKind == FunctionTemplate;
>>}
>>
>> +  /// \brief If this is a declaration that describes some template, this
>> +  /// method returns that template declaration.
>> +  TemplateDecl *getDescribedTemplate() const;
>> +
>>/// \brief Returns the function itself, or the templated function if
>> this is a
>>/// function template.
>>FunctionDecl *getAsFunction() LLVM_READONLY;
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=266719=266718=266719=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Apr 19 01:19:52
>> 2016
>> @@ -75,6 +75,8 @@ def : 

Re: r266719 - Warn if function or variable cannot be implicitly instantiated

2016-04-19 Thread Nico Weber via cfe-commits
Hi Serge,

this complains on this snippet from v8:

template 
class LSubKindOperand final : public LOperand {
 public:
  static LSubKindOperand* Create(int index, Zone* zone) {
if (index < kNumCachedOperands) return [index];
return new(zone) LSubKindOperand(index);
  }
 private:
  static LSubKindOperand* cache;
  explicit LSubKindOperand(int index);
};



On Tue, Apr 19, 2016 at 2:19 AM, Serge Pavlov via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: sepavloff
> Date: Tue Apr 19 01:19:52 2016
> New Revision: 266719
>
> URL: http://llvm.org/viewvc/llvm-project?rev=266719=rev
> Log:
> Warn if function or variable cannot be implicitly instantiated
>
> With this patch compiler emits warning if it tries to make implicit
> instantiation
> of a template but cannot find the template definition. The warning can be
> suppressed
> by explicit instantiation declaration or by command line options
> -Wundefined-var-template and -Wundefined-func-template. The implementation
> follows
> the discussion of http://reviews.llvm.org/D12326.
>
> Differential Revision: http://reviews.llvm.org/D16396
>
> Added:
> cfe/trunk/test/SemaTemplate/undefined-template.cpp
> Modified:
> cfe/trunk/include/clang/AST/DeclBase.h
> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/include/clang/Sema/Sema.h
> cfe/trunk/lib/AST/DeclBase.cpp
> cfe/trunk/lib/Sema/SemaOverload.cpp
> cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> cfe/trunk/test/CXX/temp/temp.decls/temp.mem/p1.cpp
> cfe/trunk/test/OpenMP/parallel_ast_print.cpp
> cfe/trunk/test/OpenMP/parallel_sections_ast_print.cpp
> cfe/trunk/test/OpenMP/target_parallel_ast_print.cpp
> cfe/trunk/test/OpenMP/task_ast_print.cpp
> cfe/trunk/test/OpenMP/teams_ast_print.cpp
> cfe/trunk/test/OpenMP/threadprivate_ast_print.cpp
> cfe/trunk/test/SemaCXX/PR10177.cpp
> cfe/trunk/test/SemaCXX/undefined-internal.cpp
>
> Modified: cfe/trunk/include/clang/AST/DeclBase.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=266719=266718=266719=diff
>
> ==
> --- cfe/trunk/include/clang/AST/DeclBase.h (original)
> +++ cfe/trunk/include/clang/AST/DeclBase.h Tue Apr 19 01:19:52 2016
> @@ -52,6 +52,7 @@ struct PrintingPolicy;
>  class RecordDecl;
>  class Stmt;
>  class StoredDeclsMap;
> +class TemplateDecl;
>  class TranslationUnitDecl;
>  class UsingDirectiveDecl;
>  }
> @@ -905,6 +906,10 @@ public:
> DeclKind == FunctionTemplate;
>}
>
> +  /// \brief If this is a declaration that describes some template, this
> +  /// method returns that template declaration.
> +  TemplateDecl *getDescribedTemplate() const;
> +
>/// \brief Returns the function itself, or the templated function if
> this is a
>/// function template.
>FunctionDecl *getAsFunction() LLVM_READONLY;
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=266719=266718=266719=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Apr 19 01:19:52
> 2016
> @@ -75,6 +75,8 @@ def : DiagGroup<"ctor-dtor-privacy">;
>  def GNUDesignator : DiagGroup<"gnu-designator">;
>  def GNUStringLiteralOperatorTemplate :
>DiagGroup<"gnu-string-literal-operator-template">;
> +def UndefinedVarTemplate : DiagGroup<"undefined-var-template">;
> +def UndefinedFuncTemplate : DiagGroup<"undefined-func-template">;
>
>  def DeleteIncomplete : DiagGroup<"delete-incomplete">;
>  def DeleteNonVirtualDtor : DiagGroup<"delete-non-virtual-dtor">;
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=266719=266718=266719=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Apr 19
> 01:19:52 2016
> @@ -3883,7 +3883,18 @@ def note_template_type_alias_instantiati
>"in instantiation of template type alias %0 requested here">;
>  def note_template_exception_spec_instantiation_here : Note<
>"in instantiation of exception specification for %0 requested here">;
> -
> +def warn_var_template_missing : Warning<"instantiation of variable %q0 "
> +  "required here, but no definition is available">,
> +  InGroup;
> +def warn_func_template_missing : Warning<"instantiation of function %q0 "
> +  "required here, but no definition is available">,
> +  InGroup, DefaultIgnore;
> +def note_forward_template_decl : Note<
> +  "forward declaration of template entity is 

r266719 - Warn if function or variable cannot be implicitly instantiated

2016-04-19 Thread Serge Pavlov via cfe-commits
Author: sepavloff
Date: Tue Apr 19 01:19:52 2016
New Revision: 266719

URL: http://llvm.org/viewvc/llvm-project?rev=266719=rev
Log:
Warn if function or variable cannot be implicitly instantiated

With this patch compiler emits warning if it tries to make implicit 
instantiation
of a template but cannot find the template definition. The warning can be 
suppressed
by explicit instantiation declaration or by command line options
-Wundefined-var-template and -Wundefined-func-template. The implementation 
follows
the discussion of http://reviews.llvm.org/D12326.

Differential Revision: http://reviews.llvm.org/D16396

Added:
cfe/trunk/test/SemaTemplate/undefined-template.cpp
Modified:
cfe/trunk/include/clang/AST/DeclBase.h
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/AST/DeclBase.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
cfe/trunk/test/CXX/temp/temp.decls/temp.mem/p1.cpp
cfe/trunk/test/OpenMP/parallel_ast_print.cpp
cfe/trunk/test/OpenMP/parallel_sections_ast_print.cpp
cfe/trunk/test/OpenMP/target_parallel_ast_print.cpp
cfe/trunk/test/OpenMP/task_ast_print.cpp
cfe/trunk/test/OpenMP/teams_ast_print.cpp
cfe/trunk/test/OpenMP/threadprivate_ast_print.cpp
cfe/trunk/test/SemaCXX/PR10177.cpp
cfe/trunk/test/SemaCXX/undefined-internal.cpp

Modified: cfe/trunk/include/clang/AST/DeclBase.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=266719=266718=266719=diff
==
--- cfe/trunk/include/clang/AST/DeclBase.h (original)
+++ cfe/trunk/include/clang/AST/DeclBase.h Tue Apr 19 01:19:52 2016
@@ -52,6 +52,7 @@ struct PrintingPolicy;
 class RecordDecl;
 class Stmt;
 class StoredDeclsMap;
+class TemplateDecl;
 class TranslationUnitDecl;
 class UsingDirectiveDecl;
 }
@@ -905,6 +906,10 @@ public:
DeclKind == FunctionTemplate;
   }
 
+  /// \brief If this is a declaration that describes some template, this
+  /// method returns that template declaration.
+  TemplateDecl *getDescribedTemplate() const;
+
   /// \brief Returns the function itself, or the templated function if this is 
a
   /// function template.
   FunctionDecl *getAsFunction() LLVM_READONLY;

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=266719=266718=266719=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Apr 19 01:19:52 2016
@@ -75,6 +75,8 @@ def : DiagGroup<"ctor-dtor-privacy">;
 def GNUDesignator : DiagGroup<"gnu-designator">;
 def GNUStringLiteralOperatorTemplate :
   DiagGroup<"gnu-string-literal-operator-template">;
+def UndefinedVarTemplate : DiagGroup<"undefined-var-template">;
+def UndefinedFuncTemplate : DiagGroup<"undefined-func-template">;
 
 def DeleteIncomplete : DiagGroup<"delete-incomplete">;
 def DeleteNonVirtualDtor : DiagGroup<"delete-non-virtual-dtor">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=266719=266718=266719=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Apr 19 01:19:52 
2016
@@ -3883,7 +3883,18 @@ def note_template_type_alias_instantiati
   "in instantiation of template type alias %0 requested here">;
 def note_template_exception_spec_instantiation_here : Note<
   "in instantiation of exception specification for %0 requested here">;
-  
+def warn_var_template_missing : Warning<"instantiation of variable %q0 "
+  "required here, but no definition is available">,
+  InGroup;
+def warn_func_template_missing : Warning<"instantiation of function %q0 "
+  "required here, but no definition is available">,
+  InGroup, DefaultIgnore;
+def note_forward_template_decl : Note<
+  "forward declaration of template entity is here">;
+def note_inst_declaration_hint : Note<"add an explicit instantiation "
+  "declaration to suppress this warning if %q0 is explicitly instantiated in "
+  "another translation unit">;
+
 def note_default_arg_instantiation_here : Note<
   "in instantiation of default argument for '%0' required here">;
 def note_default_function_arg_instantiation_here : Note<

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=266719=266718=266719=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++