[cfe-commits] [PATCH] Refactor redeclarable template declarations (was Re: [Request for approval] Allow clang library applications to get specializations)

[Argyrios Kyrtzidis]

Hi Peter,

On Jul 29, 2010, at 6:58 PM, Peter Collingbourne wrote:

> Hi Argiris,
> 
> On Thu, Jul 29, 2010 at 12:27:49PM +0300, Argyrios Kyrtzidis wrote:
>> I'd like to address just one thing:
>> 
>> +void RedeclarableTemplateDecl::setPreviousDeclarationImpl(
>> +                                               RedeclarableTemplateDecl *Prev) {
>> +  if (Prev) {
>> +    CommonBase *Common = Prev->getCommonPtr();
>> +    Prev = Common->Latest;
>> +    Common->Latest = this;
>> +    CommonOrPrev = Prev;
>> +  }
>> +}
>> 
>> What are the semantics if Prev is null while a previous decl is already set ? Should this not be allowed by an assert ?
> 
> I was following the semantics of the original
> {Class,Function}TemplateDecl::setPreviousDeclaration functions which
> do nothing if Prev is null.  But I agree it would be sensible to add
> an assert there, and it does not seem to break anything to do so.
> 
>> Also, I'm concerned that PCHDeclReader::VisitRedeclarableTemplateDecl() calls this version of setPreviousDeclarationImpl().
>> In general, during PCH reading, we cannot depend that all other decls are fully initialized; to be more specific, Prev may still be initializing so we should not 'let' it call getCommonPtr().
>> 
>> I'd suggest VisitRedeclarableTemplateDecl() be modified to set CommonOrPrev directly and store/read CommonBase's Latest decl.
> 
> OK, makes sense, done.
> 
> Here are the new versions of patches 1 and 2 (patch 3 is unchanged).
> OK to commit?

Looks great, please commit!

-Argiris