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

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 2 01:38:47 PDT 2020


jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM, with one suggestion.



================
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
----------------
labath wrote:
> 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.
Looks like you only changed the second case here? Potentially, `Size` could be invalid causing the error. What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78558





More information about the llvm-commits mailing list