[PATCH] D106783: [AIX][XCOFF][llvm-readobj] Replace unwrapOrError with reportUniqueWarning

[Maryam via Phabricator via llvm-commits]

MaryamBen added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/XCOFF/relocations32.yaml:11
+# CHECK-NEXT:   Section (index: 2) .data {
+# CHECK-NEXT:     {{.*}} warning: {{.*}} Invalid symbol index
+# CHECK-NEXT:   }
----------------
jhenderson wrote:
> What's the full message here that you are matching?
The warning occurs due to the invalid symbol index used to obtain the symbol name. 

```
Expected<StringRef> ErrOrSymbolName =
          Obj.getSymbolNameByIndex(Reloc.SymbolIndex);
```

The full message is the following :
full_path/llvm-readobj: warning: full_path/report_warning.yaml.tmp: Invalid symbol index


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:139
+    if (Error E = ErrOrRelocations.takeError())
+      reportUniqueWarning(std::move(E));
+
----------------
jhenderson wrote:
> MaryamBen wrote:
> > jhenderson wrote:
> > > 1) Is this tested?
> > > 2) You'll need to break out of or continue this loop here, as `*ErrOrRelocations` will result in referencing invalid memory if there is an error. (reportUniqueWarning reports the warning and continues).
> > I don't know how to test it. 
> > None of the existing tests reports the UniqueWarning
> To test this, you'll probably need to craft an XCOFF object (probably using yaml2obj) to trigger the report of the error from `Obj.relocations`. You may need to expand on yaml2obj's existing behaviour to allow creating an invalid input.
> 
> Just because something isn't tested already, doesn't mean it shouldn't be now, and this is a good chance to add the testing.
I was not able to introduce an error to check this warning but I checked the other one.


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

https://reviews.llvm.org/D106783