LGTM when fixes applied.
As discussed offline, John is buried under email, so we'll move to
post-commit review.
================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:707
@@ +706,3 @@
+ // a unique mangled name.
+ llvm::StringSet<> ObservedMangledNames;
+ for (size_t J = 0, F = VFPtrs.size(); J != F; ++J) {
----------------
Put this in #ifndef NDEBUG.
================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:673
@@ +672,3 @@
+ llvm::raw_svector_ostream Out(Name);
+ CGM.getCXXABI().getMangleContext().mangleCXXVFTable(
+ RD, VFPtr.PathToMangle, Out);
----------------
I don't know why MangleContext takes streams. It's not like we ever stream
these directly to files. I may write a patch later to change it to
SmallStringImpl &.
================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:661
@@ +660,3 @@
+ llvm::Value *VTableAddressPoint =
+ CGM.getCXXABI().getAddrOfVTable(VTableClass, Base.getBaseOffset());
+ if (!VTableAddressPoint) {
----------------
No need for "CGM.getCXXABI()" inside MicrosoftCXXABI.
================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:663
@@ +662,3 @@
+ if (!VTableAddressPoint) {
+ const CXXRecordDecl *BaseDecl = Base.getBase();
+ assert(BaseDecl->getNumVBases() &&
----------------
This will warn in a clang NDEBUG build, probably just fold it into the assert.
================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:694
@@ +693,3 @@
+
+ llvm::GlobalVariable *&VTable = VFTablesMap[ID];
+
----------------
You can avoid the double DenseMap lookup with .insert(), and you can use
llvm:tie() to avoid the ugly std::pair type:
VFTablesMapTy::iterator I;
bool Inserted;
llvm::tie(I, Inserted) = VFTablesMap.insert(0);
...
http://llvm-reviews.chandlerc.com/D1532
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
