[PATCH] D53051: [llvm-tapi] initial commit, supports ELF text stubs

[Eric Christopher via Phabricator via llvm-commits]

echristo added a comment.

Some drive by comments again. Be good to get @ributzka to take a final look.



================
Comment at: llvm/lib/TAPI/TBEHandler.cpp:1
+//===- TBEHandler.cpp -----------------------------------------------------===//
+//
----------------
This file could use some more comments in general. 


================
Comment at: llvm/lib/TAPI/TBEHandler.cpp:98
+    // Force "Symbols: {}" if empty.
+    // TODO: fix this in YAMLTraits.
+    if (List.size() == 0) {
----------------
Elaborate on what needs to be done to fix the TODO please.


================
Comment at: llvm/lib/TAPI/TBEHandler.cpp:111
+  static void mapping(IO &IO, ELFStub &Stub) {
+    if (!IO.mapTag("!tapi-tbe-v1", true))
+      IO.setError("Invalid tbe version");
----------------
I'd like to try to avoid "v1" style naming for things. Any hope of an explicit version field instead?


Repository:
  rL LLVM

https://reviews.llvm.org/D53051