[PATCH] [ms-cxxabi] Improve vbtable name mangling accuracy

Reid Kleckner rnk at google.com
Fri Jan 3 15:33:49 PST 2014


  Thanks!  Committing.  Let the virtual record layout fuzzing commence.  :)


================
Comment at: lib/AST/VTableBuilder.cpp:3183
@@ -3181,2 +3182,3 @@
 
-namespace {
+static bool setsIntersect(const llvm::SmallPtrSet<const CXXRecordDecl *, 4> &A,
+                          const llvm::ArrayRef<const CXXRecordDecl *> &B) {
----------------
David Majnemer wrote:
> Shame we don't have a `SmallPtrSetRef` :/
const ref is fine for this.

================
Comment at: include/clang/AST/VTableBuilder.h:420
@@ -417,1 +419,3 @@
+  // FIXME: Uncomment when we've moved to C++11.
+  //VBTableInfo(const VBTableInfo &) = default;
 
----------------
David Majnemer wrote:
> Shame we don't have `LLVM_DEFAULTED_FUNCTION` :/
You'd have to compile out the whole declaration in pre C++11, though.  =/

================
Comment at: lib/AST/VTableBuilder.cpp:3254
@@ +3253,3 @@
+      // wasn't already extended with Base.
+      if (P->MangledPath.empty() || P->MangledPath.back() != Base)
+        P->NextBaseToMangle = Base;
----------------
David Majnemer wrote:
> This check seems fine but is `P->MangledPath.back() == Base` truly possible?
Yes!  That's what I fixed this morning.  Consider E in:
  struct A {};
  struct B : virtual A {};
  struct C : virtual B {};
  struct D : C {};
  struct E : C, D {};

C and D both have the same vbtable paths [[C], [B]].  We combine them for E (dropping the path to B in C in D since B is virtual) to get [[C], [B], [C, D]].

This check prevents us from getting [[C, C], [B], [C, B]].

================
Comment at: lib/AST/VTableBuilder.cpp:3285
@@ -3349,3 +3284,3 @@
 
-static bool pathCompare(VBTablePath *LHS, VBTablePath *RHS) {
-  return LHS->VBInfo.MangledPath < RHS->VBInfo.MangledPath;
+static bool pathCompare(VBTableInfo *LHS, VBTableInfo *RHS) {
+  return LHS->MangledPath < RHS->MangledPath;
----------------
David Majnemer wrote:
> `LHS` and `RHS` could be `const`.
Done.


http://llvm-reviews.chandlerc.com/D2509



More information about the cfe-commits mailing list