PCH for C++ (take 2)

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

PCH for C++ (take 2)

Andrew Sutton
Here's a second pass on PCH support for CXX decls. This patch has mostly complete support for namespaces, namespace aliases, using directives, using/shadow decls, and C++ classes (and their bases and injected class names, I suppose). There's some support written for some other C++ decls, but it's incomplete and not tested--stubbed out.

Also, I probably need to back-fill some of the source location stuff. I wasn't quite sure how much source location/range information I need to serialize. Any guidance along these lines would be appreciated.

I had to add a number of get/set functions to some of the Decls in the AST since it isn't generally feasible to create/init these objects at the same time. Also, the CXX Ctor/Dtor/Conversion Decls get extra Create methods since their usual Create functions would assert with an empty DeclarationName.

I'm not very happy with the serialization of classes. I'm currently writing a large number of properties that could be deduced from the addition of constructors, destructors, operators, and base classes. One one hand, it would be more graceful to allow the class to configure itself based on elements. On the other hand, there is some computational cost associated with the tests required to do that. Does anybody want to suggest a best approach before I start writing method serialization?

Andrew Sutton
[hidden email]

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

pch.diff (56K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] PCH for C++ (take 2)

John McCall

On Jan 26, 2010, at 6:48 AM, Andrew Sutton wrote:

> Here's a second pass on PCH support for CXX decls. This patch has mostly complete support for namespaces, namespace aliases, using directives, using/shadow decls, and C++ classes (and their bases and injected class names, I suppose). There's some support written for some other C++ decls, but it's incomplete and not tested--stubbed out.
>
> Also, I probably need to back-fill some of the source location stuff. I wasn't quite sure how much source location/range information I need to serialize. Any guidance along these lines would be appreciated.

PCH is our primary AST serialization mechanism, and many of the current and future clients of serialized ASTs are major consumers of accurate source locations.  Please preserve everything in the decl.

(that said, I certainly understand that this is all a lot of work — a lot of typing, if nothing else!  We can make do with the basic structure and do grunt-work later as long as you leave breadcrumbs so that the missing information is not forgotten)

> I had to add a number of get/set functions to some of the Decls in the AST since it isn't generally feasible to create/init these objects at the same time. Also, the CXX Ctor/Dtor/Conversion Decls get extra Create methods since their usual Create functions would assert with an empty DeclarationName.

That seems reasonable.  There's an EmptyShell class that's used to distinguish PCH constructors in the Expr classes;  that's probably a good idea here for these Create functions as well.

> I'm not very happy with the serialization of classes. I'm currently writing a large number of properties that could be deduced from the addition of constructors, destructors, operators, and base classes. One one hand, it would be more graceful to allow the class to configure itself based on elements. On the other hand, there is some computational cost associated with the tests required to do that. Does anybody want to suggest a best approach before I start writing method serialization?

The computational cost is probably not a serious problem, but requiring all the children of the decl to be deserialized might be.  Probably better to save and restore the bits individually, sorry.

--- include/clang/Frontend/PCHReader.h (revision 94542)
+++ include/clang/Frontend/PCHReader.h (working copy)
+#include "clang/AST/DeclCXX.h"

You can get away with just predeclaring CXXBaseSpecifier here; same thing in PCHWriter.h.

--- include/clang/AST/DeclCXX.h (revision 94542)
+++ include/clang/AST/DeclCXX.h (working copy)
+  /// setType - Set the type of the base class. This must be an unqualified
+  /// type.
+  // FIXME: Vaidate that T is unqualified.
+  void setType(QualType T) { BaseType = T; }

That would be assert(!T->hasLocalQualifiers()).

   bool hasUserDeclaredConstructor() const {
-    assert((isDefinition() ||
+    // FIXME: Injected class names are troublesome and cause assertions in
+    // the PCH writer if we don't allow them to be queried.
+    assert((isDefinition() || isInjectedClassName() ||

Probably best to just give injected class names a different layout in the PCH record;  otherwise they're going to occupy a lot of redundant space.

John.
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] PCH for C++ (take 2)

Andrew Sutton

PCH is our primary AST serialization mechanism, and many of the current and future clients of serialized ASTs are major consumers of accurate source locations.  Please preserve everything in the decl.

Sounds fair. I'll see what I can do. I'm trying to leave breadcrumbs whever I'm unsure what the correct behavior is.
 
That seems reasonable.  There's an EmptyShell class that's used to distinguish PCH constructors in the Expr classes;  that's probably a good idea here for these Create functions as well.

That's probably a good idea. I'll check it out.
 
--- include/clang/Frontend/PCHReader.h  (revision 94542)
+++ include/clang/Frontend/PCHReader.h  (working copy)
+#include "clang/AST/DeclCXX.h"

You can get away with just predeclaring CXXBaseSpecifier here; same thing in PCHWriter.h.

Yup. Missed that one.

  bool hasUserDeclaredConstructor() const {
-    assert((isDefinition() ||
+    // FIXME: Injected class names are troublesome and cause assertions in
+    // the PCH writer if we don't allow them to be queried.
+    assert((isDefinition() || isInjectedClassName() ||

Probably best to just give injected class names a different layout in the PCH record;  otherwise they're going to occupy a lot of redundant space.

That might be easier said than done. The injected class name is serialized as part of a DeclContext, just like any other nested declaration.
 
Andrew Sutton
[hidden email]

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [cfe-dev] PCH for C++ (take 2)

John McCall
On Jan 26, 2010, at 2:26 PM, Andrew Sutton wrote:
  bool hasUserDeclaredConstructor() const {
-    assert((isDefinition() ||
+    // FIXME: Injected class names are troublesome and cause assertions in
+    // the PCH writer if we don't allow them to be queried.
+    assert((isDefinition() || isInjectedClassName() ||

Probably best to just give injected class names a different layout in the PCH record;  otherwise they're going to occupy a lot of redundant space.

That might be easier said than done. The injected class name is serialized as part of a DeclContext, just like any other nested declaration.

I mean that, when you're serializing a CXXRecordDecl, you can ask if it's an injected class name and decide how to lay out the record decl based on that.

John.

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev