clang-format: Replaced PointerBindsToType with PointerBinding to allow third kind of formatting
Daniel Jasper
djasper at google.com
Wed Oct 9 07:11:36 PDT 2013
On Tue, Oct 8, 2013 at 1:19 PM, Roman Himmes <roman at himmes.com> wrote:
> Hi,
>
> at work we have a project with a different style where the * or & is
> placed between the type and the variable. An example is: "const std::string
> & str". To make clang-format to recognize that kind of formatting I created
> a small patch, see below.
>
> I removed the old boolean setting "PointerBindsToType" and replaced it
> with a "PointerBinding" option with the three settings: Left, Middle and
> Right.
>
> Modified:
> include/clang/Format/Format.h
> lib/Format/Format.cpp
> unittests/Format/FormatTest.cpp
> lib/Format/TokenAnnotator.cpp
>
> What do you think of this patch?
>
- This would break everybody who already has a .clang-format file
somewhere. We really need to make changes to the configuration options in a
backwards compatible fashion if at all possible. We probably can change the
type of the configuration variable, but we need to adapt the config file
reader to still accept PointerBindsToType and set PointerBinding
appropriately.
- This would need a lot of additional tests to verify that we actually
format the different cases correctly (duplicate at least a few tests from
UnderstandsUsesOfStarAndAmp).
Index: include/clang/Format/Format.h
>
> ===================================================================
>
> --- include/clang/Format/Format.h (revision 191970)
>
> +++ include/clang/Format/Format.h (working copy)
>
> @@ -52,8 +52,18 @@
>
> /// \brief The penalty for breaking before the first \c <<.
>
> unsigned PenaltyBreakFirstLessLess;
>
> + /// \brief Different ways of placing pointer between type and variable
>
> + enum PointerBindingKind {
>
> + /// Put pointer or refernce at the type. Same as former setting 'PointerBindsToType: True'
>
> + PB_Left,
>
>
Typo. Also, there is no use in referencing previous version of the code.
> + /// Put pointer or reference in the middle of type and variable name like: const std::string & str
>
> + PB_Middle,
>
> + /// Put pointer or reference at the variable. Same as former setting 'PointerBindsToType: false'
>
> + PB_Right
>
> + };
>
> +
>
> /// \brief Set whether & and * bind to the type as opposed to the variable.
>
> - bool PointerBindsToType;
>
> + PointerBindingKind PointerBinding;
>
> /// \brief If \c true, analyze the formatted file for the most common binding.
>
> bool DerivePointerBinding;
>
> @@ -273,7 +283,7 @@
>
> PenaltyBreakString == R.PenaltyBreakString &&
>
> PenaltyExcessCharacter == R.PenaltyExcessCharacter &&
>
> PenaltyReturnTypeOnItsOwnLine == R.PenaltyReturnTypeOnItsOwnLine &&
>
> - PointerBindsToType == R.PointerBindsToType &&
>
> + PointerBinding == R.PointerBinding &&
>
> SpacesBeforeTrailingComments == R.SpacesBeforeTrailingComments &&
>
> Cpp11BracedListStyle == R.Cpp11BracedListStyle &&
>
> Standard == R.Standard && TabWidth == R.TabWidth &&
>
> Index: lib/Format/Format.cpp
>
> ===================================================================
>
> --- lib/Format/Format.cpp (revision 191970)
>
> +++ lib/Format/Format.cpp (working copy)
>
> @@ -34,6 +34,16 @@
>
> namespace llvm {
>
> namespace yaml {
>
> template <>
>
> +struct ScalarEnumerationTraits<clang::format::FormatStyle::PointerBindingKind> {
>
> + static void enumeration(IO &IO,
>
> + clang::format::FormatStyle::PointerBindingKind &Value) {
>
> + IO.enumCase(Value, "Left", clang::format::FormatStyle::PB_Left);
>
> + IO.enumCase(Value, "Middle", clang::format::FormatStyle::PB_Middle);
>
> + IO.enumCase(Value, "Right", clang::format::FormatStyle::PB_Right);
>
>
While this alignment might seem nice, I don't think it adds a lot of value.
I'd rather keep this file clang-format clean.
> + }
>
> +};
>
> +
>
> +template <>
>
> struct ScalarEnumerationTraits<clang::format::FormatStyle::LanguageStandard> {
>
> static void enumeration(IO &IO,
>
> clang::format::FormatStyle::LanguageStandard &Value) {
>
> @@ -144,7 +154,7 @@
>
> IO.mapOptional("PenaltyExcessCharacter", Style.PenaltyExcessCharacter);
>
> IO.mapOptional("PenaltyReturnTypeOnItsOwnLine",
>
> Style.PenaltyReturnTypeOnItsOwnLine);
>
> - IO.mapOptional("PointerBindsToType", Style.PointerBindsToType);
>
> + IO.mapOptional("PointerBinding", Style.PointerBinding);
>
> IO.mapOptional("SpacesBeforeTrailingComments",
>
> Style.SpacesBeforeTrailingComments);
>
> IO.mapOptional("Cpp11BracedListStyle", Style.Cpp11BracedListStyle);
>
> @@ -205,7 +215,7 @@
>
> LLVMStyle.MaxEmptyLinesToKeep = 1;
>
> LLVMStyle.NamespaceIndentation = FormatStyle::NI_None;
>
> LLVMStyle.ObjCSpaceBeforeProtocolList = true;
>
> - LLVMStyle.PointerBindsToType = false;
>
> + LLVMStyle.PointerBinding = FormatStyle::PB_Right;
>
> LLVMStyle.SpacesBeforeTrailingComments = 1;
>
> LLVMStyle.Standard = FormatStyle::LS_Cpp03;
>
> LLVMStyle.UseTab = FormatStyle::UT_Never;
>
> @@ -248,7 +258,7 @@
>
> GoogleStyle.MaxEmptyLinesToKeep = 1;
>
> GoogleStyle.NamespaceIndentation = FormatStyle::NI_None;
>
> GoogleStyle.ObjCSpaceBeforeProtocolList = false;
>
> - GoogleStyle.PointerBindsToType = true;
>
> + GoogleStyle.PointerBinding = FormatStyle::PB_Left;
>
> GoogleStyle.SpacesBeforeTrailingComments = 2;
>
> GoogleStyle.Standard = FormatStyle::LS_Auto;
>
> GoogleStyle.UseTab = FormatStyle::UT_Never;
>
> @@ -283,7 +293,7 @@
>
> MozillaStyle.IndentCaseLabels = true;
>
> MozillaStyle.ObjCSpaceBeforeProtocolList = false;
>
> MozillaStyle.PenaltyReturnTypeOnItsOwnLine = 200;
>
> - MozillaStyle.PointerBindsToType = true;
>
> + MozillaStyle.PointerBinding = FormatStyle::PB_Left;
>
> return MozillaStyle;
>
> }
>
> @@ -297,7 +307,7 @@
>
> Style.ColumnLimit = 0;
>
> Style.IndentWidth = 4;
>
> Style.NamespaceIndentation = FormatStyle::NI_Inner;
>
> - Style.PointerBindsToType = true;
>
> + Style.PointerBinding = FormatStyle::PB_Left;
>
> return Style;
>
> }
>
> @@ -980,9 +990,9 @@
>
> }
>
> if (Style.DerivePointerBinding) {
>
> if (CountBoundToType > CountBoundToVariable)
>
> - Style.PointerBindsToType = true;
>
> + Style.PointerBinding = FormatStyle::PB_Left;
>
> else if (CountBoundToType < CountBoundToVariable)
>
> - Style.PointerBindsToType = false;
>
> + Style.PointerBinding = FormatStyle::PB_Right;
>
>
Wouldn't we want to detect all three kinds of bindings.
> }
>
> if (Style.Standard == FormatStyle::LS_Auto) {
>
> Style.Standard = HasCpp03IncompatibleFormat ? FormatStyle::LS_Cpp11
>
> Index: lib/Format/TokenAnnotator.cpp
>
> ===================================================================
>
> --- lib/Format/TokenAnnotator.cpp (revision 191970)
>
> +++ lib/Format/TokenAnnotator.cpp (working copy)
>
> @@ -1229,14 +1229,14 @@
>
> if (Right.Type == TT_PointerOrReference)
>
> return Left.Tok.isLiteral() ||
>
> ((Left.Type != TT_PointerOrReference) && Left.isNot(tok::l_paren) &&
>
> - !Style.PointerBindsToType);
>
> + Style.PointerBinding != FormatStyle::PB_Left);
>
> if (Right.Type == TT_FunctionTypeLParen && Left.isNot(tok::l_paren) &&
>
> - (Left.Type != TT_PointerOrReference || Style.PointerBindsToType))
>
> + (Left.Type != TT_PointerOrReference || Style.PointerBinding != FormatStyle::PB_Right))
>
> return true;
>
> if (Left.Type == TT_PointerOrReference)
>
> return Right.Tok.isLiteral() || Right.Type == TT_BlockComment ||
>
> ((Right.Type != TT_PointerOrReference) &&
>
> - Right.isNot(tok::l_paren) && Style.PointerBindsToType &&
>
> + Right.isNot(tok::l_paren) && Style.PointerBinding != FormatStyle::PB_Right &&
>
> Left.Previous &&
>
> !Left.Previous->isOneOf(tok::l_paren, tok::coloncolon));
>
> if (Right.is(tok::star) && Left.is(tok::l_paren))
>
> Index: unittests/Format/FormatTest.cpp
>
> ===================================================================
>
> --- unittests/Format/FormatTest.cpp (revision 191970)
>
> +++ unittests/Format/FormatTest.cpp (working copy)
>
> @@ -3667,7 +3667,7 @@
>
> " aaaa, bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb);\n"
>
> "}");
>
> FormatStyle Style = getLLVMStyle();
>
> - Style.PointerBindsToType = true;
>
> + Style.PointerBinding = FormatStyle::PB_Left;
>
> verifyFormat("typedef bool* (Class::*Member)() const;", Style);
>
> }
>
> @@ -3890,7 +3890,7 @@
>
> verifyGoogleFormat("T** t = new T*();");
>
> FormatStyle PointerLeft = getLLVMStyle();
>
> - PointerLeft.PointerBindsToType = true;
>
> + PointerLeft.PointerBinding = FormatStyle::PB_Left;
>
> verifyFormat("delete *x;", PointerLeft);
>
> }
>
> @@ -3904,7 +3904,7 @@
>
> verifyFormat("template <class... Ts> void Foo(Ts *... ts) {}");
>
> FormatStyle PointersLeft = getLLVMStyle();
>
> - PointersLeft.PointerBindsToType = true;
>
> + PointersLeft.PointerBinding = FormatStyle::PB_Left;
>
> verifyFormat("template <class... Ts> void Foo(Ts*... ts) {}", PointersLeft);
>
> }
>
> @@ -6359,7 +6359,6 @@
>
> CHECK_PARSE_BOOL(DerivePointerBinding);
>
> CHECK_PARSE_BOOL(IndentCaseLabels);
>
> CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);
>
> - CHECK_PARSE_BOOL(PointerBindsToType);
>
> CHECK_PARSE_BOOL(Cpp11BracedListStyle);
>
> CHECK_PARSE_BOOL(IndentFunctionDeclarationAfterType);
>
> CHECK_PARSE_BOOL(SpacesInParentheses);
>
> @@ -6380,6 +6379,12 @@
>
> SpacesBeforeTrailingComments, 1234u);
>
> CHECK_PARSE("IndentWidth: 32", IndentWidth, 32u);
>
> +
>
> + Style.PointerBinding = FormatStyle::PB_Left;
>
> + CHECK_PARSE("PointerBinding: Left", PointerBinding, FormatStyle::PB_Left);
>
> + CHECK_PARSE("PointerBinding: Middle", PointerBinding, FormatStyle::PB_Middle);
>
> + CHECK_PARSE("PointerBinding: Right", PointerBinding, FormatStyle::PB_Right);
>
>
Same as above.
> +
>
> Style.Standard = FormatStyle::LS_Auto;
>
> CHECK_PARSE("Standard: Cpp03", Standard, FormatStyle::LS_Cpp03);
>
> CHECK_PARSE("Standard: Cpp11", Standard, FormatStyle::LS_Cpp11);
>
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131009/c1fa29b5/attachment.html>
More information about the cfe-commits
mailing list