clang-format: Incorrect behavior with BreakBinaryOperations

[andergnet]

Thanks for adding BreakBinaryOperations! It's a great feature.

I notice some strange behavior when dealing with the operator>>:

I'm using BreakBinaryOperations: RespectPrecedence

Before formatting:

void abc()
{
   int aaaaaa, bbbbbb, cccccc;
   aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc
      >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc;
}
void abc()
{
   int aaaaaa, bbbbbb, cccccc;
   aaaaaa << bbbbbb << cccccc << aaaaaa << bbbbbb << cccccc << aaaaaa << bbbbbb << cccccc << aaaaaa << bbbbbb << cccccc
      << aaaaaa << bbbbbb << cccccc << aaaaaa << bbbbbb << cccccc;
}

After formatting:

void abc()
{
   int aaaaaa, bbbbbb, cccccc;
   aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc;
}
void abc()
{
   int aaaaaa, bbbbbb, cccccc;
   aaaaaa
      << bbbbbb
      << cccccc
      << aaaaaa
      << bbbbbb
      << cccccc
      << aaaaaa
      << bbbbbb
      << cccccc
      << aaaaaa
      << bbbbbb
      << cccccc
      << aaaaaa
      << bbbbbb
      << cccccc
      << aaaaaa
      << bbbbbb
      << cccccc;
}

operator<< works as expected, operator>> does not.

[andergnet]

Here are all the rest of the .clang-format file:

# Common settings
DisableFormat: true
---
Language: Cpp
# BasedOnStyle:  LLVM
AccessModifierOffset: 0
AlignAfterOpenBracket: Align
AlignArrayOfStructures: Left
AlignConsecutiveAssignments:
  Enabled:         false
  AcrossEmptyLines: false
  AcrossComments:  false
  AlignCompound:   false
  AlignFunctionPointers: false
  PadOperators:    true
AlignConsecutiveBitFields:
  Enabled:         false
  AcrossEmptyLines: false
  AcrossComments:  false
  AlignCompound:   false
  AlignFunctionPointers: false
  PadOperators:    false
AlignConsecutiveDeclarations:
  Enabled:         false
  AcrossEmptyLines: false
  AcrossComments:  false
  AlignCompound:   false
  AlignFunctionPointers: false
  PadOperators:    false
AlignConsecutiveMacros:
  Enabled:         false
  AcrossEmptyLines: false
  AcrossComments:  false
  AlignCompound:   false
  AlignFunctionPointers: false
  PadOperators:    false
AlignConsecutiveShortCaseStatements:
  Enabled:         true
  AcrossEmptyLines: true
  AcrossComments:  true
  AlignCaseArrows: false
  AlignCaseColons: false
AlignConsecutiveTableGenBreakingDAGArgColons:
  Enabled:         false
  AcrossEmptyLines: false
  AcrossComments:  false
  AlignCompound:   false
  AlignFunctionPointers: false
  PadOperators:    false
AlignConsecutiveTableGenCondOperatorColons:
  Enabled:         false
  AcrossEmptyLines: false
  AcrossComments:  false
  AlignCompound:   false
  AlignFunctionPointers: false
  PadOperators:    false
AlignConsecutiveTableGenDefinitionColons:
  Enabled:         false
  AcrossEmptyLines: false
  AcrossComments:  false
  AlignCompound:   false
  AlignFunctionPointers: false
  PadOperators:    false
AlignEscapedNewlines: Right
AlignOperands:   AlignAfterOperator
AlignTrailingComments:
  Kind:            Always
  OverEmptyLines:  0
AllowAllArgumentsOnNextLine: true
AllowAllParametersOfDeclarationOnNextLine: true
AllowBreakBeforeNoexceptSpecifier: Never
AllowShortBlocksOnASingleLine: Never
AllowShortCaseExpressionOnASingleLine: true
AllowShortCaseLabelsOnASingleLine: false
AllowShortCompoundRequirementOnASingleLine: true
AllowShortEnumsOnASingleLine: true
AllowShortFunctionsOnASingleLine: None
AllowShortIfStatementsOnASingleLine: Never
AllowShortLambdasOnASingleLine: All
AllowShortLoopsOnASingleLine: false
AlwaysBreakAfterDefinitionReturnType: None
AlwaysBreakBeforeMultilineStrings: false
AttributeMacros:
  - __capability
BinPackArguments: true
BinPackParameters: true
BitFieldColonSpacing: Both
BraceWrapping:
  AfterCaseLabel:  true
  AfterClass:      true
  AfterControlStatement: Always
  AfterEnum:       false
  AfterExternBlock: true
  AfterFunction:   true
  AfterNamespace:  true
  AfterObjCDeclaration: false
  AfterStruct:     true
  AfterUnion:      true
  BeforeCatch:     true
  BeforeElse:      true
  BeforeLambdaBody: false
  BeforeWhile:     false
  IndentBraces:    false
  SplitEmptyFunction: false
  SplitEmptyRecord: true
  SplitEmptyNamespace: true
BreakAdjacentStringLiterals: true
BreakAfterAttributes: Leave
BreakAfterJavaFieldAnnotations: false
BreakAfterReturnType: None
BreakArrays:     true
BreakBeforeBinaryOperators: NonAssignment
BreakBeforeConceptDeclarations: Always
BreakBeforeBraces: Custom
BreakBeforeInlineASMColon: OnlyMultiline
BreakBeforeTernaryOperators: false
BreakBinaryOperations: RespectPrecedence
BreakConstructorInitializers: AfterColon
BreakFunctionDefinitionParameters: false
BreakInheritanceList: AfterColon
BreakStringLiterals: true
BreakTemplateDeclarations: MultiLine
ColumnLimit:    120
CommentPragmas:  '^ IWYU pragma:'
CompactNamespaces: false
ConstructorInitializerIndentWidth: 3
ContinuationIndentWidth: 3
Cpp11BracedListStyle: true
DerivePointerAlignment: false
DisableFormat:   false
EmptyLineAfterAccessModifier: Never
EmptyLineBeforeAccessModifier: LogicalBlock
ExperimentalAutoDetectBinPacking: false
FixNamespaceComments: true
ForEachMacros:
  - foreach
  - Q_FOREACH
  - BOOST_FOREACH
IfMacros:
  - KJ_IF_MAYBE
IncludeBlocks:   Preserve
IncludeCategories:
  - Regex:           '^"(llvm|llvm-c|clang|clang-c)/'
    Priority:        2
    SortPriority:    0
    CaseSensitive:   false
  - Regex:           '^(<|"(gtest|gmock|isl|json)/)'
    Priority:        3
    SortPriority:    0
    CaseSensitive:   false
  - Regex:           '.*'
    Priority:        1
    SortPriority:    0
    CaseSensitive:   false
IncludeIsMainRegex: '(Test)?$'
IncludeIsMainSourceRegex: ''
IndentAccessModifiers: false
IndentCaseBlocks: false
IndentCaseLabels: true
IndentExternBlock: AfterExternBlock
IndentGotoLabels: true
IndentPPDirectives: None
IndentRequiresClause: true
IndentWidth:     3
IndentWrappedFunctionNames: false
InsertBraces:    false
InsertNewlineAtEOF: true
InsertTrailingCommas: None
IntegerLiteralSeparator:
  Binary:          0
  BinaryMinDigits: 0
  Decimal:         0
  DecimalMinDigits: 0
  Hex:             0
  HexMinDigits:    0
JavaScriptQuotes: Leave
JavaScriptWrapImports: true
KeepEmptyLines:
  AtEndOfFile:     false
  AtStartOfBlock:  false
  AtStartOfFile:   false
LambdaBodyIndentation: Signature
LineEnding:      DeriveLF
MacroBlockBegin: ''
MacroBlockEnd:   ''
MainIncludeChar: Quote
MaxEmptyLinesToKeep: 1
NamespaceIndentation: None
ObjCBinPackProtocolList: Auto
ObjCBlockIndentWidth: 2
ObjCBreakBeforeNestedBlockParam: true
ObjCSpaceAfterProperty: false
ObjCSpaceBeforeProtocolList: true
PackConstructorInitializers: NextLineOnly
PenaltyBreakAssignment: 0
PenaltyBreakBeforeFirstCallParameter: 100
PenaltyBreakComment: 1000000
PenaltyBreakFirstLessLess: 0
PenaltyBreakOpenParenthesis: 2000
PenaltyBreakScopeResolution: 5000
PenaltyBreakString: 1000
PenaltyBreakTemplateDeclaration: 10
PenaltyExcessCharacter: 10
PenaltyIndentedWhitespace: 10
PenaltyReturnTypeOnItsOwnLine: 1000
PointerAlignment: Left
PPIndentWidth:   -1
QualifierAlignment: Leave
ReferenceAlignment: Pointer
ReflowComments:  false
RemoveBracesLLVM: false
RemoveParentheses: Leave
RemoveSemicolon: false
RequiresClausePosition: OwnLine
RequiresExpressionIndentation: OuterScope
SeparateDefinitionBlocks: Leave
ShortNamespaceLines: 1
SortIncludes:    CaseInsensitive
SortJavaStaticImport: Before
SortUsingDeclarations: LexicographicNumeric
SpaceAfterCStyleCast: false
SpaceAfterLogicalNot: false
SpaceAfterTemplateKeyword: true
SpaceAroundPointerQualifiers: Default
SpaceBeforeAssignmentOperators: true
SpaceBeforeCaseColon: false
SpaceBeforeCpp11BracedList: false
SpaceBeforeCtorInitializerColon: true
SpaceBeforeInheritanceColon: false
SpaceBeforeJsonColon: false
SpaceBeforeParens: ControlStatements
SpaceBeforeParensOptions:
  AfterControlStatements: true
  AfterForeachMacros: true
  AfterFunctionDefinitionName: false
  AfterFunctionDeclarationName: false
  AfterIfMacros:   true
  AfterOverloadedOperator: false
  AfterPlacementOperator: false
  AfterRequiresInClause: false
  AfterRequiresInExpression: false
  BeforeNonEmptyParentheses: false
SpaceBeforeRangeBasedForLoopColon: true
SpaceBeforeSquareBrackets: false
SpaceInEmptyBlock: false
SpaceInEmptyParentheses: false
SpacesBeforeTrailingComments: 1
SpacesInAngles:  Never
SpacesInConditionalStatement: false
SpacesInContainerLiterals: true
SpacesInCStyleCastParentheses: false
SpacesInLineCommentPrefix:
  Minimum:         1
  Maximum:         -1
SpacesInParens:  Never
SpacesInParensOptions:
  ExceptDoubleParentheses: false
  InCStyleCasts:   false
  InConditionalStatements: false
  InEmptyParentheses: false
  Other:           false
SpacesInSquareBrackets: false
Standard:        c++17
StatementAttributeLikeMacros:
  - Q_EMIT
StatementMacros:
  - Q_UNUSED
  - QT_REQUIRE_VERSION
TableGenBreakInsideDAGArg: DontBreak
TabWidth:        3
UseTab:          Never
VerilogBreakBetweenInstancePorts: true
WhitespaceSensitiveMacros:
  - BOOST_PP_STRINGIZE
  - CF_SWIFT_NAME
  - NS_SWIFT_NAME
  - PP_STRINGIZE
  - STRINGIZE

[llvmbot]

@llvm/issue-subscribers-clang-format

Author: Ander (andergnet)

Thanks for adding BreakBinaryOperations! It's a great feature.

I notice some strange behavior when dealing with the operator>>:

I'm using BreakBinaryOperations: RespectPrecedence

Before formatting:

void abc()
{
   int aaaaaa, bbbbbb, cccccc;
   aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc
      >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc;
}
void abc()
{
   int aaaaaa, bbbbbb, cccccc;
   aaaaaa << bbbbbb << cccccc << aaaaaa << bbbbbb << cccccc << aaaaaa << bbbbbb << cccccc << aaaaaa << bbbbbb << cccccc
      << aaaaaa << bbbbbb << cccccc << aaaaaa << bbbbbb << cccccc;
}

After formatting:

void abc()
{
   int aaaaaa, bbbbbb, cccccc;
   aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc;
}
void abc()
{
   int aaaaaa, bbbbbb, cccccc;
   aaaaaa
      << bbbbbb
      << cccccc
      << aaaaaa
      << bbbbbb
      << cccccc
      << aaaaaa
      << bbbbbb
      << cccccc
      << aaaaaa
      << bbbbbb
      << cccccc
      << aaaaaa
      << bbbbbb
      << cccccc
      << aaaaaa
      << bbbbbb
      << cccccc;
}

operator<< works as expected, operator>> does not.

[andergnet]

I forgot to mention, the clang-format version is this one (compiled from sources):

clang-format version 20.0.0git (https://github.com/llvm/llvm-project.git b4feb26606de84ff53d9b65a3b79c00a2b4d7c22)

[d3x0r]

I've been digging through the code for my own reasons; I ran across this...

https://github.com/llvm/llvm-project/blob/main/clang/lib/Format/ContinuationIndenter.cpp#L924C1-L927C37

  // Don't allow the RHS of an operator to be split over multiple lines unless
  // there is a line-break right after the operator.
  // Exclude relational operators, as there, it is always more desirable to
  // have the LHS 'left' of the RHS.

followed by some specific handling of P->MustBreakBefore || P->is(tok::lessless) ||

  // Breaking before the first "<<" is generally not desirable if the LHS is
  // short. Also always add the penalty if the LHS is split over multiple lines
  // to avoid unnecessary line breaks that just work around this penalty.

https://github.com/llvm/llvm-project/blob/main/clang/lib/Format/ContinuationIndenter.cpp#L1028

[andergnet]

Hello,
I have been looking into the code you @d3x0r pointed out but it turned out not to be relevant. The case when it's failing its related to ">>" not "<<" and the mentioned code references tok::lessless.

On the other hand that got me into debugging the issue and I noticed a relevant difference in how both operators are handled. When the code of ContinuationIndenter is executed, the operator "<<" comes as tok:lessess, but the operator ">>" comes as two tokens of type tok:greater.

That got me to this piece of code:

llvm-project/clang/lib/Format/FormatTokenLexer.cpp

Lines 536 to 558 in cbe583b

	bool FormatTokenLexer::tryMergeGreaterGreater() {
	// Merge kw_operator,greater,greater into kw_operator,greatergreater.
	if (Tokens.size() < 2)
	return false;
	auto First = Tokens.end() - 2;
	if (First[0]->isNot(tok::greater) || First[1]->isNot(tok::greater))
	return false;
	// Only merge if there currently is no whitespace between the first two ">".
	if (First[1]->hasWhitespaceBefore())
	return false;
	auto Tok = Tokens.size() > 2 ? First[-1] : nullptr;
	if (Tok && Tok->isNot(tok::kw_operator))
	return false;
	First[0]->Tok.setKind(tok::greatergreater);
	First[0]->TokenText = ">>";
	First[0]->ColumnWidth += 1;
	Tokens.erase(Tokens.end() - 1);
	return true;
	}

If I disable this condition:

  auto Tok = Tokens.size() > 2 ? First[-1] : nullptr;
  if (Tok && Tok->isNot(tok::kw_operator))
    return false;

the formatting works as expected. I have seen that this code was last modified by @owenca, maybe he can suggest a way to fix it because I do not know how to proceed to fix this without breaking something else.

Thank you.

[owenca]

On the other hand that got me into debugging the issue and I noticed a relevant difference in how both operators are handled. When the code of ContinuationIndenter is executed, the operator "<<" comes as tok:lessess, but the operator ">>" comes as two tokens of type tok:greater.

That got me to this piece of code:

llvm-project/clang/lib/Format/FormatTokenLexer.cpp

Lines 536 to 558 in cbe583b

bool FormatTokenLexer::tryMergeGreaterGreater() {
// Merge kw_operator,greater,greater into kw_operator,greatergreater.
if (Tokens.size() < 2)
return false;

auto First = Tokens.end() - 2;
if (First[0]->isNot(tok::greater) || First[1]->isNot(tok::greater))
return false;

// Only merge if there currently is no whitespace between the first two ">".
if (First[1]->hasWhitespaceBefore())
return false;

auto Tok = Tokens.size() > 2 ? First[-1] : nullptr;
if (Tok && Tok->isNot(tok::kw_operator))
return false;

First[0]->Tok.setKind(tok::greatergreater);
First[0]->TokenText = ">>";
First[0]->ColumnWidth += 1;
Tokens.erase(Tokens.end() - 1);
return true;
}
If I disable this condition:

  auto Tok = Tokens.size() > 2 ? First[-1] : nullptr;
  if (Tok && Tok->isNot(tok::kw_operator))
    return false;

the formatting works as expected. I have seen that this code was last modified by @owenca, maybe he can suggest a way to fix it because I do not know how to proceed to fix this without breaking something else.

IIRC clang-format has always split a >> token into two > tokens and never merged them back until I added that function, which only merges them if they are preceded by an operator token. So the issue must be elsewhere.

[andergnet]

Still investigating, my current progress is the following:

If I modify this line and set the varaible to true instead of false, which by the name of the function and the variable makes sense:

llvm-project/clang/lib/Format/ContinuationIndenter.cpp

Line 904 in 9ab16d4

CurrentState.NoLineBreak = true;

I get wrong behavior but at least I get it consistent in both cases:

void abc()
{
   int aaaaaa, bbbbbb, cccccc;
   aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc
      >> aaaaaa
      >> bbbbbb
      >> cccccc
      >> aaaaaa
      >> bbbbbb
      >> cccccc;
}

void abc()
{
   int aaaaaa, bbbbbb, cccccc;
   aaaaaa << bbbbbb << cccccc << aaaaaa << bbbbbb << cccccc << aaaaaa << bbbbbb << cccccc << aaaaaa << bbbbbb << cccccc
          << aaaaaa
          << bbbbbb
          << cccccc
          << aaaaaa
          << bbbbbb
          << cccccc;
}

I'll keep informing, however if someone has a clue of what is happening, be my guest :)

[andergnet]

I have created a pull request with the fix for this issue.

[owenca]

I'm using BreakBinaryOperations: RespectPrecedence

Before formatting:

void abc()
{
   int aaaaaa, bbbbbb, cccccc;
   aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc
      >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc;
}
void abc()
{
   int aaaaaa, bbbbbb, cccccc;
   aaaaaa << bbbbbb << cccccc << aaaaaa << bbbbbb << cccccc << aaaaaa << bbbbbb << cccccc << aaaaaa << bbbbbb << cccccc
      << aaaaaa << bbbbbb << cccccc << aaaaaa << bbbbbb << cccccc;
}

After formatting:

void abc()
{
   int aaaaaa, bbbbbb, cccccc;
   aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc;
}
void abc()
{
   int aaaaaa, bbbbbb, cccccc;
   aaaaaa
      << bbbbbb
      << cccccc
      << aaaaaa
      << bbbbbb
      << cccccc
      << aaaaaa
      << bbbbbb
      << cccccc
      << aaaaaa
      << bbbbbb
      << cccccc
      << aaaaaa
      << bbbbbb
      << cccccc
      << aaaaaa
      << bbbbbb
      << cccccc;
}

operator<< works as expected, operator>> does not.

The correct format for >> is:

void abc() {
  int aaaaaa, bbbbbb, cccccc;
  aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >>
      bbbbbb >> cccccc >> aaaaaa >> bbbbbb >> cccccc >> aaaaaa >> bbbbbb >>
      cccccc >> aaaaaa >> bbbbbb >> cccccc;
}

for both RespectPrecedence and Never, right?