On Jun 16, 2011, at 6:26 PM, Francois Pichet wrote:

> I struggled getting this patch right and I am not confident enough to
> submit it directly without review so here it is:
> Template function specialization at class scope for -fms-extensions
> 
> Example:
> template <class T>
> class B {
> public:
>   template <class U>
>   void f(U p) {   }
> 
>   template <>
>    void f(int p) {    }  // <= not standard C++, Microsoft extension
> };
> 
> As Doug suggested to me on the IRC chat, I created a new Decl node:
> ClassScopeFunctionSpecializationDecl
> This node hold a temporary CXXMethodDecl and it will use it to create
> an explicit specialization during class instantiation.

I (still) like this design. It defers the template argument deduction until 
template instantiation time, at which point we can actually, properly, 
determine which function template we're specializing. We can't tell that when 
we parse the function specialization.

> This patch is necessary to parse the MSVC 2010, MFC and ATL code.

> PS: I'll add serialization in a follow-up patch.

Okay!

Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td  (revision 133213)
+++ include/clang/Basic/DiagnosticSemaKinds.td  (working copy)
@@ -1867,6 +1867,9 @@

   "explicit specialization of %0 in function scope">;
 def err_template_spec_decl_class_scope : Error<
   "explicit specialization of %0 in class scope">;
+def warn_template_spec_decl_class_scope : ExtWarn<
+  "explicit specialization of %0 in class scope in a Microsoft extension">,
+  InGroup<Microsoft>;

This should say "is a Microsoft extension".

Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp       (revision 133213)
+++ lib/Sema/SemaDecl.cpp       (working copy)
@@ -1688,9 +1688,13 @@

       // Preserve triviality.
       NewMethod->setTrivial(OldMethod->isTrivial());
 
+      // MSVC allows explicit template specialization at class scope.
+      bool IsMSExplicitSpecialization = getLangOptions().Microsoft &&
+                                 NewMethod->isFunctionTemplateSpecialization();
       bool isFriend = NewMethod->getFriendObjectKind();
 
-      if (!isFriend && NewMethod->getLexicalDeclContext()->isRecord()) {
+      if (!isFriend && NewMethod->getLexicalDeclContext()->isRecord() &&
+          !IsMSExplicitSpecialization) {
         //    -- Member function declarations with the same name and the
         //       same parameter types cannot be overloaded if any of them
         //       is a static member function declaration.
@@ -4649,6 +4653,17 @@

     } else if (isFunctionTemplateSpecialization) {
       if (CurContext->isDependentContext() && CurContext->isRecord() 
           && !isFriend) {
+        if (getLangOptions().Microsoft) {
+          ClassScopeFunctionSpecializationDecl *NewSpec =
+                               ClassScopeFunctionSpecializationDecl::Create(
+                                      Context, CurContext,  SourceLocation(), 
+                                      cast<CXXMethodDecl>(NewFD));
+          CurContext->addDecl(NewSpec);
+          Redeclaration = true;
+          NewFD->setInvalidDecl();
+          return NewFD;
+        }
+
         Diag(NewFD->getLocation(), diag::err_function_specialization_in_class)
           << NewFD->getDeclName();
         NewFD->setInvalidDecl();

We shouldn't perform an early-exit here, because it means that we're missing 
out on a lot of semantic analysis, such as adding attributes and checking the 
qualification. Can we just make a note that this is a class-scope function 
specialization, do all of the extra checking, and then build the 
ClassScopeFunctionSpecializationDecl at the end? 

Also, I think it'd be cleaner if we always used 
ClassScopeFunctionSpecializationDecl to handle class-scope function 
specializations, regardless of whether we're in Microsoft mode. It would be 
great if the only check for getLangOptions().Microsoft was to decide whether to 
emit the ExtWarn vs. the Error. Plus, we'll get better testing coverage that 
way :)

Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===================================================================
--- lib/Sema/SemaTemplateInstantiateDecl.cpp    (revision 133213)
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp    (working copy)
@@ -1890,6 +1890,89 @@

   return UD;
 }
 
+Decl *TemplateDeclInstantiator::VisitClassScopeFunctionSpecializationDecl(
+                                     ClassScopeFunctionSpecializationDecl 
*Decl) {

There's a lot of redundancy between this method and 
TemplateDeclInstantiator::VisitCXXMethodDecl(). Did you try passing additional 
state to VisitCXXMethodDecl, so that we can re-use (and tweak) the code there 
rather than copying most of the code?

This part in particular confuses me:

+
+  // Instantiate NewFD.
+  SemaRef.InstantiateFunctionDefinition(SourceLocation(), NewFD, false,
+                                        false, false);

Why are we instantiating the function definition? We shouldn't need to do that.

One thing to remember is that member function templates can already be 
specialized out-of-line, and Clang supports that properly. So it seems that it 
should only take a relatively small amount of code in the instantiation of one 
of these class-scope specializations to push Sema down the path to calling this 
a function specialization.

Thanks for working on this!

        - Doug

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to