<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Sep 15, 2014 at 2:36 PM, Jean-Philippe Dufraigne <span dir="ltr"><<a href="mailto:j.dufraigne@gmail.com" target="_blank" class="cremed">j.dufraigne@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Thank you for your feedback.<br>
<br>
The following can only be added in ParsesConfigurationBools under DerivePointerAlignment if the CHECK_PARSE macro is moved above.<br>
<br>
  Style.DerivePointerAlignment = true;<br>
  CHECK_PARSE("DerivePointerBinding: false", DerivePointerAlignment, false);<br>
  CHECK_PARSE("DerivePointerBinding: true", DerivePointerAlignment, true);<br>
<br>
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.<br></blockquote><div><br></div><div>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.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Would it also make sense to have a similar set of function for ParsesConfiguration?<br>
CHECK_PARSE_ITEM and CHECK_PARSE to have:<br>
<br>
  Style.PointerAlignment = FormatStyle::PAS_Middle;<br>
  CHECK_PARSE_ITEM("Left", PointerAlignment, FormatStyle::PAS_Left);<br>
  CHECK_PARSE_ITEM("Right", PointerAlignment, FormatStyle::PAS_Right);<br>
  CHECK_PARSE_ITEM("Middle", PointerAlignment, FormatStyle::PAS_Middle);<br>
  // For backward compatibility:<br>
  CHECK_PARSE("PointerBindsToType: Left", PointerAlignment,<br>
              FormatStyle::PAS_Left);<br>
  CHECK_PARSE("PointerBindsToType: Right", PointerAlignment,<br>
              FormatStyle::PAS_Right);<br>
  CHECK_PARSE("PointerBindsToType: Middle", PointerAlignment,<br>
              FormatStyle::PAS_Middle);<br></blockquote><div><br></div><div>I don't think there is much gain at this point, but if so, I'd like to have a function/macro like this:</div><div><br></div><div>CHECK_PARSE_ENUM(</div><div>    PointerAlignment,</div><div>    {"Left", "Middle", "Right"},</div><div>    {FormatStyle::PAS_Left, FormatStyle::PAS_Middle, FormatStyle::PAS_Right});</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Since CHECK_PARSE is also used in ParsesConfigurationWithLanguages, I would avoid rename it to CHECK_PARSE_ITEM_COMPATIBLE.<br></blockquote><div><br></div><div>I don't understand what you want to rename to what and why.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I'll work on addressing the issue this evening.<br>
<br>
<a href="http://reviews.llvm.org/D5346" target="_blank" class="cremed">http://reviews.llvm.org/D5346</a><br>
<br>
<br>
</blockquote></div><br></div></div>