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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
