[PATCH] Adding a diagnostic for member templates inside a local class

[Douglas Gregor]

On Mar 14, 2013, at 6:52 AM, "Gao, Yunzhong" <Yunzhong.Gao at am.sony.com> wrote:

> 
> ________________________________________
> From: Gao, Yunzhong
> Sent: Wednesday, March 13, 2013 11:51 PM
> To: Douglas Gregor
> Cc: cfe-commits at cs.uiuc.edu
> Subject: RE: [PATCH] Adding a diagnostic for member templates inside a local class
> 
> Hi Douglas,
> Thank you for review.

Sorry for the delay; this got buried.

> I am attaching an updated patch with the following changes:
> 
>  1. The diagnostic is moved from lib/Parse to lib/SemaTemplate;
>  2. Added a new test to test/SemaTemplate instead of test/Parser;

How about putting the new test into test/CXX/<appropriate clause and paragraph.cpp>, so it matches where this restriction shows up in the C++ standard?

>  3. In test/SemaTemplate/instantiate-exception-spec-cxx11.cpp, moved the class definition outside of the
>      function body to avoid triggering the new diagnostic.
>      I did experiment with adding an expected-error annotation there, but it seems that delaying the diagnostic
>      from Parser to Sema triggers quite a few more error messages, e.g.,
>          Line 52: use of undeclared identifier 'f'
>          Line 54: no member named 'f' in 'S'
>      I think adding more expected-error annotations would unnecessarily complicate the existing test. The
>      proposed new test checks both function and class templates.

I think that test was intentionally making sure we don't blow up when this happens inside a local class. I'd be better to duplicate that part of the test.

> Could you review?

Index: lib/Sema/SemaTemplate.cpp
===================================================================
--- lib/Sema/SemaTemplate.cpp	(revision 176911)
+++ lib/Sema/SemaTemplate.cpp	(working copy)
@@ -4908,6 +4908,15 @@
   while (Ctx && isa<LinkageSpecDecl>(Ctx))
     Ctx = Ctx->getParent();
 
+  if (Ctx && Ctx->isRecord()) {
+    const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(Ctx);
+    if (RD && RD->isLocalClass()) {

It'd be nice to have a standard reference here.

In this C++-only code, Ctx->isRecord() implies that dyn_cast<CXXRecordDecl>(Ctx) will always return a non-null CXXRecordDecl, so you can use cast<> instead of dyn_cast<> and eliminate the "RD &&" check.

	- Doug



> - Gao.
> 
> ________________________________________
> From: Douglas Gregor [dgregor at apple.com]
> Sent: Wednesday, March 13, 2013 4:13 PM
> To: Gao, Yunzhong
> Cc: cfe-commits at cs.uiuc.edu
> Subject: Re: [PATCH] Adding a diagnostic for member templates inside a local class
> 
> On Mar 13, 2013, at 12:55 PM, "Gao, Yunzhong" <Yunzhong.Gao at am.sony.com> wrote:
> 
>> Hi,
>> Currently, clang++ accepts the following codes without any error message.
>> 
>> int test(void)
>> {
>> class A {
>>   template<class T> class B
>>   {  T t; };
>> };
>> 
>> return 0;
>> }
>> 
>> However, I believe that an error message is required according to the C++03 and C++11.
>> In particular, section 14.5 clause 2 (of both specs) says "A local class shall not have member templates."
>> 
>> The following patch adds a diagnostic for member templates declared inside a local class.
> 
> Generally, this looks good. However, I'd like to see it diagnosed in Sema, rather than in the parser, which we do for static data members and friends in local classes.
> 
>        - Doug
> 
> 
> 
> <member_template.patch>