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

[James Henderson via Phabricator via llvm-commits]

jhenderson 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
----------------
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`.


================
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());
----------------
If I'm not mistaken, this case needs a unit test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78558