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