<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 23 January 2015 at 17:43, Filipe Cabecinhas <span dir="ltr"><<a href="mailto:filcab@gmail.com" target="_blank">filcab@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">It's strictly better than leaving an assert/unreachable, since it's user input, not bad API usage.<div>I'm not very familiar with BitcodeReader's API, so in this case I would prefer to fix it right away, and later we could figure out how to make the API recoverable.</div></div></blockquote><div><br></div><div>I agree with both of you :-)</div><div>It is an improvement, but not ideal. If the ideal is not obvious we can do it in two steps and get the test passing for now. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div></div><div>Although:</div><div>In this case, we can return something like -1, for now, making some callers skip over the thing or returning an error. The problem is: this is supposed to be an abbrev record. If you can't read one of its parts, what do you do?</div><div>Otherwise, I can make readRecord return an std::error_code, since most of its callers do the same, and propagate the error, since all of its callers (AFAICT) return an error_code too.</div><span class=""><font color="#888888"><div><br></div></font></span></div></blockquote><div><br></div><div>In the case of bit code, the error handling strategy is somewhat clear. I think what would be needed is:</div><div><br></div><div>* Pass a DiagnosticHandlerFunction down to the constructor. The BitcodeReader can then use the one from the BitstreamReader.</div><div>* Have functions that can fail diagnose the failure and return an error_code or ErrorOr<something></div><div><br></div><div>It shouldn't be hard, but is non trivial, so I would be OK with report_fatal_error first.<br></div><div><br></div><div>Cheers,</div><div>Rafael</div><div><br></div></div></div></div>