This is an archive of the discontinued LLVM Phabricator instance.

llvm-vtabledump: A vtable dumper
ClosedPublic

Authored by majnemer on Jul 18 2014, 11:37 AM.

Details

Summary

This tool's job is to dump the vtables inside object files. It is
currently limited to MS ABI vf- and vb-tables but it will eventually
support Itanium-style v-tables as well.

Diff Detail

Repository
rL LLVM

Event Timeline

majnemer updated this revision to Diff 11653.Jul 18 2014, 11:37 AM
majnemer retitled this revision from to llvm-vtabledump: A vtable dumper.
majnemer updated this object.
majnemer added reviewers: hansw, rnk, Bigcheese.
majnemer added a subscriber: Unknown Object (MLST).
hans added a subscriber: hans.Jul 18 2014, 2:45 PM
hans added inline comments.
tools/llvm-vtabledump/Error.cpp
20 ↗(On Diff #11653)

Why the leading underscore?

27 ↗(On Diff #11653)

How about just defining this and message() in the class?

tools/llvm-vtabledump/Error.h
36 ↗(On Diff #11653)

Is this OK because we're not adding anything to namespace std, just specializing an already existing template in there?

tools/llvm-vtabledump/llvm-vtabledump.cpp
58 ↗(On Diff #11653)

Could this just call reportError(Input, EC.message()) ?

71 ↗(On Diff #11653)

Should this flush like the other reportError does?

82 ↗(On Diff #11653)

Maybe a comment about what ??_7 signifies?

Maybe the stuff in this if branch can be broken out to a helper function?

(These comments apply to the ??_8 side below too.)

135 ↗(On Diff #11653)

IIRC, \brief isn't necessary on one-line doxygen comments. Applies below too.

tools/llvm-vtabledump/llvm-vtabledump.h
1 ↗(On Diff #11653)

I thought we usually put "C++" in these things, but not in the tools/ dir?

10 ↗(On Diff #11653)

Wrong include guard name.

majnemer updated this revision to Diff 11681.Jul 18 2014, 8:35 PM
  • Address review comments.
hans accepted this revision.Jul 19 2014, 7:54 AM
hans added a reviewer: hans.

lgtm

This revision is now accepted and ready to land.Jul 19 2014, 7:54 AM
majnemer closed this revision.Jul 24 2014, 4:23 PM
majnemer updated this revision to Diff 11860.

Closed by commit rL213903 (authored by @majnemer).