[PATCH] D67328: [llvm-readobj] Warn user when dumping not supported version of stack map (PR38278).

Tue Sep 10 09:09:51 PDT 2019

jhenderson added inline comments.

================
Comment at: llvm/tools/llvm-readobj/StackMapPrinter.h:25
+  // FIXME: There could be multiple entries of different versions in
+  // .llvm_stackmaps section. See: https://bugs.llvm.org/show_bug.cgi?id=38277
+  uint64_t Offset = 0x0;
----------------
You can get rid of the ':' after "See" and add a trailing full stop.

I'd rephrase the first sentence as: "There could be multiple entries in a .llvm_stackmaps section, but this code only handles the first." The version issue isn't really the whole story here, so there's no need to specifically highlight it.

================
Comment at: llvm/tools/llvm-readobj/StackMapPrinter.h:32
+                                      " detected at offset 0x" +
+                                      utohexstr(Offset, true /* Lower case */) +
+                                      ", which is not supported."),
----------------
I think the usual pattern is `/*LowerCase=*/true` (where `LowerCase` is the parameter name).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67328