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

[Peter Collingbourne]

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?

Thanks,
-- 
Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Refactor-redeclarable-template-declarations.patch
Type: text/x-diff
Size: 35712 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20100729/f9d71342/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Store-latest-redeclaration-for-each-redeclarable-tem.patch
Type: text/x-diff
Size: 4089 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20100729/f9d71342/attachment-0001.patch>