[PATCH] D84651: [llvm-readobj] - Don't call `unwrapOrErr` in `findSectionByName`.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 30 02:37:29 PDT 2020


grimar added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/mips-abiflags.test:384-386
+# NAME-ERR-FOUND-NEXT:      warning: '[[FILE]]': unable to locate the .MIPS.options section: there is no such section, or its name can't be read
+# NAME-ERR-FOUND-NEXT:      warning: '[[FILE]]': unable to locate the .reginfo section: there is no such section, or its name can't be read
+# NAME-ERR-FOUND-NEXT:      error: '[[FILE]]': unable to locate the .got section: there is no such section, or its name can't be read
----------------
jhenderson wrote:
> I think these additional "section not found" errors/warnings are confusing the issue of what's important in the test. I'd add the respective missing sections, so that the errors aren't emitted.
> 
> Alternatively, I'm not 100% convinced we should have this kind of warning/error at all, since the earlier warning(s) might be sufficient to say there are problems reading the section names. I might be incined to just fallback to the "not found" behaviour in that case, though like I say, I could be persuaded otherwise.
> 
> Same goes for all the other test cases.
> Alternatively, I'm not 100% convinced we should have this kind of warning/error at all, since the earlier warning(s) might be sufficient to say there are problems reading the section names. I might be incined to just fallback to the "not found" behaviour

OK. This is simpler and works for me. I've changed the behavior to this one.


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

https://reviews.llvm.org/D84651



More information about the llvm-commits mailing list