[PATCH] D102601: [NFC][object] Change the input parameter of the method isDebugSection.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 18 01:10:26 PDT 2021


jhenderson added inline comments.


================
Comment at: llvm/lib/Object/MachOObjectFile.cpp:2039
+bool MachOObjectFile::isDebugSection(DataRefImpl Sec) const {
+  if (Expected<StringRef> SectionNameOrErr = getSectionName(Sec)) {
+    StringRef SectionName = SectionNameOrErr.get();
----------------
I recognise that this is the common pattern used elsewhere in this file, but if I'm not mistaken, it can lead to an unhandled Error should `getSectionName` ever fail, which is an assertion when certain options are enabled.

At the very least, there needs to be a `consumeError(SectionNameOrErr.takeError())` line after the if, with a TODO saying to report the error message properly. If you can prove that `getSectionName(Sec)` never returns an error, you can use `cantFail` and avoid the TODO instead.

Same applies to each of the other formats.


================
Comment at: llvm/lib/Object/ObjectFile.cpp:97
 
-bool ObjectFile::isDebugSection(StringRef SectionName) const {
+bool ObjectFile::isDebugSection(DataRefImpl Sec) const {
   return false;
----------------
Nit: clang-format as directed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102601



More information about the llvm-commits mailing list