<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jun 19, 2015 at 2:34 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Fri, Jun 19, 2015 at 2:24 PM, Lang Hames <span dir="ltr"><<a href="mailto:lhames@gmail.com" target="_blank">lhames@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">Hi Davide,<div><br></div><div>This was posted for review at <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D10552&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=OZ5r9VBvBy5q2XYcqBT-4wGG0xlR7yIDM58cx6UsgjE&s=3tTxGTj-YXL0OCJprC4kLqmLi5q33gz25dAM-O--ss4&e=" target="_blank">http://reviews.llvm.org/D10552</a> and reviewed by Rui, who Ok'd it. The approach was also verbally Ok'd by Nick Kledzik.</div><div><br></div><div>The phab review should have gone out to lld subscribers, but it doesn't look like emails were sent to llvm-commits. </div></div></blockquote></span><div><br>Yeah - please ensure reviews actually happen on-list. Otherwise we get complaints about unreviewed commits/'secret' review cabals, etc.<br><br>As phab says at the top of your review "<ul style="margin:0px 0px 0px 16px;padding:0px;border:0px;color:rgb(188,120,55);font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:13px;line-height:16.0029983520508px;background-color:rgb(253,245,212)"><li style="margin:0px;padding:0px;border:0px">Always subscribe llvm-commits, cfe-commits or lldb-commits! Please create a new revision, adding one of the lists."</li></ul><br>Basically if you don't write one of those names in the review/cc/to line when you create the review, you're hosed - abandon the review & start again. Yes, there's no warning you're about to make that mistake, only that message after-the-fact. It's lame.<br><br>(basically once you've created the review, that initial "hey, I'm sending this out for review, here's the attached patch and list of changed files, etc" email is never sent again - so there's no way to include the mailing list effectively at that point - if you add them after-the-fact they'll just see the new message, without any context about the code review, no patch, etc)<br></div></div></div></div></blockquote><div><br>Oh, and closing the review via 'arc' will automatically close the review. Alternatively if you want to commit manually, including the "Differential Revision: DXXXX" (I think that's the syntax, check other commits to be sure) will help us follow back to the review from a commit, and also allows Phab to auto-close the review when it picks up that commit.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></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"><span class=""><div dir="ltr"><div>That's odd, but I'm still digesting Phabricator's interface, so I might have missed a tag somewhere. I'll try to check next time I post a review.</div><div><br></div><div>Is there any reason not to return std::error_code from passes? It seems reasonable that a pass may fail. The specific case I have in mind is TLV relocation processing on MachO: If the OS version min is too low we want to error out and report back to the user. This kind of use seems to have been anticipated: The PassManager already returns an error code, although it was always default constructed (no error), and not yet checked. </div><span><font color="#888888"><div><br></div><div>- Lang.</div></font></span></div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jun 19, 2015 at 1:02 PM, Davide Italiano <span dir="ltr"><<a href="mailto:davide@freebsd.org" target="_blank">davide@freebsd.org</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"><span>On Fri, Jun 19, 2015 at 10:51 AM, Lang Hames <<a href="mailto:lhames@gmail.com" target="_blank">lhames@gmail.com</a>> wrote:<br>
</span><span>> Author: lhames<br>
> Date: Fri Jun 19 12:51:46 2015<br>
> New Revision: 240147<br>
><br>
> URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D240147-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=OZ5r9VBvBy5q2XYcqBT-4wGG0xlR7yIDM58cx6UsgjE&s=0qOitMInM5VJbjHR-Yy31bxNbzVT1ZG-ygCl4OMZYW4&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=240147&view=rev</a><br>
> Log:<br>
> [lld] Allow LLD passes to return error codes.<br>
><br>
<br>
</span>Hi Lang,<br>
this is a relatively large change. For the future it would be nice if<br>
we can discuss this before checking in (e.g. on llvm-devel).<br>
That said, do you have an use-case for this? In the past we explicitly<br>
tried to not change return type from 'void' to 'std::error_code' just<br>
in case.<br>
<br>
Thanks!<br>
<br>
--<br>
Davide<br>
</blockquote></div><br></div>
</div></div><br></span><span class="">_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></span></blockquote></div><br></div></div>
</blockquote></div><br></div></div>