[cfe-dev] [PATCH] PCH Support for C++ decls

Andrew Sutton andrew.n.sutton at gmail.com
Mon Mar 8 07:44:46 PST 2010


>
> 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.
>
> 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

- unresolved value/type decls
- friend type references
- ctor member initializers
- stubbed a function for friend template decls

There are 3 issues with this patch that I either don't like or are cause
problems.

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:

class C { };
class C; // DefinitionData not attached during reading

The DefinitionData isn't attached but the previous declaration is set
correctly. IIRC, the same situation happens with injected class names.

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.

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:

// Yes, there are two of  them
namesapce N { namespace { class C; } } // 1
namesapce N { namespace { class C; } } // 2
N::C* p; // ERROR ambiguous lookup for C.

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.

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.

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:

struct B : A {
  B() : A(), x() { } // <- ASSERT
  int x;
};

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.

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.

Andrew Sutton
andrew.n.sutton at gmail.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20100308/d90b1584/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pch.patch
Type: application/octet-stream
Size: 81060 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20100308/d90b1584/attachment.obj>


More information about the cfe-dev mailing list