[PATCH] clang-format Test: Split BackwardCompatibility out of ParsesConfiguration

Daniel Jasper djasper at google.com
Mon Sep 15 06:10:28 PDT 2014


I see, sorry for not reading more carefully. I don't think creating a
specific macro for this case is worth it. In general we don't need to
rename flags.

On Mon, Sep 15, 2014 at 3:08 PM, Jean-philippe Dufraigne <
j.dufraigne at gmail.com> wrote:

> Thanks you for your feedback.
>
> DerivePointerBinding was the legacy name of DerivePointerAlignment.
> It was one of the missing test.
>
> 2014-09-15 13:47 GMT+01:00 Daniel Jasper <djasper at google.com>:
>
>>
>>
>> On Mon, Sep 15, 2014 at 2:36 PM, Jean-Philippe Dufraigne <
>> j.dufraigne at gmail.com> wrote:
>>
>>> Thank you for your feedback.
>>>
>>> The following can only be added in ParsesConfigurationBools under
>>> DerivePointerAlignment if the CHECK_PARSE macro is moved above.
>>>
>>>   Style.DerivePointerAlignment = true;
>>>   CHECK_PARSE("DerivePointerBinding: false", DerivePointerAlignment,
>>> false);
>>>   CHECK_PARSE("DerivePointerBinding: true", DerivePointerAlignment,
>>> true);
>>>
>>> In order to keep all the test for a single flag in the same place, would
>>> it be better define a second macro instead?CHECK_PARSE_BOOL_COMPATIBLE  (or
>>> CHECK_PARSE_BOOL_LEGACY) defined and undefined with CHECK_PARSE_BOOL.
>>>
>>
>> I don't understand. What does have DerivePointerAlignment to do with it?
>> This is a separate flag, which is and will remain of bool-type and is not a
>> legacy flag.
>>
>> Would it also make sense to have a similar set of function for
>>> ParsesConfiguration?
>>> CHECK_PARSE_ITEM and CHECK_PARSE to have:
>>>
>>>   Style.PointerAlignment = FormatStyle::PAS_Middle;
>>>   CHECK_PARSE_ITEM("Left", PointerAlignment, FormatStyle::PAS_Left);
>>>   CHECK_PARSE_ITEM("Right", PointerAlignment, FormatStyle::PAS_Right);
>>>   CHECK_PARSE_ITEM("Middle", PointerAlignment, FormatStyle::PAS_Middle);
>>>   // For backward compatibility:
>>>   CHECK_PARSE("PointerBindsToType: Left", PointerAlignment,
>>>               FormatStyle::PAS_Left);
>>>   CHECK_PARSE("PointerBindsToType: Right", PointerAlignment,
>>>               FormatStyle::PAS_Right);
>>>   CHECK_PARSE("PointerBindsToType: Middle", PointerAlignment,
>>>               FormatStyle::PAS_Middle);
>>>
>>
>> I don't think there is much gain at this point, but if so, I'd like to
>> have a function/macro like this:
>>
>> CHECK_PARSE_ENUM(
>>     PointerAlignment,
>>     {"Left", "Middle", "Right"},
>>     {FormatStyle::PAS_Left, FormatStyle::PAS_Middle,
>> FormatStyle::PAS_Right});
>>
>> Since CHECK_PARSE is also used in ParsesConfigurationWithLanguages, I
>>> would avoid rename it to CHECK_PARSE_ITEM_COMPATIBLE.
>>>
>>
>> I don't understand what you want to rename to what and why.
>>
>> I'll work on addressing the issue this evening.
>>>
>>> http://reviews.llvm.org/D5346
>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140915/c00bd714/attachment.html>


More information about the cfe-commits mailing list