[lld] r240147 - [lld] Allow LLD passes to return error codes.

[David Blaikie]

On Fri, Jun 19, 2015 at 2:24 PM, Lang Hames <lhames at gmail.com> wrote:

> Hi Davide,
>
> This was posted for review at http://reviews.llvm.org/D10552
> <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=>
> and reviewed by Rui, who Ok'd it. The approach was also verbally Ok'd by
> Nick Kledzik.
>
> The phab review should have gone out to lld subscribers, but it doesn't
> look like emails were sent to llvm-commits.
>

Yeah - please ensure reviews actually happen on-list. Otherwise we get
complaints about unreviewed commits/'secret' review cabals, etc.

As phab says at the top of your review "

   - Always subscribe llvm-commits, cfe-commits or lldb-commits! Please
   create a new revision, adding one of the lists."


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.

(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)

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.
>
> 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.
>
> - Lang.
>
> On Fri, Jun 19, 2015 at 1:02 PM, Davide Italiano <davide at freebsd.org>
> wrote:
>
>> On Fri, Jun 19, 2015 at 10:51 AM, Lang Hames <lhames at gmail.com> wrote:
>> > Author: lhames
>> > Date: Fri Jun 19 12:51:46 2015
>> > New Revision: 240147
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=240147&view=rev
>> <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=>
>> > Log:
>> > [lld] Allow LLD passes to return error codes.
>> >
>>
>> Hi Lang,
>> this is a relatively large change. For the future it would be nice if
>> we can discuss this before checking in (e.g. on llvm-devel).
>> That said, do you have an use-case for this? In the past we explicitly
>> tried to not change return type from 'void' to 'std::error_code' just
>> in case.
>>
>> Thanks!
>>
>> --
>> Davide
>>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150619/3e946d67/attachment.html>