[clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine

[Manikishan]

Summary

This new Style rule is made as a part of adding support for NetBSD KNF in clang-format. This style Lines up BitField Declarations on consecutive lines with correct Indentation. The working of this Style rule shown below:

//Configuration
BitFieldDeclarationsOnePerLine: true

//Before Formatting:

unsigned int bas :3,  hh : 4, jjj : 8;


unsigned int baz:1,
                fuz:5, 
                zap:2;

//After Formatting:

unsigned int bas : 3,
             hh : 4,
             jjj : 8;

unsigned int baz : 1,
             fuz : 5,
             zap : 2;

This style is formatted even if the one-line declaration line is less than the column limit.

There is a minor Bug:
Comments after the bitfield in before the break line will not cause a proper indentation.

//Before Formatting:

unsigned int baz:1,     /* foo*/
             fuz:5,          /*bar*/
             zap:2;

//After Formatting:

unsigned int  baz : 1, /* foo*/
        fuz : 5,                 /*bar*/
        zap : 2;

[MyDeveloperDay]

You need to add a unit test in clang/unittests

docs/ClangFormatStyleOptions.rst

194

Nit: Aligns Bitfield Declarations

I think Align is a better work than Linesup

196

align

197

end with :

lib/Format/TokenAnnotator.cpp

2920

elide the {}

[mgorny]

docs/ClangFormatStyleOptions.rst

194

'separate'

include/clang/Format/Format.h

104

Also update this to match docs.

lib/Format/ContinuationIndenter.cpp

281

Space after 'if' and before '{'. Also below.

lib/Format/TokenAnnotator.cpp

2921

Misindent.

[Manikishan]

Added unittests and made the changes suggested by @mgorny

[Manikishan]

Made some missing style modifications in the last revision

[Manikishan]

Updated unittest

[MyDeveloperDay]

docs/ClangFormatStyleOptions.rst

193

Isn't the documentation normally alphabetric? shouldn't it be after AlignConsecutiveAssignments

include/clang/Format/Format.h

104

Align

112

I think this should be alphabetic in this file (not sure though check the rest of it)

are you happy with the name? BitFieldDeclsOnSeparateLines?

1. people often spell Separate incorrectly (didn't you?), this could lead to misconfigured
2. isn't this really a ''Break'' rule

I want to say this might better as something like "BreakAfterBitFieldDecl"

lib/Format/ContinuationIndenter.cpp

281

something here doesn't feel quite right,, without trying the code change myself I cannot tell, did you ever try this code without having the same clause in canBreak() and mustBreak()? (i.e. just put it in mustBreak)

The reason I ask is I'm unclear as to why the other mustBreak() rules aren't here in canBreak() if thats the case

lib/Format/TokenAnnotator.cpp

2921

This code appears 3 times (does it need to appear 3 times?), do we need some sort of

bool isBitField(FormatToken)
{
    ...
}

Should a bit field check for the existence of a number after the colon? I can't think of other C++ constructs that appear as

comma identifier colon

but given that clang-format is used for ObjC,ProtoBuf,Java,JavaScript,C# I'm pretty sure something odd is going to happen with JavaScript named parameters, to be honest I think this is going to cause the following to get reformatted

MyFunctionCall({ xPosition: 20, yPosition: 50, width: 100, height: 5, drawingNow: true });

MyFunctionCall({ xPosition: 20**, yPosition: 50,**
                            width: 100, 
                            height: 5, 
                            drawingNow: true });

or something like that

unittests/Format/FormatTest.cpp

3671

please add a test with comments (it will get logged)

unsigned int baz : 11, /*motor control flags*/
                     add: 2    /* control code for turning the lights on */ ,
                     foo: 3     /* (unused */

[Manikishan]

Changed Style name to OnePerLineBitFieldDecl

[MyDeveloperDay]

mark sure you mark off the comments as you consider them done.

lib/Format/FormatToken.h

519

this looks like it needs a clang-format as its not aligned with function that follows

527

couldn't you write this as just and lose the `if`

return (T->Tok.is(tok::comma) && Tok.is(tok::identifier) && T->Next->Tok.is(tok::colon));

[MyDeveloperDay]

Nit: if you going for "OnePerLine" in the name rather than "Break" can you keep it consistent with ConstructorInitializerAllOnOneLineOrOnePerLine and put the type first and then the operation i.e. BitFieldOnePerLine (this will help keep all BitField options together if there are ever more than one), I also think either don't use Decl at all or use the full word Declarations as in AlignConsecutiveDeclarations , AlwaysBreakTemplateDeclarations etc

[MyDeveloperDay]

lib/Format/FormatToken.h

524

do you think you might need

(T && T->Tok.is(tok::comma)......

lib/Format/TokenAnnotator.cpp

2920

can't this use Right.isBitField()

[MyDeveloperDay]

unittests/Format/FormatTest.cpp

3671

any thoughts on these comments?

[Manikishan]

unittests/Format/FormatTest.cpp

3671

Sorry, I totally have to keep this aside because of some reasons, will update within a day.

[Manikishan]

lib/Format/TokenAnnotator.cpp

2921

Yes, you are correct this patch is failing many Javascript Tests. Will refactor the implementation.

[Manikishan]

lib/Format/FormatToken.h

524

I have found a method FieldDecl::isBitField() which check whether a current field is a BitField, But FieldDecl is not used anywhere in lib/Format/ . I think it may not be related to Formatting and so I have to modify my declaration of isBitield() in FormatToken.h.
Can I get suggestions whether my understanding is correct?

[MyDeveloperDay]

lib/Format/FormatToken.h

524

FieldDecl will be part of the AST tree, but clang-format doesn't use that (or it would need compiler arguments etc..)

Take a look at TokenAnnotator.cpp... (and try running clang-format with the -debug option), it may show you that the bitfield can be identified by the TT_BitFieldColon type rather than by a colon directly.

} else if (CurrentToken && CurrentToken->is(tok::numeric_constant)) {
  Tok->Type = TT_BitFieldColon;

[Manikishan]

lib/Format/FormatToken.h

524

Sure will have a look at it, and figure out how can I check for bitfields

[MyDeveloperDay]

lib/Format/Format.cpp

455

Alphabetical

docs/ClangFormatStyleOptions.rst

* ``BAS_AlwaysBreak`` (in configuration: ``AlwaysBreak``)
     Always break after an open bracket, if the parameters don't fit
     on a single line, e.g.:
 
     .. code-block:: c++
 
       someLongFunction(
           argument1, argument2);
 
-
-
 **AlignConsecutiveAssignments** (``bool``)
   If ``true``, aligns consecutive assignments.
 
   This will align the assignment operators of consecutive lines. This
   will result in formattings like
 
   .. code-block:: c++
 
     int aaaa = 12;
     int b    = 23;
     int ccc  = 23;
 
 **AlignConsecutiveDeclarations** (``bool``)
.. code-block:: c++
     void f(int aaaaaaaaaaaaaaaaaaaa, int aaaaaaaaaaaaaaaaaaaa,
            int aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {}
 
     false:
     void f(int aaaaaaaaaaaaaaaaaaaa,
            int aaaaaaaaaaaaaaaaaaaa,
            int aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {}
 
+**BitFieldDeclarationsOnePerLine** (``bool``)
+  If ``true``, Align Bitfield Declarations on separate lines.
+
+  This will align Bitfield declarations on consecutive lines. This
+  will result in formatting like:
+
+  .. code-block:: c++
+
+    unsigned int  baz : 1,
+                  fuz : 5,
+                  zap : 2;
+
 **BraceWrapping** (``BraceWrappingFlags``)
   Control of individual brace wrapping cases.
 
   If ``BreakBeforeBraces`` is set to ``BS_Custom``, use this to specify how
   each individual brace case should be handled. Otherwise, this is ignored.
 
   .. code-block:: yaml
 

include/clang/Format/Format.h

struct FormatStyle {
   /// will result in formattings like
   /// \code
   ///   int         aaaa = 12;
   ///   float       b = 23;
   ///   std::string ccc = 23;
   /// \endcode
   bool AlignConsecutiveDeclarations;
 
+  /// If ``true``, Linesup Bitfield Declarations.
+  /// This will lineup Bitfield declarations on consecutive lines. This
+  /// will result in formatting like
+  /// \code
+  ///  unsigned int  baz : 1, /* Bitfield; line up entries if desire*/
+  ///    fuz : 5,
+  ///    zap : 2;
+  /// \endcode
+  bool BitFieldDeclarationsOnePerLine;
+
   /// Different styles for aligning escaped newlines.
   enum EscapedNewlineAlignmentStyle {
     /// Don't align escaped newlines.
     /// \code
     ///   #define A \
     ///     int aaaa; \
     ///     int b; \
     ///     int dddddddddd;
return AccessModifierOffset == R.AccessModifierOffset &&
            AllowShortLoopsOnASingleLine == R.AllowShortLoopsOnASingleLine &&
            AlwaysBreakAfterReturnType == R.AlwaysBreakAfterReturnType &&
            AlwaysBreakBeforeMultilineStrings ==
                R.AlwaysBreakBeforeMultilineStrings &&
            AlwaysBreakTemplateDeclarations ==
                R.AlwaysBreakTemplateDeclarations &&
            BinPackArguments == R.BinPackArguments &&
            BinPackParameters == R.BinPackParameters &&
+           BitFieldDeclarationsOnePerLine == R.BitFieldDeclarationsOnePerLine &&
            BreakBeforeBinaryOperators == R.BreakBeforeBinaryOperators &&
            BreakBeforeBraces == R.BreakBeforeBraces &&
            BreakBeforeTernaryOperators == R.BreakBeforeTernaryOperators &&
            BreakConstructorInitializers == R.BreakConstructorInitializers &&
            CompactNamespaces == R.CompactNamespaces &&
            BreakAfterJavaFieldAnnotations == R.BreakAfterJavaFieldAnnotations &&
            BreakStringLiterals == R.BreakStringLiterals &&
            ColumnLimit == R.ColumnLimit && CommentPragmas == R.CommentPragmas &&

lib/Format/ContinuationIndenter.cpp

LineState ContinuationIndenter::getInitialState(unsigned FirstIndent,
   moveStateToNextToken(State, DryRun, /*Newline=*/false);
   return State;
 }
 
 bool ContinuationIndenter::canBreak(const LineState &State) {
   const FormatToken &Current = *State.NextToken;
   const FormatToken &Previous = *Current.Previous;
   assert(&Previous == Current.Previous);
   if (!Current.CanBreakBefore && !(State.Stack.back().BreakBeforeClosingBrace &&
                                    Current.closesBlockOrBlockTypeList(Style)))
     return false;
   // The opening "{" of a braced list has to be on the same line as the first
   // element if it is nested in another braced init list or function call.
   if (!Current.MustBreakBefore && Previous.is(tok::l_brace) &&
       Previous.isNot(TT_DictLiteral) && Previous.BlockKind == BK_BracedInit &&
       Previous.Previous &&
       Previous.Previous->isOneOf(tok::l_brace, tok::l_paren, tok::comma))
if (Previous.is(tok::l_square) && Previous.is(TT_ObjCMethodExpr))
     return false;
 
   return !State.Stack.back().NoLineBreak;
 }
 
 bool ContinuationIndenter::mustBreak(const LineState &State) {
   const FormatToken &Current = *State.NextToken;
   const FormatToken &Previous = *Current.Previous;
+  if (Style.isCpp() && Current.isBitField() && Style.BitFieldDeclarationsOnePerLine)
+    return true;     
   if (Current.MustBreakBefore || Current.is(TT_InlineASMColon))
     return true;
   if (State.Stack.back().BreakBeforeClosingBrace &&
       Current.closesBlockOrBlockTypeList(Style))
     return true;
   if (Previous.is(tok::semi) && State.LineContainsContinuedForLoopSection)
     return true;
   if (Style.Language == FormatStyle::LK_ObjC &&
unsigned ContinuationIndenter::addTokenToState(LineState &State, bool Newline,
 
   return moveStateToNextToken(State, DryRun, Newline) + Penalty;
 }
 
 void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
                                                  unsigned ExtraSpaces) {
   FormatToken &Current = *State.NextToken;
   const FormatToken &Previous = *State.NextToken->Previous;
-  if (Current.is(tok::equal) &&
+  if (Current.isOneOf(tok::equal, tok::colon) &&
       (State.Line->First->is(tok::kw_for) || Current.NestingLevel == 0) &&
       State.Stack.back().VariablePos == 0) {
     State.Stack.back().VariablePos = State.Column;
     // Move over * and & if they are bound to the variable name.
     const FormatToken *Tok = &Previous;
     while (Tok && State.Stack.back().VariablePos >= Tok->ColumnWidth) {
       State.Stack.back().VariablePos -= Tok->ColumnWidth;
       if (Tok->SpacesRequiredBefore != 0)

lib/Format/Format.cpp

static void mapping(IO &IO, FormatStyle &Style) {
     IO.mapOptional("IndentWidth", Style.IndentWidth);
     IO.mapOptional("IndentWrappedFunctionNames",
                    Style.IndentWrappedFunctionNames);
     IO.mapOptional("JavaImportGroups", Style.JavaImportGroups);
     IO.mapOptional("JavaScriptQuotes", Style.JavaScriptQuotes);
     IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
     IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
                    Style.KeepEmptyLinesAtTheStartOfBlocks);
+    IO.mapOptional("BitFieldDeclarationsOnePerLine", Style.BitFieldDeclarationsOnePerLine);
     IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
     IO.mapOptional("MacroBlockEnd", Style.MacroBlockEnd);
     IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep);
     IO.mapOptional("NamespaceIndentation", Style.NamespaceIndentation);
     IO.mapOptional("NamespaceMacros", Style.NamespaceMacros);
     IO.mapOptional("ObjCBinPackProtocolList", Style.ObjCBinPackProtocolList);
     IO.mapOptional("ObjCBlockIndentWidth", Style.ObjCBlockIndentWidth);
     IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty);
     IO.mapOptional("ObjCSpaceBeforeProtocolList",

lib/Format/FormatToken.h

bool isCppStructuredBinding(const FormatStyle &Style) const {
     const FormatToken *T = this;
     do {
       T = T->getPreviousNonComment();
     } while (T && T->isOneOf(tok::kw_const, tok::kw_volatile, tok::amp,
                              tok::ampamp));
     return T && T->is(tok::kw_auto);
   }
 
+  /// Returns whether the token is a Bit field, and checks whether
+  /// the Style is C / C++.
+  bool isBitField() const {
+    const FormatToken *T = this;
+    T = T->getPreviousNonComment();
+    return (T->Tok.is(tok::comma) && Tok.is(tok::identifier) &&
+            T->Next->Tok.is(tok::colon));
+  }
   /// Same as opensBlockOrBlockTypeList, but for the closing token.
   bool closesBlockOrBlockTypeList(const FormatStyle &Style) const {
     if (is(TT_TemplateString) && closesScope())
       return true;
     return MatchingParen && MatchingParen->opensBlockOrBlockTypeList(Style);
   }
 
   /// Return the actual namespace token, if this token starts a namespace
   /// block.

lib/Format/TokenAnnotator.cpp

 static bool isAllmanBrace(const FormatToken &Tok) {
   return Tok.is(tok::l_brace) && Tok.BlockKind == BK_Block &&
          !Tok.isOneOf(TT_ObjCBlockLBrace, TT_LambdaLBrace, TT_DictLiteral);
 }
 
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
                                      const FormatToken &Right) {
   const FormatToken &Left = *Right.Previous;
+  if (Right.Previous->is(tok::comma) && Style.BitFieldDeclarationsOnePerLine &&
+      Right.is(tok::identifier) && (Right.Next->is(tok::colon)))
+    return true;
   if (Right.NewlinesBefore > 1 && Style.MaxEmptyLinesToKeep > 0)
     return true;
 
   if (Style.Language == FormatStyle::LK_JavaScript) {
     // FIXME: This might apply to other languages and token kinds.
     if (Right.is(tok::string_literal) && Left.is(tok::plus) && Left.Previous &&
         Left.Previous->is(tok::string_literal))
       return true;

unittests/Format/FormatTest.cpp

verifyFormat(
       "    aaaaaaaaaaaaaaaaaaaaaaaa<\n"
       "        aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa>,\n"
       "    aaaaaaaaaaaaaaaaaaaaaaaa>;");
   verifyFormat("int a = bbbb && ccc &&\n"
                "        fffff(\n"
                "#define A Just forcing a new line\n"
                "            ddd);");
 }
+TEST_F(FormatTest, AlignBitFieldDeclarationsOnConsecutiveLines){
+  FormatStyle Style = {};
+  Style.BitFieldDeclarationsOnePerLine = true;
+  verifyFormat(
+    "unsigned int baz : 11,
+                  aaa : 2,
+                  foo : 3"
+  );
+  Style.BitFieldDeclarationsOnePerLine = false;
+  verifyFormat(
+    "unsigned int baz : 11, aaa : 2, foo : 3"
+  );
+} 
 
 TEST_F(FormatTest, LineBreakingInBinaryExpressions) {
   verifyFormat(
       "bool aaaaaaa =\n"
       "    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaa).aaaaaaaaaaaaaaaaaaa() ||\n"
       "    bbbbbbbb();");
   verifyFormat(
       "bool aaaaaaa =\n"