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

Daniel Jasper djasper at google.com
Mon Sep 15 05:47:33 PDT 2014


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/7f15c172/attachment.html>


More information about the cfe-commits mailing list