<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div>On Mar 2, 2013, at 11:00 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">On Sat, Mar 2, 2013 at 10:57 AM, Argyrios Kyrtzidis <<a href="mailto:akyrtzi@gmail.com">akyrtzi@gmail.com</a>> wrote:<br><blockquote type="cite">On Mar 2, 2013, at 10:42 AM, Rafael Espíndola <<a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>> wrote:<br><br><blockquote type="cite"><blockquote type="cite">libclang from the beginning has chosen to avoid crashing the process, see llvm::CrashRecoveryContext.<br></blockquote><br>/// This class implements support for running operations in a safe context so<br>/// that crashes (memory errors, stack overflow, assertion violations) can be<br>/// detected and control restored to the crashing thread. Crash detection is<br>/// purely "best effort", the exact set of failures which can be recovered from<br>/// is platform dependent.<br><br>This looks like exactly what we need. Instead of doing<br><br>if (!foo) {<br> assert(0...);<br>}<br><br>You can cause a "crash" and let the crash recovery mechanism handle it.<br></blockquote><br>Doing that will still ignore the crash, but this time with a recovery mechanism that, while good intentioned (not bringing down the process) it's disruptive and should really not be triggered intentionally.<br></blockquote><br>It's not intentional - if it was we wouldn't have the assert there &<br>this would just be an intended/valid codepath.<br></blockquote><div><br></div>I mean if the condition for the assertion occurs, triggering the crash recovery mechanism explicitly is bad.<br><div><br></div><div>Also, returning FileID() because something failed is a valid codepath there. A loaded source location entry may be invalid because the associated file was removed from the file system, which is why this check exists:</div><div><div style="margin: 0px; font-size: 11px; font-family: Monaco; "><br></div><div style="margin: 0px; font-size: 11px; font-family: Monaco; "> <span style="color: #931a68">if</span> (E.getOffset() == 0)</div><div style="margin: 0px; font-size: 11px; font-family: Monaco; color: rgb(78, 144, 114); "><span style="color: #000000"> </span><span style="color: #931a68">return</span><span style="color: #000000"> </span><span style="color: #006141">FileID</span><span style="color: #000000">(); </span>// invalid entry.</div><div><br></div></div><br><blockquote type="cite"><br>If we have some way to take "we're in a state we don't expect to be<br>in" (& importantly: haven't tested) & turn that in to a well defined<br>crash/error reporting that seems to be the right thing to have here.<br></blockquote></div><div><br></div><div>In a dogfooding situation an assertions-enabled build can be used along with disabling crash recovery.</div><div>In a release build for users, recovering as gracefully as possible and not taking down the process is deemed more important.</div></body></html>