[PATCH] D63779: [llvm-objdump] Warn if no user specified sections (-j) are found.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/test/tools/llvm-objdump/warn-missing-section.test:21
+## - Warn for each specified section if none of them are found. Be consistent
+##   with GNU, make sure the output is in reverse order.
+# RUN: llvm-objdump --reloc --section=multi1 --section=multi2 %t.2.o 2>&1 \
----------------
This comment needs an update now.


================
Comment at: llvm/test/tools/llvm-objdump/warn-missing-section.test:63
+  Content: 90
+  Flags:   [SHF_EXECINSTR, SHF_ALLOC]
+
----------------
You do not need `Content` and `Flags` here and below I think.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:349
     return false;
+  if (!String.empty())
+    FoundSectionSet.insert(String);
----------------
I'd add a comment explaining when `String` can be empty.
Also, `String` is not a good variable name. Should it be `Name` or `SecName` may be?


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:443
+      return;
+    MissingSections.insert(S);
+  }
----------------
Instead, can you report a warning right here?

```
for (StringRef S : FilterSections) {
  if (FoundSectionSet.count(S))
    return;
  warn("section '" + S + "' mentioned in a -j/--section option, but not "
    "found in any input file");
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63779