[PATCH] D23875: Ease dealing with tagged enum ErrorDescription with some macros.

Filipe Cabecinhas via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 9 06:42:13 PDT 2016


I have submitted several patches (D24389, D24390, D24391, D24392,
D24393, and D24394) that show why it's desirable to make sure that all
these places handle *all* the cases in the enum. (They're all based on
the current patch, so use the macro)
Not just due to not having as much code, but also to make sure all
three places handle all the cases and none is forgotten. Otherwise
it's too easy to miss some places when adding new error structures)

After those, there are four more patches. I don't think it helps to
post all those right now. They're all similar (reify the error +
mechanical changes), except for the "Generic Error", which has more
details to account for.

Thank you,
 Filipe

On Thu, Sep 8, 2016 at 9:10 PM, Vitaly Buka <vitalybuka at google.com> wrote:
> This patch can't wait in your local checkout until the rest (or some draft
> of it) is ready to start review.
>
> On Thu, Sep 8, 2016 at 11:53 AM Filipe Cabecinhas
> <filcab+llvm.phabricator at gmail.com> wrote:
>>
>> I'll keep it as is, but having a hypothetical case is very different from
>> "we're in the middle of doing this, that's the end goal, I'd like so save a
>> bunch of work, even if right now the savings aren't that big".
>> After all, it wouldn't make much sense to stop after converting a few of
>> these functions.
>>
>>
>> Thank you,
>>
>>  Filipe
>>
>>
>> On Thursday, 8 September 2016, Vitaly Buka <vitalybuka at google.com> wrote:
>>>
>>> Then let's keep it on hold. It does not improve code as is. We should
>>> avoid adding real complexity to simplify hypothetical use-cases.
>>>
>>> On Thu, Sep 8, 2016 at 11:34 AM Filipe Cabecinhas
>>> <filcab+llvm.phabricator at gmail.com> wrote:
>>>>
>>>> It will save much more after the other errors are reified. Putting it in
>>>> now avoids writing all that mechanical code which we know we will delete.
>>>>
>>>> All the Report* functions (one per error) will have a structure to
>>>> describe that error. Now there are 4 of those. Later they will be around 12
>>>> (don't have the code handy right now).
>>>>
>>>> Thank you,
>>>>
>>>>  Filipe
>>>>
>>>>
>>>> On Thursday, 8 September 2016, Vitaly Buka <vitalybuka at google.com>
>>>> wrote:
>>>>>
>>>>> vitalybuka added a comment.
>>>>>
>>>>> It saves 9 line of code, but makes it less readable.
>>>>> I strongly prefer existing code.
>>>>>
>>>>>
>>>>> https://reviews.llvm.org/D23875
>>>>>
>>>>>
>>>>>
>


More information about the llvm-commits mailing list