ping? On Sat, Jul 9, 2011 at 8:19 PM, Francois Pichet <[email protected]> wrote: > Thanks for the review, here is patch #2 with some changes: > > On Fri, Jun 24, 2011 at 11:18 AM, Douglas Gregor <[email protected]> wrote: >> >> On Jun 16, 2011, at 6:26 PM, Francois Pichet wrote: >> >> >> 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". > > Done > >> >> 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 :) >> > > Ok done too. > >> 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. >> > > Ok I removed the duplication and added a flag to VisitCXXMethodDecl. > This patch is different in that the instantiation is now done properly > at the end of the translation unit like any others. > This add some small complexity (some > getClassScopeSpecializationPattern checking here and there) to not > bail out because we are dealing with a TSK_ExplicitSpecialization at > instantiation time. > > Also one potentially controversial aspect of this patch is that it > adds a new pointer (ClassScopeSpecializationPattern) to the > FunctionTemplateSpecializationInfo class. This new pointer is used to > retrieved the function pattern during the instantiation of a class > scope explicit specialization. So there is a 4-byte added for a rarely > used feature. Not sure if that's acceptable. > > > Thanks again, >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
