[cfe-commits] [PATCH][MS][Review request] - Microsoft function specialization at scope scope

[Douglas Gregor]

On Aug 1, 2011, at 10:06 AM, Francois Pichet wrote:

> ok thanks for the review. I am slowly working on the rework (but I am
> slow since this is summer time) 2 things I want to comment:
> 
> 
>> +  // Here we have an function template explicit specialization at class scope.
>> +  // The actually specialization will be postponed to template instatiation
>> +  // time via the ClassScopeFunctionSpecializationDecl node.
>> +  if (isDependantClassScopeExplicitSpecialization) {
>> +    ClassScopeFunctionSpecializationDecl *NewSpec =
>> +                         ClassScopeFunctionSpecializationDecl::Create(
>> +                                Context, CurContext,  SourceLocation(),
>> +                                cast<CXXMethodDecl>(NewFD));
>> +    CurContext->addDecl(NewSpec);
>> +    // FIXME: This is hackish: set NewFD to invalid and Redeclaration to true,
>> +    // to prevent NewFD from been pushed into scope.
>> +    NewFD->setInvalidDecl();
>> +    Redeclaration = true;
>> +  }
>> 
>> Why not return return NewSpec, and have the caller recognize that it shouldn't try to make ClassScopeFunctionSpecializationDecls visible? (Since they aren't NamedDecls anyway).
>> 
> 
> How can we return NewSpec here? This won't work, We need to return a
> FunctionDecl otherwise the function body will be correctly parsed.

Well, we *could* fix up the clients of this function to cope with ClassScopeFunctionSpecializationDecls… but it doesn't seem worth it. Could you at least introduce another flag that says "Please don't add this to the current scope", rather than setting NewFD invalid and Redeclaration=true? The current approach is brittle.

>> @@ -1514,7 +1516,7 @@
>> 
>>                             : Method);
>>     if (isFriend)
>>       Record->makeDeclVisibleInContext(DeclToAdd);
>> -    else
>> +    else if (!IsClassScopeSpecialization)
>>       Owner->addDecl(DeclToAdd);
>>   }
>> 
>> It seems like it should be okay to addDecl the class scope specializations. Did that break something?
> 
> yes it will fail to compile this:
> template <class T>
> class A {
> public:
>    template <class U>  void f(U p) { }
>    template <>  void f(int p) { }
>    void f(int p) {  } // <==error here if we addDecl.
> };

Ahhh, I understand now. Thanks!

	- Doug