Looks really good. Few nits here.

  I would add a comment about memoization just so that if this shows up on a 
profile, we remember the discusion we had on IRC about it.


================
Comment at: lib/CodeGen/CGExprCXX.cpp:1730
@@ +1729,3 @@
+       I != E; ++I) {
+    if (I->Access == AS_public) { // Ignore non-public inheritance.
+      for (CXXBasePath::iterator J = I->begin(), JE = I->end(); J != JE; ++J) {
----------------
As Doug commented, a continue seems better here.

================
Comment at: lib/CodeGen/CGExprCXX.cpp:1717
@@ +1716,3 @@
+  CXXBasePaths Paths(/*FindAmbiguities=*/true, /*RecordPaths=*/true,
+                     /*DetectVirtual=*/false);
+
----------------
Why not detect virtual? Then the path building can short circuit, and you can 
just detect it and return the -1?

================
Comment at: lib/CodeGen/CGExprCXX.cpp:1737-1740
@@ +1736,6 @@
+
+        // Accumulate the base class offsets.
+        const ASTRecordLayout &Layout = Context.getASTRecordLayout(J->Class);
+        Offset +=
+          Layout.getBaseClassOffset(J->Base->getType()->getAsCXXRecordDecl());
+      }
----------------
If we're on our second path, you can skip this computation.


http://llvm-reviews.chandlerc.com/D364
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to