================
Comment at: lib/AST/VTableBuilder.cpp:2322
@@ -2354,3 +2321,3 @@
Builder.getAddressPoints(),
- Builder.isMicrosoftABI());
+ false);
}
----------------
Reid Kleckner wrote:
> /*IsMicrosoftABI=*/
Done
================
Comment at: lib/CodeGen/CGExprConstant.cpp:56
@@ -55,4 +55,3 @@
- void AppendVTablePointer(BaseSubobject Base, llvm::Constant *VTable,
- const CXXRecordDecl *VTableClass);
+ void AppendVTablePointer(BaseSubobject Base, const CXXRecordDecl
*VTableClass);
----------------
Reid Kleckner wrote:
> This method is not defined, so you can drop it.
Good catch - I forgot to remove the declaration.
================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:325
@@ +324,3 @@
+
+ /// EmittedVFTables - Holds the set of the record decls we've deferred vtable
+ /// emission for.
----------------
Reid Kleckner wrote:
> "EmittedVFTables" is wrong. Maybe just \brief?
Done; changed the VBTablesMap comment too for consistency.
================
Comment at: include/clang/AST/Mangle.h:116
@@ -111,1 +115,3 @@
+ const CXXRecordDecl *LastVBase,
+ bool TheOnlyVFTable, raw_ostream &) = 0;
virtual void mangleCXXVTT(const CXXRecordDecl *RD,
----------------
Reid Kleckner wrote:
> Ultimately, for vbtables, I needed to walk the entire inheritance graph of
> the class to figure out how things were mangled. As a result, I ended up
> feeding the mangler a carefully constructed BasePath and dropping all the
> complex logic from the mangler. Long term, we'll have to do the same here,
> so I'd drop TheOnlyVFTable and LastVBase and adjust the BasePath before
> feeding it to the mangler.
OK, done
================
Comment at: lib/AST/MicrosoftMangle.cpp:1873
@@ +1872,3 @@
+ Mangler.getStream() << "6B"; // '6' for vftable, 'B' for const.
+ if (!TheOnlyVFTable) {
+ // FIXME: In some rare cases this code produces a bit incorrect mangling.
----------------
Reid Kleckner wrote:
> Yep, I'd move this all out to the caller.
Agreed, done
================
Comment at: include/clang/AST/Mangle.h:109
@@ -108,1 +108,3 @@
raw_ostream &) = 0;
+ // FIXME: Some of these objects only exist in select ABIs. We should probably
+ // only declare them in ABI-specific manglers?
----------------
Reid Kleckner wrote:
> A MicrosoftMangle.h which only exposes MicrosoftMangleContext and is only
> included by MicrosoftCXXABI.cpp (ditto for Itanium)? Sounds like a good
> separate patch.
>
Would you mind if I do this after landing this?
Also, it'd be a good time to rename VTable{Context,Builder} ->
ItaniumVTable{Context,Builder} then.
http://llvm-reviews.chandlerc.com/D1532
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
