[PATCH] Speed up modules compilation.

Richard Smith richard at metafoo.co.uk
Tue May 19 13:30:36 PDT 2015

Essentially this seems fine. It'd be better if `visitDepthFirst` didn't actually walk the the entire module graph in the case where it's been told to `SkipChildren`, but we can leave that improvement for a later change.

Comment at: include/clang/Serialization/ModuleManager.h:295-296
@@ -283,4 +294,4 @@
   /// which will be passed along to the user.
-  void visitDepthFirst(bool (*Visitor)(ModuleFile &M, bool Preorder, 
-                                       void *UserData), 
+  void visitDepthFirst(bool (*PostorderVisitor)(ModuleFile &M, void *UserData),
+                       DFSPreorderControl (*PreorderVisitor)(ModuleFile &M, void *UserData),
                        void *UserData);
These parameters seem backwards to me (on each node, we'll call the preorder visitor before the postorder one, so it would seem more natural for the preorder one to be written first in a call to this function).

Comment at: lib/Serialization/ASTReaderDecl.cpp:3363
@@ +3362,3 @@
+    bool needsToVisitChildren(ModuleFile &M, GlobalDeclID GlobalID) {
+      DeclID ID = Reader.mapGlobalIDToModuleFileGlobalID(M, GlobalID);
`Children` here is ambiguous, and entirely backwards from the way I think of the imports graph (where the roots import nothing and the leaves are not imported by anything). Calling this `Imports` would remove the ambiguity (likewise in the `DFSPreorderControl` enum).

Comment at: lib/Serialization/ASTReaderDecl.cpp:3376-3378
@@ +3375,5 @@
+      }
+      unsigned Offset = Result->Offset;
+      unsigned N = M.RedeclarationChains[Offset];
+      return N != 0;
+    }
A comment explaining what's going on here ("we don't need to visit a module or any of its imports if we've already deserialized the redecls from this module") would help.



More information about the cfe-commits mailing list