[llvm-commits] PATCH: add new test tool: obj2yaml

[Sean Silva]

what's the deal with this my_pair business? Either use std::pair or leave a
comment explaining why you did this. I suspect it has to do with POD-ness
and static initializers? if so, call it pod_pair.

Even better, you can probably get away with having one struct type with int
or unsigned (whichever signedness enum values have by default) as their
first member, and avoid having a bunch of bit-identical instantiations of
std::map for all these different types.

You should see whether the maps are even needed at all. A linear search may
be faster for small fixed sets of data like this.

Also, in LLVM, most (all?) code uses one space to separate the initial //
from the comment text itself. You are consistently using 2.

in writeHexStream, please make
const char *hex = "0123456789ABCDEF";
into
static const char hex[] = "0123456789ABCDEF";

+llvm::raw_ostream &writeHexNumber(llvm::raw_ostream &Out, unsigned long
long N) {
+  if ( N > 10 )
+    Out << "0x";
+  Out.write_hex(N);
+  return Out;
+}

I think you mean >= 10, otherwise, it'll just output an inexplicable "A"
with no "0x".

+const char endl = '\n';

why? either just write '\n' in the code, or as in many cases it seems in
the code, just include "\n" in the string literal instead of causing a
pointless extra call to raw_ostream's operator<<. e.g. instead of
+        Out << "        Type: " << endl;
just do
+        Out << "        Type: \n"

--Sean Silva

On Tue, Jun 12, 2012 at 8:35 AM, Marshall Clow <mclow.lists at gmail.com>wrote:

> On Jun 9, 2012, at 2:44 AM, Michael Spencer wrote:
>
> > On Fri, Jun 8, 2012 at 11:09 AM, Marshall Clow <mclow.lists at gmail.com>
> wrote:
> >> [ resending to llvm-commits instead of -dev ]
> >>
> >> This patch adds a tool called 'obj2yaml', which takes an object file,
> and produces a YAML representation of the file.
> >> This is a companion tool to Michael Spencer's proposed "yaml2obj" tool.
> >>
> >> This tool processes only COFF files; the plan is to enhance it to
> handle other object formats, as well as object archives.
> >> The primary uses for this tool is expected to be:
> >>        1) Debugging object files that are produced by llvm and lld.
> >>        2) Automated testing suites.
> >>
> >> The goal is to have obj2yaml and yaml2obj be inverse operations, so
> that one can test either by using the other.
> >> So, both obj -> yaml -> obj and yaml -> obj -> yaml are expected to be
> idempotent.
> >>
> >> -- Marshall
> >>
> >> Marshall Clow     Idio Software   <mailto:mclow.lists at gmail.com>
> >>
> >> A.D. 1517: Martin Luther nails his 95 Theses to the church door and is
> promptly moderated down to (-1, Flamebait).
> >>       -- Yu Suzuki
> >
> > The design looks good, however there are quite a few LLVM style issues.
>
> Extensively reworked. Now uses the facilities in llvm/Object/COFF.h for
> byte-sex awareness.
> Still a few 80 column issues, and the relocation dumping is not done - but
> since yaml2obj doesn't build relocation entries yet, this may be moot. ;-)
>
> -- Marshall
>
> Marshall Clow     Idio Software   <mailto:mclow.lists at gmail.com>
>
> A.D. 1517: Martin Luther nails his 95 Theses to the church door and is
> promptly moderated down to (-1, Flamebait).
>        -- Yu Suzuki
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120612/f3865a1d/attachment.html>