[PATCH] D96831: [obj2yaml,yaml2obj] Add NumBlocks to the BBAddrMapEntry yaml field.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

It looks like you somehow pushed an older version of this change? Several of the typos/minor changes you fixed in previous iterations have been undone. Please take a look and fix accordingly.

I used this link to spot differences that look bad to me: https://reviews.llvm.org/D96831?vs=324449&id=324451#toc



================
Comment at: llvm/lib/ObjectYAML/ELFYAML.cpp:1668
   IO.mapOptional("BBEntries", E.BBEntries);
+  IO.mapOptional("NumBlocks", E.NumBlocks);
 }
----------------
This line probably should be above the `BBEntries` line, right?


================
Comment at: llvm/test/tools/obj2yaml/ELF/bb-addr-map.yaml:133
+## Check that obj2yaml uses the "Content" tag to describe an .llvm_bb_addr_map section
+## when it can't extract the entries, for example, when section is truncated, or when
+## an invalid NumBlocks field is specified.
----------------
jhenderson wrote:
> 
I don't think this typo (the missing "the" before "section") has been fixed.


================
Comment at: llvm/tools/obj2yaml/elf2yaml.cpp:864
     }
-    Entries.push_back({Address, BBEntries});
+    Entries.push_back({Address, /* NumBlocks */ {}, BBEntries});
   }
----------------
jhenderson wrote:
> clang-format seems to recognise this comment styling, and it seems clearer to me.
Similarly, this issue hasn't been addressed in the actual commit that appears on github.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96831