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

[Argyrios Kyrtzidis]

On Jul 29, 2010, at 5:15 AM, Peter Collingbourne wrote:

> On Tue, Jul 27, 2010 at 10:29:14PM +0300, Argyrios Kyrtzidis wrote:
>> On Jul 27, 2010, at 10:17 PM, Peter Collingbourne wrote:
>>> I also have been working on a patch that refactors common code in the
>>> FunctionTemplateDecl and ClassTemplateDecl classes into a common base
>>> class (and provides an implementation of getLatestRedeclaration()
>>> for those two classes); I intend to build this work on top of the
>>> refactoring (I'll send out the patch soon).
>> 
>> Seems like a great plan!
> 
> I managed to get the refactoring patches into good enough shape to send out;
> they are attached.  OK to commit?

Very nice refactoring!

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 ?

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.

> I'll start working on the iterator patch in the next day or so.

Great, thanks for working on this!

-Argiris