<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 3 December 2013 15:25, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Mind using phabricator? Makes the review much faster.<div></div></div></blockquote><div>
<br></div><div>OK.  Posted as <a href="http://llvm-reviews.chandlerc.com/D2342">http://llvm-reviews.chandlerc.com/D2342</a> (which has showed up as a separate thread, <a href="http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20131202/197796.html">http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20131202/197796.html</a>).<br>
<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>I'd rather someone more familiar with exception handling in LLVM review this. I worry about it mostly because there were explicit tests for the duplication that you've had to remove.</div>
</div></blockquote><div><br></div><div>Those tests look like they were copy-and-pasted from the output of Clang and 'opt' without being checked closely -- hence use of mangled names such as "_ZTIi" (rather than more readable names that you might write if writing the LLVM assembly by hand, as I did for the test case in this patch).<br>
<br></div><div>Having the same "catch <foo>" clause twice in the same landingpad is definitely unnecessary.  When the personality function is checking whether to pass control to a landingpad, it checks each clause against the exception in order.  If the first "catch <foo>" clause matches the exception, the second "catch <foo>" clause won't be looked at, and if the first doesn't match the second won't match either.  So the second "catch <foo>" will have no effect.<br>
<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>While in the abstract your description makes perfect sense and I have no idea why those were there, I also don't know the EH side of LLVM very well at all. Bill is really the perfect reviewer here I suspect. =]</div>
</div></blockquote><div><br></div><div>OK.  I CC'd Bill originally but he hasn't responded.<br><br>Cheers,<br>Mark<br></div><br></div></div></div>