[PATCH] D74205: [llvm-dwarfdump] Add the --show-sections-sizes option

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/tools/llvm-dwarfdump/SectionSizes.cpp:34
+static size_t getMaxSectionNameWidth(const SectionSizes &Sizes) {
+  size_t MaxWidth = 13;
+  for (const auto &DebugSec : Sizes.DebugSectionSizes) {
----------------
Why `13` here? I'd think it would only need to have a minimum of the size of "SECTION" + 2 or so.


================
Comment at: llvm/tools/llvm-dwarfdump/SectionSizes.cpp:48
+  OS << "  SECTION                   SIZE\n";
+  OS << "------------        -----------------------\n";
+
----------------
The dashes under "SECTION" should be increased in number based on the column width.


================
Comment at: llvm/tools/llvm-dwarfdump/SectionSizes.cpp:97
+bool collectObjectSectionSizes(ObjectFile &Obj, DWARFContext & /*DICtx*/,
+                               Twine Filename, raw_ostream &OS) {
+  SectionSizes Sizes;
----------------
`collectObjectSectionSizes` is new code, so it should use the right signature from day 1 rather than waiting until a later change. In fact, D75727 can probably be committed first.


================
Comment at: llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp:420
 
+bool collectObjectSectionSizes(ObjectFile &Obj, DWARFContext & /*DICtx*/,
+                               Twine Filename, raw_ostream &OS);
----------------
I just want to be clear: is this how clang-format formats this comment?


================
Comment at: llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp:648
       handleFile(Object, collectStatsForObjectFile, OutputFile.os());
-  else
+  } else if (ShowSectionSizes) {
+    for (auto Object : Objects)
----------------
A thought: is there any reason we can't allow --statistics and --show-section-sizes in the same run?


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

https://reviews.llvm.org/D74205