[PATCH] D78558: [Support] Make DataExtractor error messages more clear

[Pavel Labath via Phabricator via llvm-commits]

labath marked 3 inline comments as done.
labath added inline comments.


================
Comment at: llvm/lib/Support/DataExtractor.cpp:25
+      *E = createStringError(
+          errc::illegal_byte_sequence,
+          "unexpected end of data at offset 0x%zx while reading [0x%" PRIx64
----------------
jhenderson wrote:
> FWIW, I'm not convinced `illegal_byte_sequence` is really the right thing to use. I had a discussion with someone else recently about this, but in every example usage I found online outside LLVM, it is for unicode encoding issues. The standard is silent on the exact purpose of it so the argument is that it's perfectly reasonable to use it for other bad sequences. I personally would use `invalid_argument` here, since you're trying to read using an invalid `Offset`.
Yes, invalid_argument looks better here.


================
Comment at: llvm/lib/Support/DataExtractor.cpp:30-33
+      *E = createStringError(errc::illegal_byte_sequence,
+                             "offset 0x%" PRIx64
+                             " is beyond the end of data at 0x%zx",
+                             Offset, Data.size());
----------------
jhenderson wrote:
> If I'm not mistaken, this case needs a unit test?
Test added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78558