[PATCH] D111358: TargetLibraryInfo checker tool

[Renato Golin via Phabricator via llvm-commits]

rengolin added a comment.

Hi Paul, thanks for submitting this, it looks super cool.

I've got a few comments on the syntax and options, but nothing too controversial.

The actual code looks super clean and easy to read, except for the `SeparateMode` part (to me, totally personal).

Perhaps it would be simpler if you always use an array of reports, and in separate mode, you populate the array one by one, but on joint mode you append to the first item, then report the array, however many items are there?

But none of that is really important, they're all just comments.

cheers,
--renato



================
Comment at: llvm/docs/CommandGuide/llvm-tli-checker.rst:50
+  A base directory to prepend to each library file path. This is handy
+  when there are a number of library files all in the same directory.
+
----------------
So, the syntax:

    llvm-tli-checker ... --libdir=foo/bar a.o b.o c.o

will check against:
 * foo/bar/a.o
 * foo/bar/b.o
 * foo/bar/c.o

If so, couldn't this just be `foo/bar/*` and let the shell expand it?


================
Comment at: llvm/docs/CommandGuide/llvm-tli-checker.rst:65
+  Print information for each library call known to TLI, instead of just the
+  mismatches.
+
----------------
This feels like more of a separate functionality, for example, `--list`, then just being verbose.

I usually interpret `verbose` as showing the steps taken, not showing more information.

Instead, if I just want to list the symbols (for ex. for grepping), then do I need to create an empty object file and use `--verbose`?


================
Comment at: llvm/docs/CommandGuide/llvm-tli-checker.rst:74
+
+:program:`llvm-cxxfilt` returns 0 even if there are mismatches. It returns a
+non-zero exit code if there is an unrecognized option, or no input files are
----------------
`llvm-tli-checker`?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111358/new/

https://reviews.llvm.org/D111358