from:"Ameer J via cfe\-commits"

[clang] [clang-format] Add BreakFunctionDefinitionParameters option (PR #84988)

2024-03-16 Thread Ameer J via cfe-commits


https://github.com/ameerj updated 
https://github.com/llvm/llvm-project/pull/84988

>From 0d0868ddffe1b0668a57c10cc89614ab7c840634 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Sat, 16 Mar 2024 17:03:47 -0400
Subject: [PATCH] [clang-format] Add BreakFunctionDefinitionParameters option

---
 clang/docs/ClangFormatStyleOptions.rst |  6 ++
 clang/include/clang/Format/Format.h|  7 +++
 clang/lib/Format/Format.cpp|  3 +++
 clang/lib/Format/FormatToken.h |  3 +++
 clang/lib/Format/TokenAnnotator.cpp|  7 +++
 clang/unittests/Format/FormatTest.cpp  | 18 ++
 6 files changed, 44 insertions(+)

diff --git a/clang/docs/ClangFormatStyleOptions.rst 
b/clang/docs/ClangFormatStyleOptions.rst
index 5b00a8f4c00fb8..a5e710e21c8338 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -3150,6 +3150,12 @@ the configuration (without a prefix: ``Auto``).
 
 
 
+.. _BreakFunctionDefinitionParameters:
+
+**BreakFunctionDefinitionParameters** (``Boolean``) 
:versionbadge:`clang-format 19` :ref:`¶ `
+  If ``true``, clang-format will always break before function definition
+  parameters
+
 .. _BreakInheritanceList:
 
 **BreakInheritanceList** (``BreakInheritanceListStyle``) 
:versionbadge:`clang-format 7` :ref:`¶ `
diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index a72c1b171c3e18..8bf3fac6176205 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2218,6 +2218,11 @@ struct FormatStyle {
   /// \version 3.8
   bool BreakAfterJavaFieldAnnotations;
 
+  /// If ``true``, clang-format will always break before function definition
+  /// parameters
+  /// \version 19
+  bool BreakFunctionDefinitionParameters;
+
   /// Allow breaking string literals when formatting.
   ///
   /// In C, C++, and Objective-C:
@@ -4867,6 +4872,8 @@ struct FormatStyle {
BreakBeforeInlineASMColon == R.BreakBeforeInlineASMColon &&
BreakBeforeTernaryOperators == R.BreakBeforeTernaryOperators &&
BreakConstructorInitializers == R.BreakConstructorInitializers &&
+   BreakFunctionDefinitionParameters ==
+   R.BreakFunctionDefinitionParameters &&
BreakInheritanceList == R.BreakInheritanceList &&
BreakStringLiterals == R.BreakStringLiterals &&
BreakTemplateDeclarations == R.BreakTemplateDeclarations &&
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index d5d115a3c8db85..635f8123c402c7 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -933,6 +933,8 @@ template <> struct MappingTraits {
 IO.mapOptional("BreakAfterJavaFieldAnnotations",
Style.BreakAfterJavaFieldAnnotations);
 IO.mapOptional("BreakAfterReturnType", Style.BreakAfterReturnType);
+IO.mapOptional("BreakFunctionDefinitionParameters",
+   Style.BreakFunctionDefinitionParameters);
 IO.mapOptional("BreakArrays", Style.BreakArrays);
 IO.mapOptional("BreakBeforeBinaryOperators",
Style.BreakBeforeBinaryOperators);
@@ -1443,6 +1445,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind 
Language) {
   LLVMStyle.BreakAfterAttributes = FormatStyle::ABS_Leave;
   LLVMStyle.BreakAfterJavaFieldAnnotations = false;
   LLVMStyle.BreakAfterReturnType = FormatStyle::RTBS_None;
+  LLVMStyle.BreakFunctionDefinitionParameters = false;
   LLVMStyle.BreakArrays = true;
   LLVMStyle.BreakBeforeBinaryOperators = FormatStyle::BOS_None;
   LLVMStyle.BreakBeforeBraces = FormatStyle::BS_Attach;
diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index c9022aba287187..d7cad8cec38948 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -569,6 +569,9 @@ struct FormatToken {
   /// Is optional and can be removed.
   bool Optional = false;
 
+  /// Might be function declaration open/closing paren.
+  bool MightBeFunctionDeclParen = false;
+
   /// Number of optional braces to be inserted after this token:
   ///   -1: a single left brace
   ///0: no braces
diff --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 1342d37a147915..352b2fac149b29 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1502,6 +1502,7 @@ class AnnotatingParser {
 (!Previous->isAttribute() &&
  !Previous->isOneOf(TT_RequiresClause, TT_LeadingJavaAnnotation))) 
{
   Line.MightBeFunctionDecl = true;
+  Tok->MightBeFunctionDeclParen = true;
 }
   }
   break;
@@ -5317,6 +5318,12 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine 
&Line,
   if (Right.NewlinesBefore > 1 && Style.MaxEmptyLinesToKeep > 0)
 return true;
 
+  if (Style.BreakFunctionDefinitionParameters && Line.MightBeFunctionDecl &&
+  Line.mightBeFunctionDefinition() && Left.is(tok::l_paren) &&
+  Lef

[clang] [clang-format] Add BreakFunctionDefinitionParameters option (PR #84988)

2024-03-25 Thread Ameer J via cfe-commits


ameerj wrote:

@HazardyKnusperkeks @owenca I'd like to request a review please

https://github.com/llvm/llvm-project/pull/84988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Add BreakFunctionDefinitionParameters option (PR #84988)

2024-03-26 Thread Ameer J via cfe-commits


https://github.com/ameerj updated 
https://github.com/llvm/llvm-project/pull/84988

>From 0d0868ddffe1b0668a57c10cc89614ab7c840634 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Sat, 16 Mar 2024 17:03:47 -0400
Subject: [PATCH 1/2] [clang-format] Add BreakFunctionDefinitionParameters
 option

---
 clang/docs/ClangFormatStyleOptions.rst |  6 ++
 clang/include/clang/Format/Format.h|  7 +++
 clang/lib/Format/Format.cpp|  3 +++
 clang/lib/Format/FormatToken.h |  3 +++
 clang/lib/Format/TokenAnnotator.cpp|  7 +++
 clang/unittests/Format/FormatTest.cpp  | 18 ++
 6 files changed, 44 insertions(+)

diff --git a/clang/docs/ClangFormatStyleOptions.rst 
b/clang/docs/ClangFormatStyleOptions.rst
index 5b00a8f4c00fb8..a5e710e21c8338 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -3150,6 +3150,12 @@ the configuration (without a prefix: ``Auto``).
 
 
 
+.. _BreakFunctionDefinitionParameters:
+
+**BreakFunctionDefinitionParameters** (``Boolean``) 
:versionbadge:`clang-format 19` :ref:`¶ `
+  If ``true``, clang-format will always break before function definition
+  parameters
+
 .. _BreakInheritanceList:
 
 **BreakInheritanceList** (``BreakInheritanceListStyle``) 
:versionbadge:`clang-format 7` :ref:`¶ `
diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index a72c1b171c3e18..8bf3fac6176205 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2218,6 +2218,11 @@ struct FormatStyle {
   /// \version 3.8
   bool BreakAfterJavaFieldAnnotations;
 
+  /// If ``true``, clang-format will always break before function definition
+  /// parameters
+  /// \version 19
+  bool BreakFunctionDefinitionParameters;
+
   /// Allow breaking string literals when formatting.
   ///
   /// In C, C++, and Objective-C:
@@ -4867,6 +4872,8 @@ struct FormatStyle {
BreakBeforeInlineASMColon == R.BreakBeforeInlineASMColon &&
BreakBeforeTernaryOperators == R.BreakBeforeTernaryOperators &&
BreakConstructorInitializers == R.BreakConstructorInitializers &&
+   BreakFunctionDefinitionParameters ==
+   R.BreakFunctionDefinitionParameters &&
BreakInheritanceList == R.BreakInheritanceList &&
BreakStringLiterals == R.BreakStringLiterals &&
BreakTemplateDeclarations == R.BreakTemplateDeclarations &&
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index d5d115a3c8db85..635f8123c402c7 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -933,6 +933,8 @@ template <> struct MappingTraits {
 IO.mapOptional("BreakAfterJavaFieldAnnotations",
Style.BreakAfterJavaFieldAnnotations);
 IO.mapOptional("BreakAfterReturnType", Style.BreakAfterReturnType);
+IO.mapOptional("BreakFunctionDefinitionParameters",
+   Style.BreakFunctionDefinitionParameters);
 IO.mapOptional("BreakArrays", Style.BreakArrays);
 IO.mapOptional("BreakBeforeBinaryOperators",
Style.BreakBeforeBinaryOperators);
@@ -1443,6 +1445,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind 
Language) {
   LLVMStyle.BreakAfterAttributes = FormatStyle::ABS_Leave;
   LLVMStyle.BreakAfterJavaFieldAnnotations = false;
   LLVMStyle.BreakAfterReturnType = FormatStyle::RTBS_None;
+  LLVMStyle.BreakFunctionDefinitionParameters = false;
   LLVMStyle.BreakArrays = true;
   LLVMStyle.BreakBeforeBinaryOperators = FormatStyle::BOS_None;
   LLVMStyle.BreakBeforeBraces = FormatStyle::BS_Attach;
diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index c9022aba287187..d7cad8cec38948 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -569,6 +569,9 @@ struct FormatToken {
   /// Is optional and can be removed.
   bool Optional = false;
 
+  /// Might be function declaration open/closing paren.
+  bool MightBeFunctionDeclParen = false;
+
   /// Number of optional braces to be inserted after this token:
   ///   -1: a single left brace
   ///0: no braces
diff --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 1342d37a147915..352b2fac149b29 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1502,6 +1502,7 @@ class AnnotatingParser {
 (!Previous->isAttribute() &&
  !Previous->isOneOf(TT_RequiresClause, TT_LeadingJavaAnnotation))) 
{
   Line.MightBeFunctionDecl = true;
+  Tok->MightBeFunctionDeclParen = true;
 }
   }
   break;
@@ -5317,6 +5318,12 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine 
&Line,
   if (Right.NewlinesBefore > 1 && Style.MaxEmptyLinesToKeep > 0)
 return true;
 
+  if (Style.BreakFunctionDefinitionParameters && Line.MightBeFunctionDecl &&
+  Line.mightBeFunctionDefinition() && Left.is(tok::l_paren) &&
+

[clang] [clang-format] Add BreakFunctionDefinitionParameters option (PR #84988)

2024-03-26 Thread Ameer J via cfe-commits



@@ -5317,6 +5318,12 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine 
&Line,
   if (Right.NewlinesBefore > 1 && Style.MaxEmptyLinesToKeep > 0)
 return true;
 
+  if (Style.BreakFunctionDefinitionParameters && Line.MightBeFunctionDecl &&

ameerj wrote:

Removed one unneeded check, the rest are all needed.

https://github.com/llvm/llvm-project/pull/84988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Add BreakFunctionDefinitionParameters option (PR #84988)

2024-03-26 Thread Ameer J via cfe-commits


https://github.com/ameerj updated 
https://github.com/llvm/llvm-project/pull/84988

>From 0d0868ddffe1b0668a57c10cc89614ab7c840634 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Sat, 16 Mar 2024 17:03:47 -0400
Subject: [PATCH 1/3] [clang-format] Add BreakFunctionDefinitionParameters
 option

---
 clang/docs/ClangFormatStyleOptions.rst |  6 ++
 clang/include/clang/Format/Format.h|  7 +++
 clang/lib/Format/Format.cpp|  3 +++
 clang/lib/Format/FormatToken.h |  3 +++
 clang/lib/Format/TokenAnnotator.cpp|  7 +++
 clang/unittests/Format/FormatTest.cpp  | 18 ++
 6 files changed, 44 insertions(+)

diff --git a/clang/docs/ClangFormatStyleOptions.rst 
b/clang/docs/ClangFormatStyleOptions.rst
index 5b00a8f4c00fb8..a5e710e21c8338 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -3150,6 +3150,12 @@ the configuration (without a prefix: ``Auto``).
 
 
 
+.. _BreakFunctionDefinitionParameters:
+
+**BreakFunctionDefinitionParameters** (``Boolean``) 
:versionbadge:`clang-format 19` :ref:`¶ `
+  If ``true``, clang-format will always break before function definition
+  parameters
+
 .. _BreakInheritanceList:
 
 **BreakInheritanceList** (``BreakInheritanceListStyle``) 
:versionbadge:`clang-format 7` :ref:`¶ `
diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index a72c1b171c3e18..8bf3fac6176205 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2218,6 +2218,11 @@ struct FormatStyle {
   /// \version 3.8
   bool BreakAfterJavaFieldAnnotations;
 
+  /// If ``true``, clang-format will always break before function definition
+  /// parameters
+  /// \version 19
+  bool BreakFunctionDefinitionParameters;
+
   /// Allow breaking string literals when formatting.
   ///
   /// In C, C++, and Objective-C:
@@ -4867,6 +4872,8 @@ struct FormatStyle {
BreakBeforeInlineASMColon == R.BreakBeforeInlineASMColon &&
BreakBeforeTernaryOperators == R.BreakBeforeTernaryOperators &&
BreakConstructorInitializers == R.BreakConstructorInitializers &&
+   BreakFunctionDefinitionParameters ==
+   R.BreakFunctionDefinitionParameters &&
BreakInheritanceList == R.BreakInheritanceList &&
BreakStringLiterals == R.BreakStringLiterals &&
BreakTemplateDeclarations == R.BreakTemplateDeclarations &&
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index d5d115a3c8db85..635f8123c402c7 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -933,6 +933,8 @@ template <> struct MappingTraits {
 IO.mapOptional("BreakAfterJavaFieldAnnotations",
Style.BreakAfterJavaFieldAnnotations);
 IO.mapOptional("BreakAfterReturnType", Style.BreakAfterReturnType);
+IO.mapOptional("BreakFunctionDefinitionParameters",
+   Style.BreakFunctionDefinitionParameters);
 IO.mapOptional("BreakArrays", Style.BreakArrays);
 IO.mapOptional("BreakBeforeBinaryOperators",
Style.BreakBeforeBinaryOperators);
@@ -1443,6 +1445,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind 
Language) {
   LLVMStyle.BreakAfterAttributes = FormatStyle::ABS_Leave;
   LLVMStyle.BreakAfterJavaFieldAnnotations = false;
   LLVMStyle.BreakAfterReturnType = FormatStyle::RTBS_None;
+  LLVMStyle.BreakFunctionDefinitionParameters = false;
   LLVMStyle.BreakArrays = true;
   LLVMStyle.BreakBeforeBinaryOperators = FormatStyle::BOS_None;
   LLVMStyle.BreakBeforeBraces = FormatStyle::BS_Attach;
diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index c9022aba287187..d7cad8cec38948 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -569,6 +569,9 @@ struct FormatToken {
   /// Is optional and can be removed.
   bool Optional = false;
 
+  /// Might be function declaration open/closing paren.
+  bool MightBeFunctionDeclParen = false;
+
   /// Number of optional braces to be inserted after this token:
   ///   -1: a single left brace
   ///0: no braces
diff --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 1342d37a147915..352b2fac149b29 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1502,6 +1502,7 @@ class AnnotatingParser {
 (!Previous->isAttribute() &&
  !Previous->isOneOf(TT_RequiresClause, TT_LeadingJavaAnnotation))) 
{
   Line.MightBeFunctionDecl = true;
+  Tok->MightBeFunctionDeclParen = true;
 }
   }
   break;
@@ -5317,6 +5318,12 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine 
&Line,
   if (Right.NewlinesBefore > 1 && Style.MaxEmptyLinesToKeep > 0)
 return true;
 
+  if (Style.BreakFunctionDefinitionParameters && Line.MightBeFunctionDecl &&
+  Line.mightBeFunctionDefinition() && Left.is(tok::l_paren) &&
+

[clang] [clang-format] Add BreakFunctionDefinitionParameters option (PR #84988)

2024-03-28 Thread Ameer J via cfe-commits


ameerj wrote:

@HazardyKnusperkeks Thanks for the review! 

I don't have repo permissions so I can't run all CI workflows or merge the PR.

https://github.com/llvm/llvm-project/pull/84988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Add BreakFunctionDefinitionParameters option (PR #84988)

2024-03-12 Thread Ameer J via cfe-commits


https://github.com/ameerj created 
https://github.com/llvm/llvm-project/pull/84988

This adds an option to break function definition parameters, putting them on 
the next line after the function's opening paren.

This was a missing step towards allowing styles which require all function 
definition parameters be on their own lines.

Closes #62963

>From 62023c2e3ee22a72ecd634a97e111cde9c817899 Mon Sep 17 00:00:00 2001
From: ameerj <52414509+ame...@users.noreply.github.com>
Date: Tue, 12 Mar 2024 18:40:19 -0400
Subject: [PATCH] [clang-format] Add BreakFunctionDefinitionParameters option

---
 clang/docs/ClangFormatStyleOptions.rst |  6 ++
 clang/include/clang/Format/Format.h|  7 +++
 clang/lib/Format/Format.cpp|  3 +++
 clang/lib/Format/FormatToken.h |  3 +++
 clang/lib/Format/TokenAnnotator.cpp|  8 
 clang/unittests/Format/FormatTest.cpp  | 18 ++
 6 files changed, 45 insertions(+)

diff --git a/clang/docs/ClangFormatStyleOptions.rst 
b/clang/docs/ClangFormatStyleOptions.rst
index 5b00a8f4c00fb8..a5e710e21c8338 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -3150,6 +3150,12 @@ the configuration (without a prefix: ``Auto``).
 
 
 
+.. _BreakFunctionDefinitionParameters:
+
+**BreakFunctionDefinitionParameters** (``Boolean``) 
:versionbadge:`clang-format 19` :ref:`¶ `
+  If ``true``, clang-format will always break before function definition
+  parameters
+
 .. _BreakInheritanceList:
 
 **BreakInheritanceList** (``BreakInheritanceListStyle``) 
:versionbadge:`clang-format 7` :ref:`¶ `
diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 590297fd89a398..eb115dfd2b275d 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2218,6 +2218,11 @@ struct FormatStyle {
   /// \version 3.8
   bool BreakAfterJavaFieldAnnotations;
 
+  /// If ``true``, clang-format will always break before function definition
+  /// parameters
+  /// \version 19
+  bool BreakFunctionDefinitionParameters;
+
   /// Allow breaking string literals when formatting.
   ///
   /// In C, C++, and Objective-C:
@@ -4867,6 +4872,8 @@ struct FormatStyle {
BreakBeforeInlineASMColon == R.BreakBeforeInlineASMColon &&
BreakBeforeTernaryOperators == R.BreakBeforeTernaryOperators &&
BreakConstructorInitializers == R.BreakConstructorInitializers &&
+   BreakFunctionDefinitionParameters ==
+   R.BreakFunctionDefinitionParameters &&
BreakInheritanceList == R.BreakInheritanceList &&
BreakStringLiterals == R.BreakStringLiterals &&
BreakTemplateDeclarations == R.BreakTemplateDeclarations &&
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index e64ba7eebc1ce8..ebdb4eae9520d3 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -961,6 +961,8 @@ template <> struct MappingTraits {
 IO.mapOptional("BreakAfterJavaFieldAnnotations",
Style.BreakAfterJavaFieldAnnotations);
 IO.mapOptional("BreakAfterReturnType", Style.BreakAfterReturnType);
+IO.mapOptional("BreakFunctionDefinitionParameters",
+   Style.BreakFunctionDefinitionParameters);
 IO.mapOptional("BreakArrays", Style.BreakArrays);
 IO.mapOptional("BreakBeforeBinaryOperators",
Style.BreakBeforeBinaryOperators);
@@ -1471,6 +1473,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind 
Language) {
   LLVMStyle.BreakAfterAttributes = FormatStyle::ABS_Leave;
   LLVMStyle.BreakAfterJavaFieldAnnotations = false;
   LLVMStyle.BreakAfterReturnType = FormatStyle::RTBS_None;
+  LLVMStyle.BreakFunctionDefinitionParameters = false;
   LLVMStyle.BreakArrays = true;
   LLVMStyle.BreakBeforeBinaryOperators = FormatStyle::BOS_None;
   LLVMStyle.BreakBeforeBraces = FormatStyle::BS_Attach;
diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index 31245495041960..b20682de91c0b7 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -571,6 +571,9 @@ struct FormatToken {
   /// Is optional and can be removed.
   bool Optional = false;
 
+  /// Might be function declaration open/closing paren.
+  bool MightBeFunctionDeclParen = false;
+
   /// Number of optional braces to be inserted after this token:
   ///   -1: a single left brace
   ///0: no braces
diff --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 04f0374b674e53..3d1c87d0b7253c 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1501,6 +1501,8 @@ class AnnotatingParser {
 (!Previous->isAttribute() &&
  !Previous->isOneOf(TT_RequiresClause, TT_LeadingJavaAnnotation))) 
{
   Line.MightBeFunctionDecl = true;
+  Tok->MightBeFunctionDeclParen = true;
+  Tok->MatchingParen->MightBeFunctionDeclParen

[clang] [clang-format] Add BinPackBinaryOperations configuration (PR #95013)

2024-07-01 Thread Ameer J via cfe-commits


https://github.com/ameerj updated 
https://github.com/llvm/llvm-project/pull/95013

>From a50f3d4395efd09eea8ba2e750bb785857f9a550 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:09:40 -0400
Subject: [PATCH 1/7] Add BinPackBinaryOperations

---
 clang/include/clang/Format/Format.h   | 16 
 clang/lib/Format/ContinuationIndenter.cpp | 15 +++
 clang/lib/Format/Format.cpp   |  2 ++
 3 files changed, 33 insertions(+)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 4fd6e013df25b..f30d767600061 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1192,6 +1192,21 @@ struct FormatStyle {
   /// \version 3.7
   bool BinPackArguments;
 
+  /// If ``false``, binary operations will either be all on the same line
+  /// or each operation will have one line each.
+  /// \code
+  ///   true:
+  ///    &&  ||
+  /// aaa;
+  ///
+  ///   false:
+  ///    &&
+  ///    ||
+  ///   aaa;
+  /// \endcode
+  /// \version 19
+  bool BinPackBinaryOperations;
+
   /// If ``false``, a function declaration's or function definition's
   /// parameters will either all be on the same line or will have one line 
each.
   /// \code
@@ -4978,6 +4993,7 @@ struct FormatStyle {
R.AlwaysBreakBeforeMultilineStrings &&
AttributeMacros == R.AttributeMacros &&
BinPackArguments == R.BinPackArguments &&
+   BinPackBinaryOperations == R.BinPackBinaryOperations &&
BinPackParameters == R.BinPackParameters &&
BitFieldColonSpacing == R.BitFieldColonSpacing &&
BracedInitializerIndentWidth == R.BracedInitializerIndentWidth &&
diff --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index b07360425ca6e..0780d219b68b4 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -146,6 +146,14 @@ static bool startsNextParameter(const FormatToken &Current,
Style.BreakInheritanceList != FormatStyle::BILS_BeforeComma));
 }
 
+// Returns \c true if \c Current starts a new operand in a binary operation.
+static bool startsNextOperand(const FormatToken &Current,
+  const FormatStyle &Style) {
+  const FormatToken &Previous = *Current.Previous;
+  return Previous.is(TT_BinaryOperator) && !Current.isTrailingComment() &&
+ (Previous.getPrecedence() > prec::Conditional);
+}
+
 static bool opensProtoMessageField(const FormatToken &LessTok,
const FormatStyle &Style) {
   if (LessTok.isNot(tok::less))
@@ -837,6 +845,9 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState 
&State, bool DryRun,
   }
   if (CurrentState.AvoidBinPacking && startsNextParameter(Current, Style))
 CurrentState.NoLineBreak = true;
+  if (!Style.BinPackBinaryOperations && startsNextOperand(Current, Style))
+CurrentState.NoLineBreak = true;
+
   if (startsSegmentOfBuilderTypeCall(Current) &&
   State.Column > getNewLineColumn(State)) {
 CurrentState.ContainsUnwrappedBuilder = true;
@@ -1203,6 +1214,10 @@ unsigned 
ContinuationIndenter::addTokenOnNewLine(LineState &State,
 }
   }
 
+  if (!Style.BinPackBinaryOperations && Previous.is(TT_BinaryOperator) &&
+  (Previous.getPrecedence() > prec::Conditional)) {
+CurrentState.BreakBeforeParameter = true;
+  }
   return Penalty;
 }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index c015e03fa15e7..4d71ce95afe1d 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -935,6 +935,7 @@ template <> struct MappingTraits {
Style.AlwaysBreakBeforeMultilineStrings);
 IO.mapOptional("AttributeMacros", Style.AttributeMacros);
 IO.mapOptional("BinPackArguments", Style.BinPackArguments);
+IO.mapOptional("BinPackBinaryOperations", Style.BinPackBinaryOperations);
 IO.mapOptional("BinPackParameters", Style.BinPackParameters);
 IO.mapOptional("BitFieldColonSpacing", Style.BitFieldColonSpacing);
 IO.mapOptional("BracedInitializerIndentWidth",
@@ -1439,6 +1440,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind 
Language) {
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AttributeMacros.push_back("__capability");
   LLVMStyle.BinPackArguments = true;
+  LLVMStyle.BinPackBinaryOperations = true;
   LLVMStyle.BinPackParameters = true;
   LLVMStyle.BitFieldColonSpacing = FormatStyle::BFCS_Both;
   LLVMStyle.BracedInitializerIndentWidth = std::nullopt;

>From 1fe691c6ee0393cc3f40f0c5e94e01fe79a909c3 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:43:56 -0400
Subject: [PATCH 2/7] Add BinPackBinaryOperations unit tests

---
 clang/unittests/Format/Forma

[clang] [clang-format] Add BinPackBinaryOperations configuration (PR #95013)

2024-07-01 Thread Ameer J via cfe-commits



@@ -27492,6 +27492,86 @@ TEST_F(FormatTest, SpaceBetweenKeywordAndLiteral) {
   verifyFormat("return sizeof \"5\";");
 }
 
+TEST_F(FormatTest, BinPackBinaryOperations) {
+  auto Style = getLLVMStyle();
+  Style.BinPackBinaryOperations = false;
+
+  // Logical operations
+  verifyFormat("if (condition1 && condition2) {\n"
+   "}",
+   Style);
+  verifyFormat("if (condition1 && // comment\n"
+   "condition2 &&\n"
+   "(condition3 || condition4) && // comment\n"
+   "condition5 &&\n"
+   "condition6) {\n"
+   "}",
+   Style);
+  verifyFormat("if (loongcondition1 &&\n"
+   "loongcondition2) {\n"
+   "}",
+   Style);
+
+  // Arithmetic
+  verifyFormat("const int result = lhs + rhs;\n", Style);
+  verifyFormat("const int result = loongop1 +\n"
+   "   loongop2 +\n"
+   "   loongop3;\n",
+   Style);
+
+  // clang-format off
+  verifyFormat("const int result =\n"
+   "operand1 + operand2 - (operand3 + operand4) - operand5 * 
operand6;\n",
+   Style);
+  // clang-format on
+
+  verifyFormat("const int result = longOperand1 +\n"
+   "   longOperand2 -\n"
+   "   (longOperand3 + longOperand4) -\n"
+   "   longOperand5 * longOperand6;\n",

ameerj wrote:

Issue was related to fake parens. Latest commit resolves the bug.

https://github.com/llvm/llvm-project/pull/95013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Add BinPackBinaryOperations configuration (PR #95013)

2024-07-01 Thread Ameer J via cfe-commits


https://github.com/ameerj updated 
https://github.com/llvm/llvm-project/pull/95013

>From a50f3d4395efd09eea8ba2e750bb785857f9a550 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:09:40 -0400
Subject: [PATCH 1/8] Add BinPackBinaryOperations

---
 clang/include/clang/Format/Format.h   | 16 
 clang/lib/Format/ContinuationIndenter.cpp | 15 +++
 clang/lib/Format/Format.cpp   |  2 ++
 3 files changed, 33 insertions(+)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 4fd6e013df25b..f30d767600061 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1192,6 +1192,21 @@ struct FormatStyle {
   /// \version 3.7
   bool BinPackArguments;
 
+  /// If ``false``, binary operations will either be all on the same line
+  /// or each operation will have one line each.
+  /// \code
+  ///   true:
+  ///    &&  ||
+  /// aaa;
+  ///
+  ///   false:
+  ///    &&
+  ///    ||
+  ///   aaa;
+  /// \endcode
+  /// \version 19
+  bool BinPackBinaryOperations;
+
   /// If ``false``, a function declaration's or function definition's
   /// parameters will either all be on the same line or will have one line 
each.
   /// \code
@@ -4978,6 +4993,7 @@ struct FormatStyle {
R.AlwaysBreakBeforeMultilineStrings &&
AttributeMacros == R.AttributeMacros &&
BinPackArguments == R.BinPackArguments &&
+   BinPackBinaryOperations == R.BinPackBinaryOperations &&
BinPackParameters == R.BinPackParameters &&
BitFieldColonSpacing == R.BitFieldColonSpacing &&
BracedInitializerIndentWidth == R.BracedInitializerIndentWidth &&
diff --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index b07360425ca6e..0780d219b68b4 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -146,6 +146,14 @@ static bool startsNextParameter(const FormatToken &Current,
Style.BreakInheritanceList != FormatStyle::BILS_BeforeComma));
 }
 
+// Returns \c true if \c Current starts a new operand in a binary operation.
+static bool startsNextOperand(const FormatToken &Current,
+  const FormatStyle &Style) {
+  const FormatToken &Previous = *Current.Previous;
+  return Previous.is(TT_BinaryOperator) && !Current.isTrailingComment() &&
+ (Previous.getPrecedence() > prec::Conditional);
+}
+
 static bool opensProtoMessageField(const FormatToken &LessTok,
const FormatStyle &Style) {
   if (LessTok.isNot(tok::less))
@@ -837,6 +845,9 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState 
&State, bool DryRun,
   }
   if (CurrentState.AvoidBinPacking && startsNextParameter(Current, Style))
 CurrentState.NoLineBreak = true;
+  if (!Style.BinPackBinaryOperations && startsNextOperand(Current, Style))
+CurrentState.NoLineBreak = true;
+
   if (startsSegmentOfBuilderTypeCall(Current) &&
   State.Column > getNewLineColumn(State)) {
 CurrentState.ContainsUnwrappedBuilder = true;
@@ -1203,6 +1214,10 @@ unsigned 
ContinuationIndenter::addTokenOnNewLine(LineState &State,
 }
   }
 
+  if (!Style.BinPackBinaryOperations && Previous.is(TT_BinaryOperator) &&
+  (Previous.getPrecedence() > prec::Conditional)) {
+CurrentState.BreakBeforeParameter = true;
+  }
   return Penalty;
 }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index c015e03fa15e7..4d71ce95afe1d 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -935,6 +935,7 @@ template <> struct MappingTraits {
Style.AlwaysBreakBeforeMultilineStrings);
 IO.mapOptional("AttributeMacros", Style.AttributeMacros);
 IO.mapOptional("BinPackArguments", Style.BinPackArguments);
+IO.mapOptional("BinPackBinaryOperations", Style.BinPackBinaryOperations);
 IO.mapOptional("BinPackParameters", Style.BinPackParameters);
 IO.mapOptional("BitFieldColonSpacing", Style.BitFieldColonSpacing);
 IO.mapOptional("BracedInitializerIndentWidth",
@@ -1439,6 +1440,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind 
Language) {
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AttributeMacros.push_back("__capability");
   LLVMStyle.BinPackArguments = true;
+  LLVMStyle.BinPackBinaryOperations = true;
   LLVMStyle.BinPackParameters = true;
   LLVMStyle.BitFieldColonSpacing = FormatStyle::BFCS_Both;
   LLVMStyle.BracedInitializerIndentWidth = std::nullopt;

>From 1fe691c6ee0393cc3f40f0c5e94e01fe79a909c3 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:43:56 -0400
Subject: [PATCH 2/8] Add BinPackBinaryOperations unit tests

---
 clang/unittests/Format/Forma

[clang] [clang-format] Add BinPackBinaryOperations configuration (PR #95013)

2024-07-01 Thread Ameer J via cfe-commits


https://github.com/ameerj updated 
https://github.com/llvm/llvm-project/pull/95013

>From a50f3d4395efd09eea8ba2e750bb785857f9a550 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:09:40 -0400
Subject: [PATCH 1/9] Add BinPackBinaryOperations

---
 clang/include/clang/Format/Format.h   | 16 
 clang/lib/Format/ContinuationIndenter.cpp | 15 +++
 clang/lib/Format/Format.cpp   |  2 ++
 3 files changed, 33 insertions(+)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 4fd6e013df25b..f30d767600061 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1192,6 +1192,21 @@ struct FormatStyle {
   /// \version 3.7
   bool BinPackArguments;
 
+  /// If ``false``, binary operations will either be all on the same line
+  /// or each operation will have one line each.
+  /// \code
+  ///   true:
+  ///    &&  ||
+  /// aaa;
+  ///
+  ///   false:
+  ///    &&
+  ///    ||
+  ///   aaa;
+  /// \endcode
+  /// \version 19
+  bool BinPackBinaryOperations;
+
   /// If ``false``, a function declaration's or function definition's
   /// parameters will either all be on the same line or will have one line 
each.
   /// \code
@@ -4978,6 +4993,7 @@ struct FormatStyle {
R.AlwaysBreakBeforeMultilineStrings &&
AttributeMacros == R.AttributeMacros &&
BinPackArguments == R.BinPackArguments &&
+   BinPackBinaryOperations == R.BinPackBinaryOperations &&
BinPackParameters == R.BinPackParameters &&
BitFieldColonSpacing == R.BitFieldColonSpacing &&
BracedInitializerIndentWidth == R.BracedInitializerIndentWidth &&
diff --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index b07360425ca6e..0780d219b68b4 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -146,6 +146,14 @@ static bool startsNextParameter(const FormatToken &Current,
Style.BreakInheritanceList != FormatStyle::BILS_BeforeComma));
 }
 
+// Returns \c true if \c Current starts a new operand in a binary operation.
+static bool startsNextOperand(const FormatToken &Current,
+  const FormatStyle &Style) {
+  const FormatToken &Previous = *Current.Previous;
+  return Previous.is(TT_BinaryOperator) && !Current.isTrailingComment() &&
+ (Previous.getPrecedence() > prec::Conditional);
+}
+
 static bool opensProtoMessageField(const FormatToken &LessTok,
const FormatStyle &Style) {
   if (LessTok.isNot(tok::less))
@@ -837,6 +845,9 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState 
&State, bool DryRun,
   }
   if (CurrentState.AvoidBinPacking && startsNextParameter(Current, Style))
 CurrentState.NoLineBreak = true;
+  if (!Style.BinPackBinaryOperations && startsNextOperand(Current, Style))
+CurrentState.NoLineBreak = true;
+
   if (startsSegmentOfBuilderTypeCall(Current) &&
   State.Column > getNewLineColumn(State)) {
 CurrentState.ContainsUnwrappedBuilder = true;
@@ -1203,6 +1214,10 @@ unsigned 
ContinuationIndenter::addTokenOnNewLine(LineState &State,
 }
   }
 
+  if (!Style.BinPackBinaryOperations && Previous.is(TT_BinaryOperator) &&
+  (Previous.getPrecedence() > prec::Conditional)) {
+CurrentState.BreakBeforeParameter = true;
+  }
   return Penalty;
 }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index c015e03fa15e7..4d71ce95afe1d 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -935,6 +935,7 @@ template <> struct MappingTraits {
Style.AlwaysBreakBeforeMultilineStrings);
 IO.mapOptional("AttributeMacros", Style.AttributeMacros);
 IO.mapOptional("BinPackArguments", Style.BinPackArguments);
+IO.mapOptional("BinPackBinaryOperations", Style.BinPackBinaryOperations);
 IO.mapOptional("BinPackParameters", Style.BinPackParameters);
 IO.mapOptional("BitFieldColonSpacing", Style.BitFieldColonSpacing);
 IO.mapOptional("BracedInitializerIndentWidth",
@@ -1439,6 +1440,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind 
Language) {
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AttributeMacros.push_back("__capability");
   LLVMStyle.BinPackArguments = true;
+  LLVMStyle.BinPackBinaryOperations = true;
   LLVMStyle.BinPackParameters = true;
   LLVMStyle.BitFieldColonSpacing = FormatStyle::BFCS_Both;
   LLVMStyle.BracedInitializerIndentWidth = std::nullopt;

>From 1fe691c6ee0393cc3f40f0c5e94e01fe79a909c3 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:43:56 -0400
Subject: [PATCH 2/9] Add BinPackBinaryOperations unit tests

---
 clang/unittests/Format/Forma

[clang] [clang-format] Add BinPackBinaryOperations configuration (PR #95013)

2024-07-01 Thread Ameer J via cfe-commits


https://github.com/ameerj updated 
https://github.com/llvm/llvm-project/pull/95013

>From a50f3d4395efd09eea8ba2e750bb785857f9a550 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:09:40 -0400
Subject: [PATCH 01/10] Add BinPackBinaryOperations

---
 clang/include/clang/Format/Format.h   | 16 
 clang/lib/Format/ContinuationIndenter.cpp | 15 +++
 clang/lib/Format/Format.cpp   |  2 ++
 3 files changed, 33 insertions(+)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 4fd6e013df25b..f30d767600061 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1192,6 +1192,21 @@ struct FormatStyle {
   /// \version 3.7
   bool BinPackArguments;
 
+  /// If ``false``, binary operations will either be all on the same line
+  /// or each operation will have one line each.
+  /// \code
+  ///   true:
+  ///    &&  ||
+  /// aaa;
+  ///
+  ///   false:
+  ///    &&
+  ///    ||
+  ///   aaa;
+  /// \endcode
+  /// \version 19
+  bool BinPackBinaryOperations;
+
   /// If ``false``, a function declaration's or function definition's
   /// parameters will either all be on the same line or will have one line 
each.
   /// \code
@@ -4978,6 +4993,7 @@ struct FormatStyle {
R.AlwaysBreakBeforeMultilineStrings &&
AttributeMacros == R.AttributeMacros &&
BinPackArguments == R.BinPackArguments &&
+   BinPackBinaryOperations == R.BinPackBinaryOperations &&
BinPackParameters == R.BinPackParameters &&
BitFieldColonSpacing == R.BitFieldColonSpacing &&
BracedInitializerIndentWidth == R.BracedInitializerIndentWidth &&
diff --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index b07360425ca6e..0780d219b68b4 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -146,6 +146,14 @@ static bool startsNextParameter(const FormatToken &Current,
Style.BreakInheritanceList != FormatStyle::BILS_BeforeComma));
 }
 
+// Returns \c true if \c Current starts a new operand in a binary operation.
+static bool startsNextOperand(const FormatToken &Current,
+  const FormatStyle &Style) {
+  const FormatToken &Previous = *Current.Previous;
+  return Previous.is(TT_BinaryOperator) && !Current.isTrailingComment() &&
+ (Previous.getPrecedence() > prec::Conditional);
+}
+
 static bool opensProtoMessageField(const FormatToken &LessTok,
const FormatStyle &Style) {
   if (LessTok.isNot(tok::less))
@@ -837,6 +845,9 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState 
&State, bool DryRun,
   }
   if (CurrentState.AvoidBinPacking && startsNextParameter(Current, Style))
 CurrentState.NoLineBreak = true;
+  if (!Style.BinPackBinaryOperations && startsNextOperand(Current, Style))
+CurrentState.NoLineBreak = true;
+
   if (startsSegmentOfBuilderTypeCall(Current) &&
   State.Column > getNewLineColumn(State)) {
 CurrentState.ContainsUnwrappedBuilder = true;
@@ -1203,6 +1214,10 @@ unsigned 
ContinuationIndenter::addTokenOnNewLine(LineState &State,
 }
   }
 
+  if (!Style.BinPackBinaryOperations && Previous.is(TT_BinaryOperator) &&
+  (Previous.getPrecedence() > prec::Conditional)) {
+CurrentState.BreakBeforeParameter = true;
+  }
   return Penalty;
 }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index c015e03fa15e7..4d71ce95afe1d 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -935,6 +935,7 @@ template <> struct MappingTraits {
Style.AlwaysBreakBeforeMultilineStrings);
 IO.mapOptional("AttributeMacros", Style.AttributeMacros);
 IO.mapOptional("BinPackArguments", Style.BinPackArguments);
+IO.mapOptional("BinPackBinaryOperations", Style.BinPackBinaryOperations);
 IO.mapOptional("BinPackParameters", Style.BinPackParameters);
 IO.mapOptional("BitFieldColonSpacing", Style.BitFieldColonSpacing);
 IO.mapOptional("BracedInitializerIndentWidth",
@@ -1439,6 +1440,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind 
Language) {
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AttributeMacros.push_back("__capability");
   LLVMStyle.BinPackArguments = true;
+  LLVMStyle.BinPackBinaryOperations = true;
   LLVMStyle.BinPackParameters = true;
   LLVMStyle.BitFieldColonSpacing = FormatStyle::BFCS_Both;
   LLVMStyle.BracedInitializerIndentWidth = std::nullopt;

>From 1fe691c6ee0393cc3f40f0c5e94e01fe79a909c3 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:43:56 -0400
Subject: [PATCH 02/10] Add BinPackBinaryOperations unit tests

---
 clang/unittests/Format/F

[clang] [clang-format] Add BinPackBinaryOperations configuration (PR #95013)

2024-07-02 Thread Ameer J via cfe-commits


https://github.com/ameerj updated 
https://github.com/llvm/llvm-project/pull/95013

>From a50f3d4395efd09eea8ba2e750bb785857f9a550 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:09:40 -0400
Subject: [PATCH 01/10] Add BinPackBinaryOperations

---
 clang/include/clang/Format/Format.h   | 16 
 clang/lib/Format/ContinuationIndenter.cpp | 15 +++
 clang/lib/Format/Format.cpp   |  2 ++
 3 files changed, 33 insertions(+)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 4fd6e013df25b..f30d767600061 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1192,6 +1192,21 @@ struct FormatStyle {
   /// \version 3.7
   bool BinPackArguments;
 
+  /// If ``false``, binary operations will either be all on the same line
+  /// or each operation will have one line each.
+  /// \code
+  ///   true:
+  ///    &&  ||
+  /// aaa;
+  ///
+  ///   false:
+  ///    &&
+  ///    ||
+  ///   aaa;
+  /// \endcode
+  /// \version 19
+  bool BinPackBinaryOperations;
+
   /// If ``false``, a function declaration's or function definition's
   /// parameters will either all be on the same line or will have one line 
each.
   /// \code
@@ -4978,6 +4993,7 @@ struct FormatStyle {
R.AlwaysBreakBeforeMultilineStrings &&
AttributeMacros == R.AttributeMacros &&
BinPackArguments == R.BinPackArguments &&
+   BinPackBinaryOperations == R.BinPackBinaryOperations &&
BinPackParameters == R.BinPackParameters &&
BitFieldColonSpacing == R.BitFieldColonSpacing &&
BracedInitializerIndentWidth == R.BracedInitializerIndentWidth &&
diff --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index b07360425ca6e..0780d219b68b4 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -146,6 +146,14 @@ static bool startsNextParameter(const FormatToken &Current,
Style.BreakInheritanceList != FormatStyle::BILS_BeforeComma));
 }
 
+// Returns \c true if \c Current starts a new operand in a binary operation.
+static bool startsNextOperand(const FormatToken &Current,
+  const FormatStyle &Style) {
+  const FormatToken &Previous = *Current.Previous;
+  return Previous.is(TT_BinaryOperator) && !Current.isTrailingComment() &&
+ (Previous.getPrecedence() > prec::Conditional);
+}
+
 static bool opensProtoMessageField(const FormatToken &LessTok,
const FormatStyle &Style) {
   if (LessTok.isNot(tok::less))
@@ -837,6 +845,9 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState 
&State, bool DryRun,
   }
   if (CurrentState.AvoidBinPacking && startsNextParameter(Current, Style))
 CurrentState.NoLineBreak = true;
+  if (!Style.BinPackBinaryOperations && startsNextOperand(Current, Style))
+CurrentState.NoLineBreak = true;
+
   if (startsSegmentOfBuilderTypeCall(Current) &&
   State.Column > getNewLineColumn(State)) {
 CurrentState.ContainsUnwrappedBuilder = true;
@@ -1203,6 +1214,10 @@ unsigned 
ContinuationIndenter::addTokenOnNewLine(LineState &State,
 }
   }
 
+  if (!Style.BinPackBinaryOperations && Previous.is(TT_BinaryOperator) &&
+  (Previous.getPrecedence() > prec::Conditional)) {
+CurrentState.BreakBeforeParameter = true;
+  }
   return Penalty;
 }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index c015e03fa15e7..4d71ce95afe1d 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -935,6 +935,7 @@ template <> struct MappingTraits {
Style.AlwaysBreakBeforeMultilineStrings);
 IO.mapOptional("AttributeMacros", Style.AttributeMacros);
 IO.mapOptional("BinPackArguments", Style.BinPackArguments);
+IO.mapOptional("BinPackBinaryOperations", Style.BinPackBinaryOperations);
 IO.mapOptional("BinPackParameters", Style.BinPackParameters);
 IO.mapOptional("BitFieldColonSpacing", Style.BitFieldColonSpacing);
 IO.mapOptional("BracedInitializerIndentWidth",
@@ -1439,6 +1440,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind 
Language) {
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AttributeMacros.push_back("__capability");
   LLVMStyle.BinPackArguments = true;
+  LLVMStyle.BinPackBinaryOperations = true;
   LLVMStyle.BinPackParameters = true;
   LLVMStyle.BitFieldColonSpacing = FormatStyle::BFCS_Both;
   LLVMStyle.BracedInitializerIndentWidth = std::nullopt;

>From 1fe691c6ee0393cc3f40f0c5e94e01fe79a909c3 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:43:56 -0400
Subject: [PATCH 02/10] Add BinPackBinaryOperations unit tests

---
 clang/unittests/Format/F

[clang] [clang-format] Add BreakBinaryOperations configuration (PR #95013)

2024-08-09 Thread Ameer J via cfe-commits


ameerj wrote:

Thanks for the review feedback @owenca @HazardyKnusperkeks
Please merge this PR for me if you feel it is ready as I don't have merge 
permissions

https://github.com/llvm/llvm-project/pull/95013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Add BinPackBinaryOperations configuration (PR #95013)

2024-06-10 Thread Ameer J via cfe-commits


https://github.com/ameerj created 
https://github.com/llvm/llvm-project/pull/95013

By default, clang-format packs binary operations, but it may be desirable to 
have compound operations be on individual lines instead of being packed.

This PR adds the option `BinPackBinaryOperations`, which mimics the behavior of 
`BinPackArguments` and `BinPackParameters`, but applied to binary operations.
When `BinPackBinaryOperations` is set to `false`, binary operations will either 
be all on the same line, or each operation will have one line each.

This applies to all logical and arithmetic/bitwise binary operations

Maybe partially addresses #79487 ?

>From a50f3d4395efd09eea8ba2e750bb785857f9a550 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:09:40 -0400
Subject: [PATCH 1/3] Add BinPackBinaryOperations

---
 clang/include/clang/Format/Format.h   | 16 
 clang/lib/Format/ContinuationIndenter.cpp | 15 +++
 clang/lib/Format/Format.cpp   |  2 ++
 3 files changed, 33 insertions(+)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 4fd6e013df25b..f30d767600061 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1192,6 +1192,21 @@ struct FormatStyle {
   /// \version 3.7
   bool BinPackArguments;
 
+  /// If ``false``, binary operations will either be all on the same line
+  /// or each operation will have one line each.
+  /// \code
+  ///   true:
+  ///    &&  ||
+  /// aaa;
+  ///
+  ///   false:
+  ///    &&
+  ///    ||
+  ///   aaa;
+  /// \endcode
+  /// \version 19
+  bool BinPackBinaryOperations;
+
   /// If ``false``, a function declaration's or function definition's
   /// parameters will either all be on the same line or will have one line 
each.
   /// \code
@@ -4978,6 +4993,7 @@ struct FormatStyle {
R.AlwaysBreakBeforeMultilineStrings &&
AttributeMacros == R.AttributeMacros &&
BinPackArguments == R.BinPackArguments &&
+   BinPackBinaryOperations == R.BinPackBinaryOperations &&
BinPackParameters == R.BinPackParameters &&
BitFieldColonSpacing == R.BitFieldColonSpacing &&
BracedInitializerIndentWidth == R.BracedInitializerIndentWidth &&
diff --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index b07360425ca6e..0780d219b68b4 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -146,6 +146,14 @@ static bool startsNextParameter(const FormatToken &Current,
Style.BreakInheritanceList != FormatStyle::BILS_BeforeComma));
 }
 
+// Returns \c true if \c Current starts a new operand in a binary operation.
+static bool startsNextOperand(const FormatToken &Current,
+  const FormatStyle &Style) {
+  const FormatToken &Previous = *Current.Previous;
+  return Previous.is(TT_BinaryOperator) && !Current.isTrailingComment() &&
+ (Previous.getPrecedence() > prec::Conditional);
+}
+
 static bool opensProtoMessageField(const FormatToken &LessTok,
const FormatStyle &Style) {
   if (LessTok.isNot(tok::less))
@@ -837,6 +845,9 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState 
&State, bool DryRun,
   }
   if (CurrentState.AvoidBinPacking && startsNextParameter(Current, Style))
 CurrentState.NoLineBreak = true;
+  if (!Style.BinPackBinaryOperations && startsNextOperand(Current, Style))
+CurrentState.NoLineBreak = true;
+
   if (startsSegmentOfBuilderTypeCall(Current) &&
   State.Column > getNewLineColumn(State)) {
 CurrentState.ContainsUnwrappedBuilder = true;
@@ -1203,6 +1214,10 @@ unsigned 
ContinuationIndenter::addTokenOnNewLine(LineState &State,
 }
   }
 
+  if (!Style.BinPackBinaryOperations && Previous.is(TT_BinaryOperator) &&
+  (Previous.getPrecedence() > prec::Conditional)) {
+CurrentState.BreakBeforeParameter = true;
+  }
   return Penalty;
 }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index c015e03fa15e7..4d71ce95afe1d 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -935,6 +935,7 @@ template <> struct MappingTraits {
Style.AlwaysBreakBeforeMultilineStrings);
 IO.mapOptional("AttributeMacros", Style.AttributeMacros);
 IO.mapOptional("BinPackArguments", Style.BinPackArguments);
+IO.mapOptional("BinPackBinaryOperations", Style.BinPackBinaryOperations);
 IO.mapOptional("BinPackParameters", Style.BinPackParameters);
 IO.mapOptional("BitFieldColonSpacing", Style.BitFieldColonSpacing);
 IO.mapOptional("BracedInitializerIndentWidth",
@@ -1439,6 +1440,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind 
Language) {
   L

[clang] [clang-format] Add BinPackBinaryOperations configuration (PR #95013)

2024-06-11 Thread Ameer J via cfe-commits



@@ -27492,6 +27492,86 @@ TEST_F(FormatTest, SpaceBetweenKeywordAndLiteral) {
   verifyFormat("return sizeof \"5\";");
 }
 
+TEST_F(FormatTest, BinPackBinaryOperations) {
+  auto Style = getLLVMStyle();
+  Style.BinPackBinaryOperations = false;
+
+  // Logical operations
+  verifyFormat("if (condition1 && condition2) {\n"
+   "}",
+   Style);
+  verifyFormat("if (condition1 && // comment\n"
+   "condition2 &&\n"
+   "(condition3 || condition4) && // comment\n"
+   "condition5 &&\n"
+   "condition6) {\n"
+   "}",
+   Style);
+  verifyFormat("if (loongcondition1 &&\n"
+   "loongcondition2) {\n"
+   "}",
+   Style);
+
+  // Arithmetic
+  verifyFormat("const int result = lhs + rhs;\n", Style);
+  verifyFormat("const int result = loongop1 +\n"
+   "   loongop2 +\n"
+   "   loongop3;\n",
+   Style);
+
+  // clang-format off
+  verifyFormat("const int result =\n"
+   "operand1 + operand2 - (operand3 + operand4) - operand5 * 
operand6;\n",
+   Style);
+  // clang-format on
+
+  verifyFormat("const int result = longOperand1 +\n"
+   "   longOperand2 -\n"
+   "   (longOperand3 + longOperand4) -\n"
+   "   longOperand5 * longOperand6;\n",

ameerj wrote:

I guess I'm indifferent to whether this happens or not, so I'll remove it from 
the test.

It seems to be because multiplication is a different precedence compared to 
addition and subtraction

https://github.com/llvm/llvm-project/pull/95013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Add BinPackBinaryOperations configuration (PR #95013)

2024-06-11 Thread Ameer J via cfe-commits



@@ -1203,6 +1214,10 @@ unsigned 
ContinuationIndenter::addTokenOnNewLine(LineState &State,
 }
   }
 
+  if (!Style.BinPackBinaryOperations && Previous.is(TT_BinaryOperator) &&
+  (Previous.getPrecedence() > prec::Conditional)) {
+CurrentState.BreakBeforeParameter = true;

ameerj wrote:

This is for cases such as this:
```c++
result = operand1 +
 operand2 -
 operand3 +
 operand4 -
 operand5 +
 operand6;
```
without it, the formatting could do this:
```c++
result = operand1 +
 operand2 -
 operand3 + operand4 - operand5 + operand6;
```

https://github.com/llvm/llvm-project/pull/95013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Add BinPackBinaryOperations configuration (PR #95013)

2024-06-11 Thread Ameer J via cfe-commits


https://github.com/ameerj updated 
https://github.com/llvm/llvm-project/pull/95013

>From a50f3d4395efd09eea8ba2e750bb785857f9a550 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:09:40 -0400
Subject: [PATCH 1/6] Add BinPackBinaryOperations

---
 clang/include/clang/Format/Format.h   | 16 
 clang/lib/Format/ContinuationIndenter.cpp | 15 +++
 clang/lib/Format/Format.cpp   |  2 ++
 3 files changed, 33 insertions(+)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 4fd6e013df25b..f30d767600061 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1192,6 +1192,21 @@ struct FormatStyle {
   /// \version 3.7
   bool BinPackArguments;
 
+  /// If ``false``, binary operations will either be all on the same line
+  /// or each operation will have one line each.
+  /// \code
+  ///   true:
+  ///    &&  ||
+  /// aaa;
+  ///
+  ///   false:
+  ///    &&
+  ///    ||
+  ///   aaa;
+  /// \endcode
+  /// \version 19
+  bool BinPackBinaryOperations;
+
   /// If ``false``, a function declaration's or function definition's
   /// parameters will either all be on the same line or will have one line 
each.
   /// \code
@@ -4978,6 +4993,7 @@ struct FormatStyle {
R.AlwaysBreakBeforeMultilineStrings &&
AttributeMacros == R.AttributeMacros &&
BinPackArguments == R.BinPackArguments &&
+   BinPackBinaryOperations == R.BinPackBinaryOperations &&
BinPackParameters == R.BinPackParameters &&
BitFieldColonSpacing == R.BitFieldColonSpacing &&
BracedInitializerIndentWidth == R.BracedInitializerIndentWidth &&
diff --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index b07360425ca6e..0780d219b68b4 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -146,6 +146,14 @@ static bool startsNextParameter(const FormatToken &Current,
Style.BreakInheritanceList != FormatStyle::BILS_BeforeComma));
 }
 
+// Returns \c true if \c Current starts a new operand in a binary operation.
+static bool startsNextOperand(const FormatToken &Current,
+  const FormatStyle &Style) {
+  const FormatToken &Previous = *Current.Previous;
+  return Previous.is(TT_BinaryOperator) && !Current.isTrailingComment() &&
+ (Previous.getPrecedence() > prec::Conditional);
+}
+
 static bool opensProtoMessageField(const FormatToken &LessTok,
const FormatStyle &Style) {
   if (LessTok.isNot(tok::less))
@@ -837,6 +845,9 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState 
&State, bool DryRun,
   }
   if (CurrentState.AvoidBinPacking && startsNextParameter(Current, Style))
 CurrentState.NoLineBreak = true;
+  if (!Style.BinPackBinaryOperations && startsNextOperand(Current, Style))
+CurrentState.NoLineBreak = true;
+
   if (startsSegmentOfBuilderTypeCall(Current) &&
   State.Column > getNewLineColumn(State)) {
 CurrentState.ContainsUnwrappedBuilder = true;
@@ -1203,6 +1214,10 @@ unsigned 
ContinuationIndenter::addTokenOnNewLine(LineState &State,
 }
   }
 
+  if (!Style.BinPackBinaryOperations && Previous.is(TT_BinaryOperator) &&
+  (Previous.getPrecedence() > prec::Conditional)) {
+CurrentState.BreakBeforeParameter = true;
+  }
   return Penalty;
 }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index c015e03fa15e7..4d71ce95afe1d 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -935,6 +935,7 @@ template <> struct MappingTraits {
Style.AlwaysBreakBeforeMultilineStrings);
 IO.mapOptional("AttributeMacros", Style.AttributeMacros);
 IO.mapOptional("BinPackArguments", Style.BinPackArguments);
+IO.mapOptional("BinPackBinaryOperations", Style.BinPackBinaryOperations);
 IO.mapOptional("BinPackParameters", Style.BinPackParameters);
 IO.mapOptional("BitFieldColonSpacing", Style.BitFieldColonSpacing);
 IO.mapOptional("BracedInitializerIndentWidth",
@@ -1439,6 +1440,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind 
Language) {
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AttributeMacros.push_back("__capability");
   LLVMStyle.BinPackArguments = true;
+  LLVMStyle.BinPackBinaryOperations = true;
   LLVMStyle.BinPackParameters = true;
   LLVMStyle.BitFieldColonSpacing = FormatStyle::BFCS_Both;
   LLVMStyle.BracedInitializerIndentWidth = std::nullopt;

>From 1fe691c6ee0393cc3f40f0c5e94e01fe79a909c3 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:43:56 -0400
Subject: [PATCH 2/6] Add BinPackBinaryOperations unit tests

---
 clang/unittests/Format/Forma

[clang] [clang-format] Add BinPackBinaryOperations configuration (PR #95013)

2024-06-12 Thread Ameer J via cfe-commits



@@ -27492,6 +27492,86 @@ TEST_F(FormatTest, SpaceBetweenKeywordAndLiteral) {
   verifyFormat("return sizeof \"5\";");
 }
 
+TEST_F(FormatTest, BinPackBinaryOperations) {
+  auto Style = getLLVMStyle();
+  Style.BinPackBinaryOperations = false;
+
+  // Logical operations
+  verifyFormat("if (condition1 && condition2) {\n"
+   "}",
+   Style);
+  verifyFormat("if (condition1 && // comment\n"
+   "condition2 &&\n"
+   "(condition3 || condition4) && // comment\n"
+   "condition5 &&\n"
+   "condition6) {\n"
+   "}",
+   Style);
+  verifyFormat("if (loongcondition1 &&\n"
+   "loongcondition2) {\n"
+   "}",
+   Style);
+
+  // Arithmetic
+  verifyFormat("const int result = lhs + rhs;\n", Style);
+  verifyFormat("const int result = loongop1 +\n"
+   "   loongop2 +\n"
+   "   loongop3;\n",
+   Style);
+
+  // clang-format off
+  verifyFormat("const int result =\n"
+   "operand1 + operand2 - (operand3 + operand4) - operand5 * 
operand6;\n",
+   Style);
+  // clang-format on
+
+  verifyFormat("const int result = longOperand1 +\n"
+   "   longOperand2 -\n"
+   "   (longOperand3 + longOperand4) -\n"
+   "   longOperand5 * longOperand6;\n",

ameerj wrote:

I'm admittedly not too familiar with the codebase. There seems to be some logic 
to differentiate a sequence of operations based on their precedence, and 
consider them to overflow the column limit or not accordingly. 

i.e. the operations inside the parens, or the multiplication do not overflow 
the column limit, so they don't break. But the surrounding operations that are 
of the addition precedence overflow the column limit, and are subject to break 
when BinPackBinaryOperations is false.

My attempts to fix this and have all ops on a newline are breaking inside 
parentheses as well, which we wouldn't want, and adding indentation where there 
shouldn't be. I will need some guidance if this is legitimately a deal-breaker 
😅 

https://github.com/llvm/llvm-project/pull/95013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Add BinPackBinaryOperations configuration (PR #95013)

2024-06-18 Thread Ameer J via cfe-commits



@@ -27492,6 +27492,86 @@ TEST_F(FormatTest, SpaceBetweenKeywordAndLiteral) {
   verifyFormat("return sizeof \"5\";");
 }
 
+TEST_F(FormatTest, BinPackBinaryOperations) {
+  auto Style = getLLVMStyle();
+  Style.BinPackBinaryOperations = false;
+
+  // Logical operations
+  verifyFormat("if (condition1 && condition2) {\n"
+   "}",
+   Style);
+  verifyFormat("if (condition1 && // comment\n"
+   "condition2 &&\n"
+   "(condition3 || condition4) && // comment\n"
+   "condition5 &&\n"
+   "condition6) {\n"
+   "}",
+   Style);
+  verifyFormat("if (loongcondition1 &&\n"
+   "loongcondition2) {\n"
+   "}",
+   Style);
+
+  // Arithmetic
+  verifyFormat("const int result = lhs + rhs;\n", Style);
+  verifyFormat("const int result = loongop1 +\n"
+   "   loongop2 +\n"
+   "   loongop3;\n",
+   Style);
+
+  // clang-format off
+  verifyFormat("const int result =\n"
+   "operand1 + operand2 - (operand3 + operand4) - operand5 * 
operand6;\n",
+   Style);
+  // clang-format on
+
+  verifyFormat("const int result = longOperand1 +\n"
+   "   longOperand2 -\n"
+   "   (longOperand3 + longOperand4) -\n"
+   "   longOperand5 * longOperand6;\n",

ameerj wrote:

@HazardyKnusperkeks Would it be acceptable to specify that this would set every 
operation _of the same precedence_ on individual lines if they fit?
And if users report issues with the way it currently works, then we can look 
into resolving this

https://github.com/llvm/llvm-project/pull/95013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Add BreakBinaryOperations configuration (PR #95013)

2024-07-23 Thread Ameer J via cfe-commits


https://github.com/ameerj updated 
https://github.com/llvm/llvm-project/pull/95013

>From a50f3d4395efd09eea8ba2e750bb785857f9a550 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:09:40 -0400
Subject: [PATCH 01/18] Add BinPackBinaryOperations

---
 clang/include/clang/Format/Format.h   | 16 
 clang/lib/Format/ContinuationIndenter.cpp | 15 +++
 clang/lib/Format/Format.cpp   |  2 ++
 3 files changed, 33 insertions(+)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 4fd6e013df25b..f30d767600061 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1192,6 +1192,21 @@ struct FormatStyle {
   /// \version 3.7
   bool BinPackArguments;
 
+  /// If ``false``, binary operations will either be all on the same line
+  /// or each operation will have one line each.
+  /// \code
+  ///   true:
+  ///    &&  ||
+  /// aaa;
+  ///
+  ///   false:
+  ///    &&
+  ///    ||
+  ///   aaa;
+  /// \endcode
+  /// \version 19
+  bool BinPackBinaryOperations;
+
   /// If ``false``, a function declaration's or function definition's
   /// parameters will either all be on the same line or will have one line 
each.
   /// \code
@@ -4978,6 +4993,7 @@ struct FormatStyle {
R.AlwaysBreakBeforeMultilineStrings &&
AttributeMacros == R.AttributeMacros &&
BinPackArguments == R.BinPackArguments &&
+   BinPackBinaryOperations == R.BinPackBinaryOperations &&
BinPackParameters == R.BinPackParameters &&
BitFieldColonSpacing == R.BitFieldColonSpacing &&
BracedInitializerIndentWidth == R.BracedInitializerIndentWidth &&
diff --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index b07360425ca6e..0780d219b68b4 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -146,6 +146,14 @@ static bool startsNextParameter(const FormatToken &Current,
Style.BreakInheritanceList != FormatStyle::BILS_BeforeComma));
 }
 
+// Returns \c true if \c Current starts a new operand in a binary operation.
+static bool startsNextOperand(const FormatToken &Current,
+  const FormatStyle &Style) {
+  const FormatToken &Previous = *Current.Previous;
+  return Previous.is(TT_BinaryOperator) && !Current.isTrailingComment() &&
+ (Previous.getPrecedence() > prec::Conditional);
+}
+
 static bool opensProtoMessageField(const FormatToken &LessTok,
const FormatStyle &Style) {
   if (LessTok.isNot(tok::less))
@@ -837,6 +845,9 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState 
&State, bool DryRun,
   }
   if (CurrentState.AvoidBinPacking && startsNextParameter(Current, Style))
 CurrentState.NoLineBreak = true;
+  if (!Style.BinPackBinaryOperations && startsNextOperand(Current, Style))
+CurrentState.NoLineBreak = true;
+
   if (startsSegmentOfBuilderTypeCall(Current) &&
   State.Column > getNewLineColumn(State)) {
 CurrentState.ContainsUnwrappedBuilder = true;
@@ -1203,6 +1214,10 @@ unsigned 
ContinuationIndenter::addTokenOnNewLine(LineState &State,
 }
   }
 
+  if (!Style.BinPackBinaryOperations && Previous.is(TT_BinaryOperator) &&
+  (Previous.getPrecedence() > prec::Conditional)) {
+CurrentState.BreakBeforeParameter = true;
+  }
   return Penalty;
 }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index c015e03fa15e7..4d71ce95afe1d 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -935,6 +935,7 @@ template <> struct MappingTraits {
Style.AlwaysBreakBeforeMultilineStrings);
 IO.mapOptional("AttributeMacros", Style.AttributeMacros);
 IO.mapOptional("BinPackArguments", Style.BinPackArguments);
+IO.mapOptional("BinPackBinaryOperations", Style.BinPackBinaryOperations);
 IO.mapOptional("BinPackParameters", Style.BinPackParameters);
 IO.mapOptional("BitFieldColonSpacing", Style.BitFieldColonSpacing);
 IO.mapOptional("BracedInitializerIndentWidth",
@@ -1439,6 +1440,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind 
Language) {
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AttributeMacros.push_back("__capability");
   LLVMStyle.BinPackArguments = true;
+  LLVMStyle.BinPackBinaryOperations = true;
   LLVMStyle.BinPackParameters = true;
   LLVMStyle.BitFieldColonSpacing = FormatStyle::BFCS_Both;
   LLVMStyle.BracedInitializerIndentWidth = std::nullopt;

>From 1fe691c6ee0393cc3f40f0c5e94e01fe79a909c3 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:43:56 -0400
Subject: [PATCH 02/18] Add BinPackBinaryOperations unit tests

---
 clang/unittests/Format/F

[clang] [clang-format] Add BreakBinaryOperations configuration (PR #95013)

2024-07-29 Thread Ameer J via cfe-commits


ameerj wrote:

@owenca Can you review once more please?

https://github.com/llvm/llvm-project/pull/95013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Add BinPackBinaryOperations configuration (PR #95013)

2024-07-17 Thread Ameer J via cfe-commits


https://github.com/ameerj updated 
https://github.com/llvm/llvm-project/pull/95013

>From a50f3d4395efd09eea8ba2e750bb785857f9a550 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:09:40 -0400
Subject: [PATCH 01/15] Add BinPackBinaryOperations

---
 clang/include/clang/Format/Format.h   | 16 
 clang/lib/Format/ContinuationIndenter.cpp | 15 +++
 clang/lib/Format/Format.cpp   |  2 ++
 3 files changed, 33 insertions(+)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 4fd6e013df25b..f30d767600061 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1192,6 +1192,21 @@ struct FormatStyle {
   /// \version 3.7
   bool BinPackArguments;
 
+  /// If ``false``, binary operations will either be all on the same line
+  /// or each operation will have one line each.
+  /// \code
+  ///   true:
+  ///    &&  ||
+  /// aaa;
+  ///
+  ///   false:
+  ///    &&
+  ///    ||
+  ///   aaa;
+  /// \endcode
+  /// \version 19
+  bool BinPackBinaryOperations;
+
   /// If ``false``, a function declaration's or function definition's
   /// parameters will either all be on the same line or will have one line 
each.
   /// \code
@@ -4978,6 +4993,7 @@ struct FormatStyle {
R.AlwaysBreakBeforeMultilineStrings &&
AttributeMacros == R.AttributeMacros &&
BinPackArguments == R.BinPackArguments &&
+   BinPackBinaryOperations == R.BinPackBinaryOperations &&
BinPackParameters == R.BinPackParameters &&
BitFieldColonSpacing == R.BitFieldColonSpacing &&
BracedInitializerIndentWidth == R.BracedInitializerIndentWidth &&
diff --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index b07360425ca6e..0780d219b68b4 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -146,6 +146,14 @@ static bool startsNextParameter(const FormatToken &Current,
Style.BreakInheritanceList != FormatStyle::BILS_BeforeComma));
 }
 
+// Returns \c true if \c Current starts a new operand in a binary operation.
+static bool startsNextOperand(const FormatToken &Current,
+  const FormatStyle &Style) {
+  const FormatToken &Previous = *Current.Previous;
+  return Previous.is(TT_BinaryOperator) && !Current.isTrailingComment() &&
+ (Previous.getPrecedence() > prec::Conditional);
+}
+
 static bool opensProtoMessageField(const FormatToken &LessTok,
const FormatStyle &Style) {
   if (LessTok.isNot(tok::less))
@@ -837,6 +845,9 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState 
&State, bool DryRun,
   }
   if (CurrentState.AvoidBinPacking && startsNextParameter(Current, Style))
 CurrentState.NoLineBreak = true;
+  if (!Style.BinPackBinaryOperations && startsNextOperand(Current, Style))
+CurrentState.NoLineBreak = true;
+
   if (startsSegmentOfBuilderTypeCall(Current) &&
   State.Column > getNewLineColumn(State)) {
 CurrentState.ContainsUnwrappedBuilder = true;
@@ -1203,6 +1214,10 @@ unsigned 
ContinuationIndenter::addTokenOnNewLine(LineState &State,
 }
   }
 
+  if (!Style.BinPackBinaryOperations && Previous.is(TT_BinaryOperator) &&
+  (Previous.getPrecedence() > prec::Conditional)) {
+CurrentState.BreakBeforeParameter = true;
+  }
   return Penalty;
 }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index c015e03fa15e7..4d71ce95afe1d 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -935,6 +935,7 @@ template <> struct MappingTraits {
Style.AlwaysBreakBeforeMultilineStrings);
 IO.mapOptional("AttributeMacros", Style.AttributeMacros);
 IO.mapOptional("BinPackArguments", Style.BinPackArguments);
+IO.mapOptional("BinPackBinaryOperations", Style.BinPackBinaryOperations);
 IO.mapOptional("BinPackParameters", Style.BinPackParameters);
 IO.mapOptional("BitFieldColonSpacing", Style.BitFieldColonSpacing);
 IO.mapOptional("BracedInitializerIndentWidth",
@@ -1439,6 +1440,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind 
Language) {
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AttributeMacros.push_back("__capability");
   LLVMStyle.BinPackArguments = true;
+  LLVMStyle.BinPackBinaryOperations = true;
   LLVMStyle.BinPackParameters = true;
   LLVMStyle.BitFieldColonSpacing = FormatStyle::BFCS_Both;
   LLVMStyle.BracedInitializerIndentWidth = std::nullopt;

>From 1fe691c6ee0393cc3f40f0c5e94e01fe79a909c3 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:43:56 -0400
Subject: [PATCH 02/15] Add BinPackBinaryOperations unit tests

---
 clang/unittests/Format/F

[clang] [clang-format] Add BinPackBinaryOperations configuration (PR #95013)

2024-07-17 Thread Ameer J via cfe-commits


ameerj wrote:

> The logical && and bitwise | operations are not broken up into one per line, 
> and the column limit is exceeded.

@owenca this should be fixed in the latest commit

https://github.com/llvm/llvm-project/pull/95013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Add BinPackBinaryOperations configuration (PR #95013)

2024-07-17 Thread Ameer J via cfe-commits

ameerj wrote:

> It seems 
> [AlignOperands](https://clang.llvm.org/docs/ClangFormatStyleOptions.html#alignoperands)
>  is supposed to do what this new option would, so maybe we should fix/extend 
> `AlignOperands` instead?

This new setting is only responsible for breaking long expressions, not 
aligning.
`AlignOperands` can be used in conjunction with this new setting for different 
alignment styles.

https://github.com/llvm/llvm-project/pull/95013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Add BinPackBinaryOperations configuration (PR #95013)

2024-07-17 Thread Ameer J via cfe-commits


https://github.com/ameerj updated 
https://github.com/llvm/llvm-project/pull/95013

>From a50f3d4395efd09eea8ba2e750bb785857f9a550 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:09:40 -0400
Subject: [PATCH 01/16] Add BinPackBinaryOperations

---
 clang/include/clang/Format/Format.h   | 16 
 clang/lib/Format/ContinuationIndenter.cpp | 15 +++
 clang/lib/Format/Format.cpp   |  2 ++
 3 files changed, 33 insertions(+)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 4fd6e013df25b..f30d767600061 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1192,6 +1192,21 @@ struct FormatStyle {
   /// \version 3.7
   bool BinPackArguments;
 
+  /// If ``false``, binary operations will either be all on the same line
+  /// or each operation will have one line each.
+  /// \code
+  ///   true:
+  ///    &&  ||
+  /// aaa;
+  ///
+  ///   false:
+  ///    &&
+  ///    ||
+  ///   aaa;
+  /// \endcode
+  /// \version 19
+  bool BinPackBinaryOperations;
+
   /// If ``false``, a function declaration's or function definition's
   /// parameters will either all be on the same line or will have one line 
each.
   /// \code
@@ -4978,6 +4993,7 @@ struct FormatStyle {
R.AlwaysBreakBeforeMultilineStrings &&
AttributeMacros == R.AttributeMacros &&
BinPackArguments == R.BinPackArguments &&
+   BinPackBinaryOperations == R.BinPackBinaryOperations &&
BinPackParameters == R.BinPackParameters &&
BitFieldColonSpacing == R.BitFieldColonSpacing &&
BracedInitializerIndentWidth == R.BracedInitializerIndentWidth &&
diff --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index b07360425ca6e..0780d219b68b4 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -146,6 +146,14 @@ static bool startsNextParameter(const FormatToken &Current,
Style.BreakInheritanceList != FormatStyle::BILS_BeforeComma));
 }
 
+// Returns \c true if \c Current starts a new operand in a binary operation.
+static bool startsNextOperand(const FormatToken &Current,
+  const FormatStyle &Style) {
+  const FormatToken &Previous = *Current.Previous;
+  return Previous.is(TT_BinaryOperator) && !Current.isTrailingComment() &&
+ (Previous.getPrecedence() > prec::Conditional);
+}
+
 static bool opensProtoMessageField(const FormatToken &LessTok,
const FormatStyle &Style) {
   if (LessTok.isNot(tok::less))
@@ -837,6 +845,9 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState 
&State, bool DryRun,
   }
   if (CurrentState.AvoidBinPacking && startsNextParameter(Current, Style))
 CurrentState.NoLineBreak = true;
+  if (!Style.BinPackBinaryOperations && startsNextOperand(Current, Style))
+CurrentState.NoLineBreak = true;
+
   if (startsSegmentOfBuilderTypeCall(Current) &&
   State.Column > getNewLineColumn(State)) {
 CurrentState.ContainsUnwrappedBuilder = true;
@@ -1203,6 +1214,10 @@ unsigned 
ContinuationIndenter::addTokenOnNewLine(LineState &State,
 }
   }
 
+  if (!Style.BinPackBinaryOperations && Previous.is(TT_BinaryOperator) &&
+  (Previous.getPrecedence() > prec::Conditional)) {
+CurrentState.BreakBeforeParameter = true;
+  }
   return Penalty;
 }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index c015e03fa15e7..4d71ce95afe1d 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -935,6 +935,7 @@ template <> struct MappingTraits {
Style.AlwaysBreakBeforeMultilineStrings);
 IO.mapOptional("AttributeMacros", Style.AttributeMacros);
 IO.mapOptional("BinPackArguments", Style.BinPackArguments);
+IO.mapOptional("BinPackBinaryOperations", Style.BinPackBinaryOperations);
 IO.mapOptional("BinPackParameters", Style.BinPackParameters);
 IO.mapOptional("BitFieldColonSpacing", Style.BitFieldColonSpacing);
 IO.mapOptional("BracedInitializerIndentWidth",
@@ -1439,6 +1440,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind 
Language) {
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AttributeMacros.push_back("__capability");
   LLVMStyle.BinPackArguments = true;
+  LLVMStyle.BinPackBinaryOperations = true;
   LLVMStyle.BinPackParameters = true;
   LLVMStyle.BitFieldColonSpacing = FormatStyle::BFCS_Both;
   LLVMStyle.BracedInitializerIndentWidth = std::nullopt;

>From 1fe691c6ee0393cc3f40f0c5e94e01fe79a909c3 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:43:56 -0400
Subject: [PATCH 02/16] Add BinPackBinaryOperations unit tests

---
 clang/unittests/Format/F

[clang] [clang-format] Add BinPackBinaryOperations configuration (PR #95013)

2024-07-17 Thread Ameer J via cfe-commits


ameerj wrote:

> I'm not finding the true/false every clear, can we use an enum?

Reworked this to now be an enum. Please let me know if the intent is more clear 
now or if anything needs further clarification.

https://github.com/llvm/llvm-project/pull/95013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Add BreakBinaryOperations configuration (PR #95013)

2024-07-17 Thread Ameer J via cfe-commits


https://github.com/ameerj edited https://github.com/llvm/llvm-project/pull/95013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Add BreakBinaryOperations configuration (PR #95013)

2024-07-17 Thread Ameer J via cfe-commits


https://github.com/ameerj edited https://github.com/llvm/llvm-project/pull/95013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Add BreakBinaryOperations configuration (PR #95013)

2024-07-22 Thread Ameer J via cfe-commits


https://github.com/ameerj updated 
https://github.com/llvm/llvm-project/pull/95013

>From a50f3d4395efd09eea8ba2e750bb785857f9a550 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:09:40 -0400
Subject: [PATCH 01/17] Add BinPackBinaryOperations

---
 clang/include/clang/Format/Format.h   | 16 
 clang/lib/Format/ContinuationIndenter.cpp | 15 +++
 clang/lib/Format/Format.cpp   |  2 ++
 3 files changed, 33 insertions(+)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 4fd6e013df25b..f30d767600061 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1192,6 +1192,21 @@ struct FormatStyle {
   /// \version 3.7
   bool BinPackArguments;
 
+  /// If ``false``, binary operations will either be all on the same line
+  /// or each operation will have one line each.
+  /// \code
+  ///   true:
+  ///    &&  ||
+  /// aaa;
+  ///
+  ///   false:
+  ///    &&
+  ///    ||
+  ///   aaa;
+  /// \endcode
+  /// \version 19
+  bool BinPackBinaryOperations;
+
   /// If ``false``, a function declaration's or function definition's
   /// parameters will either all be on the same line or will have one line 
each.
   /// \code
@@ -4978,6 +4993,7 @@ struct FormatStyle {
R.AlwaysBreakBeforeMultilineStrings &&
AttributeMacros == R.AttributeMacros &&
BinPackArguments == R.BinPackArguments &&
+   BinPackBinaryOperations == R.BinPackBinaryOperations &&
BinPackParameters == R.BinPackParameters &&
BitFieldColonSpacing == R.BitFieldColonSpacing &&
BracedInitializerIndentWidth == R.BracedInitializerIndentWidth &&
diff --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index b07360425ca6e..0780d219b68b4 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -146,6 +146,14 @@ static bool startsNextParameter(const FormatToken &Current,
Style.BreakInheritanceList != FormatStyle::BILS_BeforeComma));
 }
 
+// Returns \c true if \c Current starts a new operand in a binary operation.
+static bool startsNextOperand(const FormatToken &Current,
+  const FormatStyle &Style) {
+  const FormatToken &Previous = *Current.Previous;
+  return Previous.is(TT_BinaryOperator) && !Current.isTrailingComment() &&
+ (Previous.getPrecedence() > prec::Conditional);
+}
+
 static bool opensProtoMessageField(const FormatToken &LessTok,
const FormatStyle &Style) {
   if (LessTok.isNot(tok::less))
@@ -837,6 +845,9 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState 
&State, bool DryRun,
   }
   if (CurrentState.AvoidBinPacking && startsNextParameter(Current, Style))
 CurrentState.NoLineBreak = true;
+  if (!Style.BinPackBinaryOperations && startsNextOperand(Current, Style))
+CurrentState.NoLineBreak = true;
+
   if (startsSegmentOfBuilderTypeCall(Current) &&
   State.Column > getNewLineColumn(State)) {
 CurrentState.ContainsUnwrappedBuilder = true;
@@ -1203,6 +1214,10 @@ unsigned 
ContinuationIndenter::addTokenOnNewLine(LineState &State,
 }
   }
 
+  if (!Style.BinPackBinaryOperations && Previous.is(TT_BinaryOperator) &&
+  (Previous.getPrecedence() > prec::Conditional)) {
+CurrentState.BreakBeforeParameter = true;
+  }
   return Penalty;
 }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index c015e03fa15e7..4d71ce95afe1d 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -935,6 +935,7 @@ template <> struct MappingTraits {
Style.AlwaysBreakBeforeMultilineStrings);
 IO.mapOptional("AttributeMacros", Style.AttributeMacros);
 IO.mapOptional("BinPackArguments", Style.BinPackArguments);
+IO.mapOptional("BinPackBinaryOperations", Style.BinPackBinaryOperations);
 IO.mapOptional("BinPackParameters", Style.BinPackParameters);
 IO.mapOptional("BitFieldColonSpacing", Style.BitFieldColonSpacing);
 IO.mapOptional("BracedInitializerIndentWidth",
@@ -1439,6 +1440,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind 
Language) {
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AttributeMacros.push_back("__capability");
   LLVMStyle.BinPackArguments = true;
+  LLVMStyle.BinPackBinaryOperations = true;
   LLVMStyle.BinPackParameters = true;
   LLVMStyle.BitFieldColonSpacing = FormatStyle::BFCS_Both;
   LLVMStyle.BracedInitializerIndentWidth = std::nullopt;

>From 1fe691c6ee0393cc3f40f0c5e94e01fe79a909c3 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:43:56 -0400
Subject: [PATCH 02/17] Add BinPackBinaryOperations unit tests

---
 clang/unittests/Format/F

[clang] [clang-format] Add BreakBinaryOperations configuration (PR #95013)

2024-07-22 Thread Ameer J via cfe-commits


ameerj wrote:

> Failed Tests (1):

Fixed in the latest commit

https://github.com/llvm/llvm-project/pull/95013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Add BinPackBinaryOperations configuration (PR #95013)

2024-07-09 Thread Ameer J via cfe-commits


ameerj wrote:

Thanks for the review and approval @HazardyKnusperkeks !

I don't have write permissions to the repo so please merge this for me when you 
have a moment.

https://github.com/llvm/llvm-project/pull/95013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Add BinPackBinaryOperations configuration (PR #95013)

2024-07-10 Thread Ameer J via cfe-commits


https://github.com/ameerj updated 
https://github.com/llvm/llvm-project/pull/95013

>From a50f3d4395efd09eea8ba2e750bb785857f9a550 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:09:40 -0400
Subject: [PATCH 01/11] Add BinPackBinaryOperations

---
 clang/include/clang/Format/Format.h   | 16 
 clang/lib/Format/ContinuationIndenter.cpp | 15 +++
 clang/lib/Format/Format.cpp   |  2 ++
 3 files changed, 33 insertions(+)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 4fd6e013df25b..f30d767600061 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1192,6 +1192,21 @@ struct FormatStyle {
   /// \version 3.7
   bool BinPackArguments;
 
+  /// If ``false``, binary operations will either be all on the same line
+  /// or each operation will have one line each.
+  /// \code
+  ///   true:
+  ///    &&  ||
+  /// aaa;
+  ///
+  ///   false:
+  ///    &&
+  ///    ||
+  ///   aaa;
+  /// \endcode
+  /// \version 19
+  bool BinPackBinaryOperations;
+
   /// If ``false``, a function declaration's or function definition's
   /// parameters will either all be on the same line or will have one line 
each.
   /// \code
@@ -4978,6 +4993,7 @@ struct FormatStyle {
R.AlwaysBreakBeforeMultilineStrings &&
AttributeMacros == R.AttributeMacros &&
BinPackArguments == R.BinPackArguments &&
+   BinPackBinaryOperations == R.BinPackBinaryOperations &&
BinPackParameters == R.BinPackParameters &&
BitFieldColonSpacing == R.BitFieldColonSpacing &&
BracedInitializerIndentWidth == R.BracedInitializerIndentWidth &&
diff --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index b07360425ca6e..0780d219b68b4 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -146,6 +146,14 @@ static bool startsNextParameter(const FormatToken &Current,
Style.BreakInheritanceList != FormatStyle::BILS_BeforeComma));
 }
 
+// Returns \c true if \c Current starts a new operand in a binary operation.
+static bool startsNextOperand(const FormatToken &Current,
+  const FormatStyle &Style) {
+  const FormatToken &Previous = *Current.Previous;
+  return Previous.is(TT_BinaryOperator) && !Current.isTrailingComment() &&
+ (Previous.getPrecedence() > prec::Conditional);
+}
+
 static bool opensProtoMessageField(const FormatToken &LessTok,
const FormatStyle &Style) {
   if (LessTok.isNot(tok::less))
@@ -837,6 +845,9 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState 
&State, bool DryRun,
   }
   if (CurrentState.AvoidBinPacking && startsNextParameter(Current, Style))
 CurrentState.NoLineBreak = true;
+  if (!Style.BinPackBinaryOperations && startsNextOperand(Current, Style))
+CurrentState.NoLineBreak = true;
+
   if (startsSegmentOfBuilderTypeCall(Current) &&
   State.Column > getNewLineColumn(State)) {
 CurrentState.ContainsUnwrappedBuilder = true;
@@ -1203,6 +1214,10 @@ unsigned 
ContinuationIndenter::addTokenOnNewLine(LineState &State,
 }
   }
 
+  if (!Style.BinPackBinaryOperations && Previous.is(TT_BinaryOperator) &&
+  (Previous.getPrecedence() > prec::Conditional)) {
+CurrentState.BreakBeforeParameter = true;
+  }
   return Penalty;
 }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index c015e03fa15e7..4d71ce95afe1d 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -935,6 +935,7 @@ template <> struct MappingTraits {
Style.AlwaysBreakBeforeMultilineStrings);
 IO.mapOptional("AttributeMacros", Style.AttributeMacros);
 IO.mapOptional("BinPackArguments", Style.BinPackArguments);
+IO.mapOptional("BinPackBinaryOperations", Style.BinPackBinaryOperations);
 IO.mapOptional("BinPackParameters", Style.BinPackParameters);
 IO.mapOptional("BitFieldColonSpacing", Style.BitFieldColonSpacing);
 IO.mapOptional("BracedInitializerIndentWidth",
@@ -1439,6 +1440,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind 
Language) {
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AttributeMacros.push_back("__capability");
   LLVMStyle.BinPackArguments = true;
+  LLVMStyle.BinPackBinaryOperations = true;
   LLVMStyle.BinPackParameters = true;
   LLVMStyle.BitFieldColonSpacing = FormatStyle::BFCS_Both;
   LLVMStyle.BracedInitializerIndentWidth = std::nullopt;

>From 1fe691c6ee0393cc3f40f0c5e94e01fe79a909c3 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:43:56 -0400
Subject: [PATCH 02/11] Add BinPackBinaryOperations unit tests

---
 clang/unittests/Format/F

[clang] [clang-format] Add BinPackBinaryOperations configuration (PR #95013)

2024-07-10 Thread Ameer J via cfe-commits



@@ -146,6 +146,14 @@ static bool startsNextParameter(const FormatToken &Current,
Style.BreakInheritanceList != FormatStyle::BILS_BeforeComma));
 }
 
+// Returns \c true if \c Current starts a new operand in a binary operation.

ameerj wrote:

The wording was meant to match the comment for `startsNextParameter`, but I 
changed the wording to clarify a bit. 

In a case like this: `op1 * (op2 + op3)`
The function would return true for `(` since that's what starts the next 
operand for the `*` operation.
It would be a bit inaccurate to say `(` "`is the right operand of a binary 
operator`"

https://github.com/llvm/llvm-project/pull/95013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Add BinPackBinaryOperations configuration (PR #95013)

2024-07-15 Thread Ameer J via cfe-commits


https://github.com/ameerj updated 
https://github.com/llvm/llvm-project/pull/95013

>From a50f3d4395efd09eea8ba2e750bb785857f9a550 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:09:40 -0400
Subject: [PATCH 01/14] Add BinPackBinaryOperations

---
 clang/include/clang/Format/Format.h   | 16 
 clang/lib/Format/ContinuationIndenter.cpp | 15 +++
 clang/lib/Format/Format.cpp   |  2 ++
 3 files changed, 33 insertions(+)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 4fd6e013df25b..f30d767600061 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1192,6 +1192,21 @@ struct FormatStyle {
   /// \version 3.7
   bool BinPackArguments;
 
+  /// If ``false``, binary operations will either be all on the same line
+  /// or each operation will have one line each.
+  /// \code
+  ///   true:
+  ///    &&  ||
+  /// aaa;
+  ///
+  ///   false:
+  ///    &&
+  ///    ||
+  ///   aaa;
+  /// \endcode
+  /// \version 19
+  bool BinPackBinaryOperations;
+
   /// If ``false``, a function declaration's or function definition's
   /// parameters will either all be on the same line or will have one line 
each.
   /// \code
@@ -4978,6 +4993,7 @@ struct FormatStyle {
R.AlwaysBreakBeforeMultilineStrings &&
AttributeMacros == R.AttributeMacros &&
BinPackArguments == R.BinPackArguments &&
+   BinPackBinaryOperations == R.BinPackBinaryOperations &&
BinPackParameters == R.BinPackParameters &&
BitFieldColonSpacing == R.BitFieldColonSpacing &&
BracedInitializerIndentWidth == R.BracedInitializerIndentWidth &&
diff --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index b07360425ca6e..0780d219b68b4 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -146,6 +146,14 @@ static bool startsNextParameter(const FormatToken &Current,
Style.BreakInheritanceList != FormatStyle::BILS_BeforeComma));
 }
 
+// Returns \c true if \c Current starts a new operand in a binary operation.
+static bool startsNextOperand(const FormatToken &Current,
+  const FormatStyle &Style) {
+  const FormatToken &Previous = *Current.Previous;
+  return Previous.is(TT_BinaryOperator) && !Current.isTrailingComment() &&
+ (Previous.getPrecedence() > prec::Conditional);
+}
+
 static bool opensProtoMessageField(const FormatToken &LessTok,
const FormatStyle &Style) {
   if (LessTok.isNot(tok::less))
@@ -837,6 +845,9 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState 
&State, bool DryRun,
   }
   if (CurrentState.AvoidBinPacking && startsNextParameter(Current, Style))
 CurrentState.NoLineBreak = true;
+  if (!Style.BinPackBinaryOperations && startsNextOperand(Current, Style))
+CurrentState.NoLineBreak = true;
+
   if (startsSegmentOfBuilderTypeCall(Current) &&
   State.Column > getNewLineColumn(State)) {
 CurrentState.ContainsUnwrappedBuilder = true;
@@ -1203,6 +1214,10 @@ unsigned 
ContinuationIndenter::addTokenOnNewLine(LineState &State,
 }
   }
 
+  if (!Style.BinPackBinaryOperations && Previous.is(TT_BinaryOperator) &&
+  (Previous.getPrecedence() > prec::Conditional)) {
+CurrentState.BreakBeforeParameter = true;
+  }
   return Penalty;
 }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index c015e03fa15e7..4d71ce95afe1d 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -935,6 +935,7 @@ template <> struct MappingTraits {
Style.AlwaysBreakBeforeMultilineStrings);
 IO.mapOptional("AttributeMacros", Style.AttributeMacros);
 IO.mapOptional("BinPackArguments", Style.BinPackArguments);
+IO.mapOptional("BinPackBinaryOperations", Style.BinPackBinaryOperations);
 IO.mapOptional("BinPackParameters", Style.BinPackParameters);
 IO.mapOptional("BitFieldColonSpacing", Style.BitFieldColonSpacing);
 IO.mapOptional("BracedInitializerIndentWidth",
@@ -1439,6 +1440,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind 
Language) {
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AttributeMacros.push_back("__capability");
   LLVMStyle.BinPackArguments = true;
+  LLVMStyle.BinPackBinaryOperations = true;
   LLVMStyle.BinPackParameters = true;
   LLVMStyle.BitFieldColonSpacing = FormatStyle::BFCS_Both;
   LLVMStyle.BracedInitializerIndentWidth = std::nullopt;

>From 1fe691c6ee0393cc3f40f0c5e94e01fe79a909c3 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:43:56 -0400
Subject: [PATCH 02/14] Add BinPackBinaryOperations unit tests

---
 clang/unittests/Format/F

[clang] [clang-format] Add BinPackBinaryOperations configuration (PR #95013)

2024-07-15 Thread Ameer J via cfe-commits



@@ -27628,6 +27628,119 @@ TEST_F(FormatTest, SpaceBetweenKeywordAndLiteral) {
   verifyFormat("return sizeof \"5\";");
 }
 
+TEST_F(FormatTest, BinPackBinaryOperations) {
+  auto Style = getLLVMStyleWithColumns(60);
+  // Logical operations
+  verifyFormat("if (condition1 && condition2) {\n"
+   "}",
+   Style);
+
+  verifyFormat("if (condition1 && condition2 &&\n"
+   "(condition3 || condition4) && condition5 &&\n"
+   "condition6) {\n"
+   "}",
+   Style);
+
+  verifyFormat("if (loongcondition1 &&\n"
+   "loongcondition2) {\n"
+   "}",
+   Style);
+
+  // Arithmetic
+  verifyFormat("const int result = lhs + rhs;", Style);
+
+  verifyFormat("const int result = lngop1 + longop2 +\n"
+   "   loongop3;",
+   Style);
+
+  verifyFormat("result = longOperand1 + longOperand2 -\n"
+   " (longOperand3 + longOperand4) -\n"
+   " longOperand5 * longOperand6;",
+   Style);
+
+  verifyFormat("const int result =\n"
+   "operand1 + operand2 - (operand3 + operand4);\n",
+   Style);
+
+  Style.BinPackBinaryOperations = false;
+
+  // Logical operations
+  verifyFormat("if (condition1 && condition2) {\n"
+   "}",
+   Style);
+
+  verifyFormat("if (condition1 && // comment\n"
+   "condition2 &&\n"
+   "(condition3 || condition4) && // comment\n"
+   "condition5 &&\n"
+   "condition6) {\n"
+   "}",
+   Style);
+
+  verifyFormat("if (loongcondition1 &&\n"
+   "loongcondition2) {\n"
+   "}",
+   Style);
+
+  // Arithmetic
+  verifyFormat("const int result = lhs + rhs;\n", Style);

ameerj wrote:

Fixed, thanks for the through review!

https://github.com/llvm/llvm-project/pull/95013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Add BreakBinaryOperations configuration (PR #95013)

2024-08-05 Thread Ameer J via cfe-commits


https://github.com/ameerj updated 
https://github.com/llvm/llvm-project/pull/95013

>From a50f3d4395efd09eea8ba2e750bb785857f9a550 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:09:40 -0400
Subject: [PATCH 01/19] Add BinPackBinaryOperations

---
 clang/include/clang/Format/Format.h   | 16 
 clang/lib/Format/ContinuationIndenter.cpp | 15 +++
 clang/lib/Format/Format.cpp   |  2 ++
 3 files changed, 33 insertions(+)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 4fd6e013df25b..f30d767600061 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1192,6 +1192,21 @@ struct FormatStyle {
   /// \version 3.7
   bool BinPackArguments;
 
+  /// If ``false``, binary operations will either be all on the same line
+  /// or each operation will have one line each.
+  /// \code
+  ///   true:
+  ///    &&  ||
+  /// aaa;
+  ///
+  ///   false:
+  ///    &&
+  ///    ||
+  ///   aaa;
+  /// \endcode
+  /// \version 19
+  bool BinPackBinaryOperations;
+
   /// If ``false``, a function declaration's or function definition's
   /// parameters will either all be on the same line or will have one line 
each.
   /// \code
@@ -4978,6 +4993,7 @@ struct FormatStyle {
R.AlwaysBreakBeforeMultilineStrings &&
AttributeMacros == R.AttributeMacros &&
BinPackArguments == R.BinPackArguments &&
+   BinPackBinaryOperations == R.BinPackBinaryOperations &&
BinPackParameters == R.BinPackParameters &&
BitFieldColonSpacing == R.BitFieldColonSpacing &&
BracedInitializerIndentWidth == R.BracedInitializerIndentWidth &&
diff --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index b07360425ca6e..0780d219b68b4 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -146,6 +146,14 @@ static bool startsNextParameter(const FormatToken &Current,
Style.BreakInheritanceList != FormatStyle::BILS_BeforeComma));
 }
 
+// Returns \c true if \c Current starts a new operand in a binary operation.
+static bool startsNextOperand(const FormatToken &Current,
+  const FormatStyle &Style) {
+  const FormatToken &Previous = *Current.Previous;
+  return Previous.is(TT_BinaryOperator) && !Current.isTrailingComment() &&
+ (Previous.getPrecedence() > prec::Conditional);
+}
+
 static bool opensProtoMessageField(const FormatToken &LessTok,
const FormatStyle &Style) {
   if (LessTok.isNot(tok::less))
@@ -837,6 +845,9 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState 
&State, bool DryRun,
   }
   if (CurrentState.AvoidBinPacking && startsNextParameter(Current, Style))
 CurrentState.NoLineBreak = true;
+  if (!Style.BinPackBinaryOperations && startsNextOperand(Current, Style))
+CurrentState.NoLineBreak = true;
+
   if (startsSegmentOfBuilderTypeCall(Current) &&
   State.Column > getNewLineColumn(State)) {
 CurrentState.ContainsUnwrappedBuilder = true;
@@ -1203,6 +1214,10 @@ unsigned 
ContinuationIndenter::addTokenOnNewLine(LineState &State,
 }
   }
 
+  if (!Style.BinPackBinaryOperations && Previous.is(TT_BinaryOperator) &&
+  (Previous.getPrecedence() > prec::Conditional)) {
+CurrentState.BreakBeforeParameter = true;
+  }
   return Penalty;
 }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index c015e03fa15e7..4d71ce95afe1d 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -935,6 +935,7 @@ template <> struct MappingTraits {
Style.AlwaysBreakBeforeMultilineStrings);
 IO.mapOptional("AttributeMacros", Style.AttributeMacros);
 IO.mapOptional("BinPackArguments", Style.BinPackArguments);
+IO.mapOptional("BinPackBinaryOperations", Style.BinPackBinaryOperations);
 IO.mapOptional("BinPackParameters", Style.BinPackParameters);
 IO.mapOptional("BitFieldColonSpacing", Style.BitFieldColonSpacing);
 IO.mapOptional("BracedInitializerIndentWidth",
@@ -1439,6 +1440,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind 
Language) {
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AttributeMacros.push_back("__capability");
   LLVMStyle.BinPackArguments = true;
+  LLVMStyle.BinPackBinaryOperations = true;
   LLVMStyle.BinPackParameters = true;
   LLVMStyle.BitFieldColonSpacing = FormatStyle::BFCS_Both;
   LLVMStyle.BracedInitializerIndentWidth = std::nullopt;

>From 1fe691c6ee0393cc3f40f0c5e94e01fe79a909c3 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 10 Jun 2024 12:43:56 -0400
Subject: [PATCH 02/19] Add BinPackBinaryOperations unit tests

---
 clang/unittests/Format/F

[clang] [clang-format] Add BreakBinaryOperations configuration (PR #95013)

2024-08-05 Thread Ameer J via cfe-commits


https://github.com/ameerj edited https://github.com/llvm/llvm-project/pull/95013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] clang-format-ignore: Add support for double asterisk patterns (PR #110560)

2024-10-11 Thread Ameer J via cfe-commits


ameerj wrote:

@mydeveloperday @owenca 
bump for review feedback

https://github.com/llvm/llvm-project/pull/110560
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] clang-format-ignore: Add support for double asterisk patterns (PR #110560)

2024-10-04 Thread Ameer J via cfe-commits



@@ -164,6 +164,40 @@ TEST_F(MatchFilePathTest, Path) {
   EXPECT_FALSE(match("foo\\", R"(foo*\)"));
 }
 
+TEST_F(MatchFilePathTest, DoubleAsterisk) {
+  EXPECT_TRUE(match("a/b/c/d.cpp", "**b**"));
+  EXPECT_TRUE(match("a/b/c/d.cpp", "**/b/**"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "**d_*"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "**/d_*"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "**d_**"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "**/d_**"));
+
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "**/b/c/**"));
+
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "a/**/b/c/d_e.cpp"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "a/**/c/d_e.cpp"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "a/**/b/**/d_e.cpp"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "**/b/**/d_e.cpp"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "a/**/**/b/**"));
+
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "**/d"));
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "**/b/d"));
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "**/b/d/**"));
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "**/b/c/"));
+
+  // Multiple consecutive asterisks are treated as **
+  EXPECT_TRUE(match("a/b/c/d.cpp", "***b"));
+  EXPECT_TRUE(match("a/b/c/d.cpp", "/b/***"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "***d_**"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "/d_*"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "***/b/c/*"));
+
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "*/d"));
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "***/b/d"));
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "*/b/d/***"));
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "***/b/c"));

ameerj wrote:

> should there be some tests with repetition?

I'm not sure I understand what you are asking.
Can you provide examples of the path and the ignore pattern that you expect to 
be problematic?

> if we are doing this an saying "Its like .gitignore" should we support

I am only indicating that the `**` glob pattern is implemented in .gitignore, 
and we follow the same rules they do for this specific pattern.

I do not think the current implementation could easily support theese glob 
patterns the same way .gitignore does. 
My understanding is that there is some "state" where a lower level `!` negates 
an earlier match/ignore?
I think the current implementation only considers one pattern in the file at a 
time, in isolation of all other lines in the .clang-format-ignore file.

@owenca may understand better and have an opinion.

https://github.com/llvm/llvm-project/pull/110560
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] clang-format-ignore: Add support for double asterisk patterns (PR #110560)

2024-11-08 Thread Ameer J via cfe-commits



@@ -164,6 +164,40 @@ TEST_F(MatchFilePathTest, Path) {
   EXPECT_FALSE(match("foo\\", R"(foo*\)"));
 }
 
+TEST_F(MatchFilePathTest, DoubleAsterisk) {
+  EXPECT_TRUE(match("a/b/c/d.cpp", "**b**"));
+  EXPECT_TRUE(match("a/b/c/d.cpp", "**/b/**"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "**d_*"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "**/d_*"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "**d_**"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "**/d_**"));
+
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "**/b/c/**"));
+
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "a/**/b/c/d_e.cpp"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "a/**/c/d_e.cpp"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "a/**/b/**/d_e.cpp"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "**/b/**/d_e.cpp"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "a/**/**/b/**"));
+
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "**/d"));
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "**/b/d"));
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "**/b/d/**"));
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "**/b/c/"));
+
+  // Multiple consecutive asterisks are treated as **
+  EXPECT_TRUE(match("a/b/c/d.cpp", "***b"));
+  EXPECT_TRUE(match("a/b/c/d.cpp", "/b/***"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "***d_**"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "/d_*"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "***/b/c/*"));
+
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "*/d"));
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "***/b/d"));
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "*/b/d/***"));
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "***/b/c"));

ameerj wrote:

> How did you manage before clang-format gained the .clang-format-ignore 
> functionality?

PAL has not used clang-format at all, my contributions have been an effort to 
support the PAL formatting style so we can start to use the tool.

> > * Simplifying Subdirectory Exclusion: `**` allows for easy exclusion of 
> > entire subdirectories. Currently you need to specify each depth level in a 
> > subdirectory individually.
> 
> We have `DisableFormat` for that.

I was not aware of `DisableFormat`, it's not as intuitive as having everything 
in the ignore file but it can be a compromise if you're completely opposed to 
adding the `**` change. 


> > * Pattern-Based File Ignoring: `**` simplifies ignoring specific file 
> > patterns that may appear at any level within a directory tree, which is 
> > especially useful when the exact depth of these files can vary
> 
> I was conscious of this use case, but you can do something like `find . -name 
> dont-format-me >> .clang-format-ignore` even though it's less than ideal.

Like you said, this is not ideal as it relies on anyone adding/moving files 
that may need to be ignored to regenerate the clang-format-ignore file.

`**` could handle these cases without a need for manual intervention.

> If we were to add `**` to match gitignore, we should also handle it at the 
> beginning and the end of a pathname.

Are you seeing a gap in the unit tests I have? I think I have these cases 
covered but can add more specific unit tests if you have an example.

https://github.com/llvm/llvm-project/pull/110560
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] clang-format-ignore: Add support for double asterisk patterns (PR #110560)

2024-10-23 Thread Ameer J via cfe-commits


ameerj wrote:

@HazardyKnusperkeks Can I get your eyes on this PR please?

https://github.com/llvm/llvm-project/pull/110560
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] clang-format-ignore: Add support for double asterisk patterns (PR #110560)

2024-09-30 Thread Ameer J via cfe-commits


https://github.com/ameerj created 
https://github.com/llvm/llvm-project/pull/110560

Adds `**` support for `.clang-format-ignore` to support similar pattern 
matching to 
[.gitignore](https://mirrors.edge.kernel.org/pub/software/scm/git/docs/gitignore.html#_pattern_format)

closes #110160

>From bf284fe6a25f7947d73b9df12221cefb5363db64 Mon Sep 17 00:00:00 2001
From: ameerj 
Date: Mon, 30 Sep 2024 15:45:40 -0400
Subject: [PATCH] MatchFilePath: Add support for double asterisk patterns

---
 clang/lib/Format/MatchFilePath.cpp   | 22 +++--
 clang/unittests/Format/MatchFilePathTest.cpp | 34 
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Format/MatchFilePath.cpp 
b/clang/lib/Format/MatchFilePath.cpp
index 062b334dcdd8fd..3d7861499b9a46 100644
--- a/clang/lib/Format/MatchFilePath.cpp
+++ b/clang/lib/Format/MatchFilePath.cpp
@@ -49,11 +49,29 @@ bool matchFilePath(StringRef Pattern, StringRef FilePath) {
 return false;
   break;
 case '*': {
-  while (++I < EOP && Pattern[I] == '*') { // Skip consecutive stars.
+  if (I + 1 < EOP && Pattern[I + 1] == '*') {
+// Handle '**' pattern
+while (++I < EOP && Pattern[I] == '*') { // Skip consecutive stars.
+}
+if (I == EOP)
+  return true; // '**' at the end matches everything
+if (Pattern[I] == Separator) {
+  // Try to match the rest of the pattern without consuming the
+  // separator for the case where we want to match "zero" directories
+  // e.g. "a/**/b" matches "a/b"
+  if (matchFilePath(Pattern.substr(I + 1), FilePath.substr(J)))
+return true;
+}
+while (J < End) {
+  if (matchFilePath(Pattern.substr(I), FilePath.substr(J)))
+return true;
+  ++J;
+}
+return false;
   }
   const auto K = FilePath.find(Separator, J); // Index of next `Separator`.
   const bool NoMoreSeparatorsInFilePath = K == StringRef::npos;
-  if (I == EOP) // `Pattern` ends with a star.
+  if (++I == EOP) // `Pattern` ends with a star.
 return NoMoreSeparatorsInFilePath;
   // `Pattern` ends with a lone backslash.
   if (Pattern[I] == '\\' && ++I == EOP)
diff --git a/clang/unittests/Format/MatchFilePathTest.cpp 
b/clang/unittests/Format/MatchFilePathTest.cpp
index 28f665635718e5..a6df090a802128 100644
--- a/clang/unittests/Format/MatchFilePathTest.cpp
+++ b/clang/unittests/Format/MatchFilePathTest.cpp
@@ -164,6 +164,40 @@ TEST_F(MatchFilePathTest, Path) {
   EXPECT_FALSE(match("foo\\", R"(foo*\)"));
 }
 
+TEST_F(MatchFilePathTest, DoubleAsterisk) {
+  EXPECT_TRUE(match("a/b/c/d.cpp", "**b**"));
+  EXPECT_TRUE(match("a/b/c/d.cpp", "**/b/**"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "**d_*"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "**/d_*"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "**d_**"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "**/d_**"));
+
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "**/b/c/**"));
+
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "a/**/b/c/d_e.cpp"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "a/**/c/d_e.cpp"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "a/**/b/**/d_e.cpp"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "**/b/**/d_e.cpp"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "a/**/**/b/**"));
+
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "**/d"));
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "**/b/d"));
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "**/b/d/**"));
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "**/b/c/"));
+
+  // Multiple consecutive asterisks are treated as **
+  EXPECT_TRUE(match("a/b/c/d.cpp", "***b"));
+  EXPECT_TRUE(match("a/b/c/d.cpp", "/b/***"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "***d_**"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "/d_*"));
+  EXPECT_TRUE(match("a/b/c/d_e.cpp", "***/b/c/*"));
+
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "*/d"));
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "***/b/d"));
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "*/b/d/***"));
+  EXPECT_FALSE(match("a/b/c/d_e.cpp", "***/b/c"));
+}
+
 } // namespace
 } // namespace format
 } // namespace clang

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] clang-format-ignore: Add support for double asterisk patterns (PR #110560)

2024-12-31 Thread Ameer J via cfe-commits


ameerj wrote:

Closing in favor of #121404 


https://github.com/llvm/llvm-project/pull/110560
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] clang-format-ignore: Add support for double asterisk patterns (PR #110560)

2024-12-31 Thread Ameer J via cfe-commits


https://github.com/ameerj closed 
https://github.com/llvm/llvm-project/pull/110560
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

45 matches

Mail list logo