On Tue, Oct 8, 2013 at 1:19 PM, Roman Himmes <[email protected]> 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
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
