<div class="gmail_quote"><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div bgcolor="#FFFFFF"><div class="im"><blockquote type="cite"><div>

<div class="gmail_quote"><div>I've spent the morning addressing issues in the patch and had some questions, comments. I'm not sending up a new patch just yet, though.<br></div></div></div></blockquote></div></div>

</blockquote><div>I had planned to send this up on Saturday, but couldn't connect to the server in the morning so I worked on other stuff instead. Attached is a new version of C++/PCH Decl support. I think I've addressed all of your comments. I've also added support for<br>

<br>- unresolved value/type decls<br>- friend type references<br>- ctor member initializers<br>- stubbed a function for friend template decls<br><br>There are 3 issues with this patch that I either don't like or are cause problems.<br>

<br>1. I needed a way to attach the DefinitionData of a CXXRecordDecl that is not a definition to a previously defined record declaration via the PCH reader (hence the attachDefinition()) function in CXXRecordDecl. It basically addresses this problem:<br>

<br>class C { };<br>class C; // DefinitionData not attached during reading<br></div></div><br>The DefinitionData isn't attached but the previous declaration is set correctly. IIRC, the same situation happens with injected class names.<br>

<br>I'm not thrilled with this solution. I think the best solution would be to make setPreviousDeclaration automatically share (forward?) DefinitionData. I just didn't want to open that can of worms in this patch.<br>

<br>2. Anonymous namespaces within other namespaces appear to be incorrectly linked in semantic analysis but not PCH serialization. Of course, my interpretation of C++ semantics may also be wrong :) Let's say we have:<br>

<br>// Yes, there are two of  them<br>namesapce N { namespace { class C; } } // 1<br>namesapce N { namespace { class C; } } // 2<br>
N::C* p; // ERROR ambiguous lookup for C.<br><br>My understanding is that the anonymous namespace should be linked so that 1 and 2 refer to the same class. Is that right? I added a test to SemaCXX/namespaces.cpp to check.<br>

<br>Strangely, the PCH serialization seems to deal with this correctly. Emphasis on "seems to". It doesn't generate an error, and it doesn't crash... I can only guess that this is somehow an issue with lookup or a bizarre artifact of lazy deserialization.<br>

<br>3. There seems to be a bug in the PCH reader when you explicitly initialize member variables of a base class. Let's say you have:<br><br>struct B : A {<br>  B() : A(), x() { } // <- ASSERT<br>  int x;<br>};<br>

<br>The assertion occurs in the deserialization (ReadStmt) of the ImplicitValueInit expr for x(). Somehow, after visiting that stmt/expr, the Idx == Record.size() - 1. Curiously, if B has no base classes, this seems to work fine.<br>

<br>I'm not sure what the status of this patch is going to be since a) clang/llvm seems to be in a code freeze and b) this patch introduces an assertion. I have a lot of other things to take care of so I may not be able to address those issues in a timely manner.<br>

<br>Andrew Sutton<br><a href="mailto:andrew.n.sutton@gmail.com">andrew.n.sutton@gmail.com</a><br>