[PATCH] D63713: WIP: DataExtractor error handling

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: lib/Support/DataExtractor.cpp:37
+              bool isLittleEndian, const char *Data, llvm::Error *Err) {
+  setErrorToUnchecked(Err);
   uint32_t offset = *offset_ptr;
----------------
I /think/ these should be early-returns if the Error is already set (assuming the goal is to support repeated calls to deserialization functions with the same Error and only check once at the end of the sequence of deserializations)

Also, you will need to be careful with how you check if the Error is already set - since you want to make sure you don't set the Error to the "checked" state (otherwise the caller won't be forced to check it, which is bad).

Hmm, perhaps you actually want to test that the Error is checked. If the error is unchecked, then return early & leave it to the caller to check.

The "unexpectedEndReached" can then just do "if (E) *E = ...;" without having to check the error state.

If the goal is to treat the Error as a straight out parameter without the repeated deserialization support (because something at a higher level will handle that in some way) - then maybe this should be ErrorAsOutParameter instead?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63713