[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 147002.
Typz marked 6 inline comments as done.
Typz added a comment.

Address review comment & rebase


Repository:
  rC Clang

https://reviews.llvm.org/D43290

Files:
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6708,6 +6708,15 @@
"};");
   verifyFormat("#define A {a, a},");
 
+  // Avoid breaking between equal sign and opening brace
+  FormatStyle AvoidBreakingFirstArgument = getLLVMStyle();
+  AvoidBreakingFirstArgument.PenaltyBreakBeforeFirstCallParameter = 200;
+  verifyFormat("const std::unordered_map MyHashTable =\n"
+   "{{\"a\", 0},\n"
+   " {\"b\", 1},\n"
+   " {\"c\", 2}};",
+   AvoidBreakingFirstArgument);
+
   // Binpacking only if there is no trailing comma
   verifyFormat("const Aa a = {aa, bb,\n"
"  cc, dd};",
@@ -6864,6 +6873,21 @@
   verifyFormat("vector foo = { ::SomeGlobalFunction() };", ExtraSpaces);
   verifyFormat("const struct A a = { .a = 1, .b = 2 };", ExtraSpaces);
   verifyFormat("const struct A a = { [0] = 1, [1] = 2 };", ExtraSpaces);
+
+  // Avoid breaking between initializer/equal sign and opening brace
+  ExtraSpaces.PenaltyBreakBeforeFirstCallParameter = 200;
+  verifyFormat("const std::unordered_map MyHashTable = {\n"
+   "  { \"a\", 0 },\n"
+   "  { \"b\", 1 },\n"
+   "  { \"c\", 2 }\n"
+   "};",
+   ExtraSpaces);
+  verifyFormat("const std::unordered_map MyHashTable{\n"
+   "  { \"a\", 0 },\n"
+   "  { \"b\", 1 },\n"
+   "  { \"c\", 2 }\n"
+   "};",
+   ExtraSpaces);
 }
 
 TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2311,6 +2311,8 @@
   if (Left.opensScope()) {
 if (Style.AlignAfterOpenBracket == FormatStyle::BAS_DontAlign)
   return 0;
+if (Left.is(tok::l_brace) && !Style.Cpp11BracedListStyle)
+  return 19;
 return Left.ParameterCount > 1 ? Style.PenaltyBreakBeforeFirstCallParameter
: 19;
   }
@@ -3048,6 +3050,9 @@
   if (Left.is(tok::equal) && !Right.isOneOf(tok::kw_default, tok::kw_delete) &&
   Line.Type == LT_VirtualFunctionDecl && Left.NestingLevel == 0)
 return false;
+  if (Left.is(tok::equal) && Right.is(tok::l_brace) &&
+  !Style.Cpp11BracedListStyle)
+return false;
   if (Left.is(tok::l_paren) && Left.is(TT_AttributeParen))
 return false;
   if (Left.is(tok::l_paren) && Left.Previous &&


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6708,6 +6708,15 @@
"};");
   verifyFormat("#define A {a, a},");
 
+  // Avoid breaking between equal sign and opening brace
+  FormatStyle AvoidBreakingFirstArgument = getLLVMStyle();
+  AvoidBreakingFirstArgument.PenaltyBreakBeforeFirstCallParameter = 200;
+  verifyFormat("const std::unordered_map MyHashTable =\n"
+   "{{\"a\", 0},\n"
+   " {\"b\", 1},\n"
+   " {\"c\", 2}};",
+   AvoidBreakingFirstArgument);
+
   // Binpacking only if there is no trailing comma
   verifyFormat("const Aa a = {aa, bb,\n"
"  cc, dd};",
@@ -6864,6 +6873,21 @@
   verifyFormat("vector foo = { ::SomeGlobalFunction() };", ExtraSpaces);
   verifyFormat("const struct A a = { .a = 1, .b = 2 };", ExtraSpaces);
   verifyFormat("const struct A a = { [0] = 1, [1] = 2 };", ExtraSpaces);
+
+  // Avoid breaking between initializer/equal sign and opening brace
+  ExtraSpaces.PenaltyBreakBeforeFirstCallParameter = 200;
+  verifyFormat("const std::unordered_map MyHashTable = {\n"
+   "  { \"a\", 0 },\n"
+   "  { \"b\", 1 },\n"
+   "  { \"c\", 2 }\n"
+   "};",
+   ExtraSpaces);
+  verifyFormat("const std::unordered_map MyHashTable{\n"
+   "  { \"a\", 0 },\n"
+   "  { \"b\", 1 },\n"
+   "  { \"c\", 2 }\n"
+   "};",
+   ExtraSpaces);
 }
 
 TEST_F(Format

[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC332434: clang-format: tweak formatting of variable 
initialization blocks (authored by Typz, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D43290?vs=147002&id=147003#toc

Repository:
  rC Clang

https://reviews.llvm.org/D43290

Files:
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp


Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2311,6 +2311,8 @@
   if (Left.opensScope()) {
 if (Style.AlignAfterOpenBracket == FormatStyle::BAS_DontAlign)
   return 0;
+if (Left.is(tok::l_brace) && !Style.Cpp11BracedListStyle)
+  return 19;
 return Left.ParameterCount > 1 ? Style.PenaltyBreakBeforeFirstCallParameter
: 19;
   }
@@ -3048,6 +3050,9 @@
   if (Left.is(tok::equal) && !Right.isOneOf(tok::kw_default, tok::kw_delete) &&
   Line.Type == LT_VirtualFunctionDecl && Left.NestingLevel == 0)
 return false;
+  if (Left.is(tok::equal) && Right.is(tok::l_brace) &&
+  !Style.Cpp11BracedListStyle)
+return false;
   if (Left.is(tok::l_paren) && Left.is(TT_AttributeParen))
 return false;
   if (Left.is(tok::l_paren) && Left.Previous &&
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6708,6 +6708,15 @@
"};");
   verifyFormat("#define A {a, a},");
 
+  // Avoid breaking between equal sign and opening brace
+  FormatStyle AvoidBreakingFirstArgument = getLLVMStyle();
+  AvoidBreakingFirstArgument.PenaltyBreakBeforeFirstCallParameter = 200;
+  verifyFormat("const std::unordered_map MyHashTable =\n"
+   "{{\"a\", 0},\n"
+   " {\"b\", 1},\n"
+   " {\"c\", 2}};",
+   AvoidBreakingFirstArgument);
+
   // Binpacking only if there is no trailing comma
   verifyFormat("const Aa a = {aa, bb,\n"
"  cc, dd};",
@@ -6864,6 +6873,21 @@
   verifyFormat("vector foo = { ::SomeGlobalFunction() };", ExtraSpaces);
   verifyFormat("const struct A a = { .a = 1, .b = 2 };", ExtraSpaces);
   verifyFormat("const struct A a = { [0] = 1, [1] = 2 };", ExtraSpaces);
+
+  // Avoid breaking between initializer/equal sign and opening brace
+  ExtraSpaces.PenaltyBreakBeforeFirstCallParameter = 200;
+  verifyFormat("const std::unordered_map MyHashTable = {\n"
+   "  { \"a\", 0 },\n"
+   "  { \"b\", 1 },\n"
+   "  { \"c\", 2 }\n"
+   "};",
+   ExtraSpaces);
+  verifyFormat("const std::unordered_map MyHashTable{\n"
+   "  { \"a\", 0 },\n"
+   "  { \"b\", 1 },\n"
+   "  { \"c\", 2 }\n"
+   "};",
+   ExtraSpaces);
 }
 
 TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {


Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2311,6 +2311,8 @@
   if (Left.opensScope()) {
 if (Style.AlignAfterOpenBracket == FormatStyle::BAS_DontAlign)
   return 0;
+if (Left.is(tok::l_brace) && !Style.Cpp11BracedListStyle)
+  return 19;
 return Left.ParameterCount > 1 ? Style.PenaltyBreakBeforeFirstCallParameter
: 19;
   }
@@ -3048,6 +3050,9 @@
   if (Left.is(tok::equal) && !Right.isOneOf(tok::kw_default, tok::kw_delete) &&
   Line.Type == LT_VirtualFunctionDecl && Left.NestingLevel == 0)
 return false;
+  if (Left.is(tok::equal) && Right.is(tok::l_brace) &&
+  !Style.Cpp11BracedListStyle)
+return false;
   if (Left.is(tok::l_paren) && Left.is(TT_AttributeParen))
 return false;
   if (Left.is(tok::l_paren) && Left.Previous &&
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6708,6 +6708,15 @@
"};");
   verifyFormat("#define A {a, a},");
 
+  // Avoid breaking between equal sign and opening brace
+  FormatStyle AvoidBreakingFirstArgument = getLLVMStyle();
+  AvoidBreakingFirstArgument.PenaltyBreakBeforeFirstCallParameter = 200;
+  verifyFormat("const std::unordered_map MyHashTable =\n"
+   "{{\"a\", 0},\n"
+   " {\"b\", 1},\n"
+   " {\"c\", 2}};",
+   AvoidBreakingFirstArgument);
+
   // Binpacking only if there is no t

[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 147005.
Typz marked 3 inline comments as done.
Typz added a comment.

Rebase on latest master & address review comments


Repository:
  rC Clang

https://reviews.llvm.org/D42684

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/Format.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -5475,7 +5475,7 @@
"const typename  aaa);");
 
   FormatStyle AlwaysBreak = getLLVMStyle();
-  AlwaysBreak.AlwaysBreakTemplateDeclarations = true;
+  AlwaysBreak.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
   verifyFormat("template \nclass C {};", AlwaysBreak);
   verifyFormat("template \nvoid f();", AlwaysBreak);
   verifyFormat("template \nvoid f() {}", AlwaysBreak);
@@ -5493,6 +5493,32 @@
"public:\n"
"  E *f();\n"
"};");
+
+  FormatStyle NeverBreak = getLLVMStyle();
+  NeverBreak.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_No;
+  verifyFormat("template  class C {};", NeverBreak);
+  verifyFormat("template  void f();", NeverBreak);
+  verifyFormat("template  void f() {}", NeverBreak);
+  verifyFormat("template \nvoid foo(aa ) {}",
+   NeverBreak);
+  verifyFormat("void aaa(\n"
+   "ccc);",
+   NeverBreak);
+  verifyFormat("template  class Fooo,\n"
+   "  template  class Baaar>\n"
+   "struct C {};",
+   NeverBreak);
+  verifyFormat("template  // T can be A, B or C.\n"
+   "struct C {};",
+   NeverBreak);
+  verifyFormat("template  class A {\n"
+   "public:\n"
+   "  E *f();\n"
+   "};", NeverBreak);
+  NeverBreak.PenaltyBreakTemplateDeclaration = 100;
+  verifyFormat("template  void\nfoo(aa ) {}",
+   NeverBreak);
 }
 
 TEST_F(FormatTest, WrapsTemplateParameters) {
@@ -10440,7 +10466,6 @@
   CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortIfStatementsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
-  CHECK_PARSE_BOOL(AlwaysBreakTemplateDeclarations);
   CHECK_PARSE_BOOL(BinPackArguments);
   CHECK_PARSE_BOOL(BinPackParameters);
   CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
@@ -10506,6 +10531,8 @@
   PenaltyBreakAssignment, 1234u);
   CHECK_PARSE("PenaltyBreakBeforeFirstCallParameter: 1234",
   PenaltyBreakBeforeFirstCallParameter, 1234u);
+  CHECK_PARSE("PenaltyBreakTemplateDeclaration: 1234",
+  PenaltyBreakTemplateDeclaration, 1234u);
   CHECK_PARSE("PenaltyExcessCharacter: 1234", PenaltyExcessCharacter, 1234u);
   CHECK_PARSE("PenaltyReturnTypeOnItsOwnLine: 1234",
   PenaltyReturnTypeOnItsOwnLine, 1234u);
@@ -10660,6 +10687,18 @@
   AlwaysBreakAfterReturnType,
   FormatStyle::RTBS_TopLevelDefinitions);
 
+  Style.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
+  CHECK_PARSE("AlwaysBreakTemplateDeclarations: No", AlwaysBreakTemplateDeclarations,
+  FormatStyle::BTDS_No);
+  CHECK_PARSE("AlwaysBreakTemplateDeclarations: MultiLine", AlwaysBreakTemplateDeclarations,
+  FormatStyle::BTDS_MultiLine);
+  CHECK_PARSE("AlwaysBreakTemplateDeclarations: Yes", AlwaysBreakTemplateDeclarations,
+  FormatStyle::BTDS_Yes);
+  CHECK_PARSE("AlwaysBreakTemplateDeclarations: false", AlwaysBreakTemplateDeclarations,
+  FormatStyle::BTDS_MultiLine);
+  CHECK_PARSE("AlwaysBreakTemplateDeclarations: true", AlwaysBreakTemplateDeclarations,
+  FormatStyle::BTDS_Yes);
+
   Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
   CHECK_PARSE("AlwaysBreakAfterDefinitionReturnType: None",
   AlwaysBreakAfterDefinitionReturnType, FormatStyle::DRTBS_None);
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2338,6 +2338,8 @@
   return 2;
 return 1;
   }
+  if (Left.ClosesTemplateDeclaration)
+return Style.PenaltyBreakTemplateDeclaration;
   if (Left.is(TT_ConditionalExpr))
 return prec::Conditional;
   prec::Level Level = Left.getPrecedence();
@@ -2869,7 +2871,7 @@
   if (Right.Previous->ClosesTemplateDeclaration &&
   Right.Previous->MatchingParen &&
   Right.Previous->MatchingParen->NestingLevel == 0 &&
-  Style.AlwaysBreakTemplateDeclarations)
+  Style.AlwaysBreakTemplateDeclarations == FormatStyle::BTDS_Yes)
 return true;
   if (Righ

[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL332436: clang-format: Allow optimizer to break template 
declaration. (authored by Typz, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D42684?vs=147005&id=147006#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D42684

Files:
  cfe/trunk/docs/ClangFormatStyleOptions.rst
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/ContinuationIndenter.cpp
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp

Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -351,14 +351,44 @@
   /// \endcode
   bool AlwaysBreakBeforeMultilineStrings;
 
-  /// If ``true``, always break after the ``template<...>`` of a template
-  /// declaration.
-  /// \code
-  ///true:  false:
-  ///template   vs. template  class C {};
-  ///class C {};
-  /// \endcode
-  bool AlwaysBreakTemplateDeclarations;
+  /// Different ways to break after the template declaration.
+  enum BreakTemplateDeclarationsStyle {
+  /// Do not force break before declaration.
+  /// ``PenaltyBreakTemplateDeclaration`` is taken into account.
+  /// \code
+  ///template  T foo() {
+  ///}
+  ///template  T foo(int a,
+  ///int b) {
+  ///}
+  /// \endcode
+  BTDS_No,
+  /// Force break after template declaration only when the following
+  /// declaration spans multiple lines.
+  /// \code
+  ///template  T foo() {
+  ///}
+  ///template 
+  ///T foo(int a,
+  ///  int b) {
+  ///}
+  /// \endcode
+  BTDS_MultiLine,
+  /// Always break after template declaration.
+  /// \code
+  ///template 
+  ///T foo() {
+  ///}
+  ///template 
+  ///T foo(int a,
+  ///  int b) {
+  ///}
+  /// \endcode
+  BTDS_Yes
+  };
+
+  /// The template declaration breaking style to use.
+  BreakTemplateDeclarationsStyle AlwaysBreakTemplateDeclarations;
 
   /// If ``false``, a function call's arguments will either be all on the
   /// same line or will have one line each.
@@ -1295,6 +1325,9 @@
   /// The penalty for each line break introduced inside a string literal.
   unsigned PenaltyBreakString;
 
+  /// The penalty for breaking after template declaration.
+  unsigned PenaltyBreakTemplateDeclaration;
+
   /// The penalty for each character outside of the column limit.
   unsigned PenaltyExcessCharacter;
 
@@ -1679,6 +1712,8 @@
PenaltyBreakString == R.PenaltyBreakString &&
PenaltyExcessCharacter == R.PenaltyExcessCharacter &&
PenaltyReturnTypeOnItsOwnLine == R.PenaltyReturnTypeOnItsOwnLine &&
+   PenaltyBreakTemplateDeclaration ==
+   R.PenaltyBreakTemplateDeclaration &&
PointerAlignment == R.PointerAlignment &&
RawStringFormats == R.RawStringFormats &&
SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2338,6 +2338,8 @@
   return 2;
 return 1;
   }
+  if (Left.ClosesTemplateDeclaration)
+return Style.PenaltyBreakTemplateDeclaration;
   if (Left.is(TT_ConditionalExpr))
 return prec::Conditional;
   prec::Level Level = Left.getPrecedence();
@@ -2869,7 +2871,7 @@
   if (Right.Previous->ClosesTemplateDeclaration &&
   Right.Previous->MatchingParen &&
   Right.Previous->MatchingParen->NestingLevel == 0 &&
-  Style.AlwaysBreakTemplateDeclarations)
+  Style.AlwaysBreakTemplateDeclarations == FormatStyle::BTDS_Yes)
 return true;
   if (Right.is(TT_CtorInitializerComma) &&
   Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeComma &&
Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp
===
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp
@@ -402,6 +402,12 @@
 Style.Language == FormatStyle::LK_JavaScript))
 return true;
 
+  // If the template declaration spans multiple lines, force wrap before the
+  // function/class declaration
+  if (Previous.ClosesTemplateDeclaration &&
+  State.Stack.back().BreakBeforeParameter)
+return true;
+
   if (State.Column <= NewLineColumn)
 return false;
 
@@ -453,7 +459,7 @@
 // for cases whe

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 147011.
Typz added a comment.

Rebase on latest master


Repository:
  rC Clang

https://reviews.llvm.org/D43015

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/Format.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1316,15 +1316,40 @@
   verifyFormat("class ::A::B {};");
 }
 
-TEST_F(FormatTest, BreakBeforeInheritanceComma) {
-  FormatStyle StyleWithInheritanceBreak = getLLVMStyle();
-  StyleWithInheritanceBreak.BreakBeforeInheritanceComma = true;
-
-  verifyFormat("class MyClass : public X {};", StyleWithInheritanceBreak);
+TEST_F(FormatTest, BreakInheritanceStyle) {
+  FormatStyle StyleWithInheritanceBreakBeforeComma = getLLVMStyle();
+  StyleWithInheritanceBreakBeforeComma.BreakInheritanceList =
+  FormatStyle::BILS_BeforeComma;
+  verifyFormat("class MyClass : public X {};",
+   StyleWithInheritanceBreakBeforeComma);
   verifyFormat("class MyClass\n"
": public X\n"
", public Y {};",
-   StyleWithInheritanceBreak);
+   StyleWithInheritanceBreakBeforeComma);
+  verifyFormat("class AA\n"
+   ": public BB\n"
+   ", public CC {};",
+   StyleWithInheritanceBreakBeforeComma);
+  verifyFormat("struct a\n"
+   ": public aaa< // break\n"
+   "  > {};",
+   StyleWithInheritanceBreakBeforeComma);
+
+  FormatStyle StyleWithInheritanceBreakAfterColon = getLLVMStyle();
+  StyleWithInheritanceBreakAfterColon.BreakInheritanceList =
+  FormatStyle::BILS_AfterColon;
+  verifyFormat("class MyClass : public X {};",
+   StyleWithInheritanceBreakAfterColon);
+  verifyFormat("class MyClass : public X, public Y {};",
+   StyleWithInheritanceBreakAfterColon);
+  verifyFormat("class AA :\n"
+   "public BB,\n"
+   "public CC {};",
+   StyleWithInheritanceBreakAfterColon);
+  verifyFormat("struct a :\n"
+   "public aaa< // break\n"
+   "> {};",
+   StyleWithInheritanceBreakAfterColon);
 }
 
 TEST_F(FormatTest, FormatsVariableDeclarationsAfterStructOrClass) {
@@ -3726,6 +3751,23 @@
"  aa,\n"
"  bb {}",
Style);
+
+  // `ConstructorInitializerIndentWidth` actually applies to InheritanceList as well
+  Style.BreakInheritanceList = FormatStyle::BILS_BeforeColon;
+  verifyFormat("class SomeClass\n"
+   "  : public aa,\n"
+   "public bb {};",
+   Style);
+  Style.BreakInheritanceList = FormatStyle::BILS_BeforeComma;
+  verifyFormat("class SomeClass\n"
+   "  : public aa\n"
+   "  , public bb {};",
+   Style);
+  Style.BreakInheritanceList = FormatStyle::BILS_AfterColon;
+  verifyFormat("class SomeClass :\n"
+   "  public aa,\n"
+   "  public bb {};",
+   Style);
 }
 
 #ifndef EXPENSIVE_CHECKS
@@ -9101,7 +9143,7 @@
"  (2) {}",
CtorInitializerStyle);
 
-  FormatStyle InheritanceStyle = getLLVMStyle();
+  FormatStyle InheritanceStyle = getLLVMStyleWithColumns(30);
   InheritanceStyle.SpaceBeforeInheritanceColon = false;
   verifyFormat("class Foo: public Bar {};", InheritanceStyle);
   verifyFormat("Foo::Foo() : foo(1) {}", InheritanceStyle);
@@ -9117,6 +9159,29 @@
"default:\n"
"}",
InheritanceStyle);
+  InheritanceStyle.BreakInheritanceList = FormatStyle::BILS_AfterColon;
+  verifyFormat("class Foo:\n"
+   "public aa,\n"
+   "public bb {\n"
+   "}",
+   InheritanceStyle);
+  InheritanceStyle.BreakInheritanceList = FormatStyle::BILS_BeforeComma;
+  verifyFormat("class Foo\n"
+   ": public aa\n"
+   ", public bb {\n"
+   "}",
+   Inher

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


Repository:
  rC Clang

https://reviews.llvm.org/D43015



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

[PATCH] D33440: clang-format: better handle statement macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 147024.
Typz added a comment.
Herald added a subscriber: mgrang.

Rebase on latest master


Repository:
  rC Clang

https://reviews.llvm.org/D33440

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/FormatTokenLexer.h
  lib/Format/UnwrappedLineParser.cpp
  lib/Format/UnwrappedLineParser.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2472,6 +2472,45 @@
getLLVMStyleWithColumns(40)));
 
   verifyFormat("MACRO(>)");
+
+  // Some macros contain an implicit semicolon
+  FormatStyle Style = getLLVMStyle();
+  Style.StatementMacros.push_back("FOO");
+  verifyFormat("FOO(a) int b = 0;");
+  verifyFormat("FOO(a)\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO(a);\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO(argc, argv, \"4.0.2\")\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO()\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("void f() {\n"
+   "  FOO(a)\n"
+   "  return a;\n"
+   "}",
+   Style);
+  verifyFormat("FOO(a)\n"
+   "FOO(b)",
+   Style);
+  verifyFormat("int a = 0;\n"
+   "FOO(b)\n"
+   "int c = 0;",
+   Style);
+  verifyFormat("int a = 0;\n"
+   "int x = FOO(a)\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("void foo(int a) { FOO(a) }\n"
+   "uint32_t bar() {}",
+   Style);
 }
 
 TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) {
@@ -10728,6 +10767,12 @@
   CHECK_PARSE("ForEachMacros: [BOOST_FOREACH, Q_FOREACH]", ForEachMacros,
   BoostAndQForeach);
 
+  Style.StatementMacros.clear();
+  CHECK_PARSE("StatementMacros: [QUNUSED]", StatementMacros,
+  std::vector{"QUNUSED"});
+  CHECK_PARSE("StatementMacros: [QUNUSED, QT_REQUIRE_VERSION]", StatementMacros,
+  std::vector({"QUNUSED", "QT_REQUIRE_VERSION"}));
+
   Style.IncludeStyle.IncludeCategories.clear();
   std::vector ExpectedCategories = {
   {"abc/.*", 2}, {".*", 1}};
Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -125,6 +125,7 @@
   void parseObjCInterfaceOrImplementation();
   bool parseObjCProtocol();
   void parseJavaScriptEs6ImportExport();
+  void parseStatementMacro();
   bool tryToParseLambda();
   bool tryToParseLambdaIntroducer();
   void tryToParseJSFunction();
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -473,6 +473,10 @@
   }
   LBraceStack.pop_back();
   break;
+case tok::identifier:
+  if (!Tok->is(TT_StatementMacro))
+  break;
+  LLVM_FALLTHROUGH;
 case tok::at:
 case tok::semi:
 case tok::kw_if:
@@ -1098,6 +1102,10 @@
 return;
   }
 }
+if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  parseStatementMacro();
+  return;
+}
 // In all other cases, parse the declaration.
 break;
   default:
@@ -1297,6 +1305,11 @@
 return;
   }
 
+  if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+parseStatementMacro();
+return;
+  }
+
   // See if the following token should start a new unwrapped line.
   StringRef Text = FormatTok->TokenText;
   nextToken();
@@ -2295,6 +2308,16 @@
   }
 }
 
+void UnwrappedLineParser::parseStatementMacro()
+{
+  nextToken();
+  if (FormatTok->is(tok::l_paren))
+parseParens();
+  if (FormatTok->is(tok::semi))
+nextToken();
+  addUnwrappedLine();
+}
+
 LLVM_ATTRIBUTE_UNUSED static void printDebugInfo(const UnwrappedLine &Line,
  StringRef Prefix = "") {
   llvm::dbgs() << Prefix << "Line(" << Line.Level
Index: lib/Format/FormatTokenLexer.h
===
--- lib/Format/FormatTokenLexer.h
+++ lib/Format/FormatTokenLexer.h
@@ -22,6 +22,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Format/Format.h"
 #include "llvm/Support/Regex.h"
+#include "llvm/ADT/MapVector.h"
 
 #include 
 
@@ -99,7 +100,8 @@
   // Index (in 'Tokens') of the last token that starts a new line.
   unsigned FirstInLineIndex;
   SmallVector Tokens;
-  SmallVector ForEachMacros;
+
+  llvm::SmallMapVector Macros;
 
   bool FormattingDisabled;
 
Index: lib/Format/FormatTokenLexer.cpp
=

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D37813#958222, @klimek wrote:

> Some initial design work has been done, and Krasimir said that he's 
> interested. No timeline though :(


@klimek , @krasimir : any progress on this new preprocessor-like way to 
configure macros ?


Repository:
  rC Clang

https://reviews.llvm.org/D37813



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

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 147027.
Typz added a comment.

Rebase


Repository:
  rC Clang

https://reviews.llvm.org/D37813

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -53,6 +53,7 @@
 "  int i;\n"
 "  int j;\n"
 "}"));
+
   EXPECT_EQ("namespace {\n"
 "  int i;\n"
 "  int j;\n"
@@ -249,6 +250,85 @@
 "// unrelated"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, AddsMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+
+  EXPECT_EQ("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A)",
+fixNamespaceEndComments("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(\"Test1\")",
+fixNamespaceEndComments("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+}
+
 TEST_F(NamespaceEndCommentsFixerTest, AddsNewlineIfNeeded) {
   EXPECT_EQ("namespace A {\n"
 "  int i;\n"
@@ -381,6 +461,54 @@
 "}; /* unnamed namespace */"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, KeepsValidMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"} // end anonymous TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"} // end anonymous TESTSUITE()",
+Style));
+  EXPECT_EQ("TESTSUITE(A) {\n"
+"  int i;\n"
+"} /* en

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 147031.
Typz added a comment.

Rebase


Repository:
  rC Clang

https://reviews.llvm.org/D32478

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/ContinuationIndenter.h
  lib/Format/Format.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/FormatTestJS.cpp

Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -267,7 +267,7 @@
   verifyFormat("var x = a() in\n"
".aa.aa;");
   FormatStyle Style = getGoogleJSStyleWithColumns(80);
-  Style.AlignOperands = true;
+  Style.AlignOperands = FormatStyle::OAS_Align;
   verifyFormat("var x = a() in\n"
".aa.aa;",
Style);
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -3319,6 +3319,9 @@
"  > c) {\n"
"}",
Style);
+  verifyFormat("return a\n"
+   "   && bbb;",
+   Style);
   verifyFormat("return (a)\n"
"   // comment\n"
"   + b;",
@@ -3347,11 +3350,103 @@
 
   Style.ColumnLimit = 60;
   verifyFormat("zz\n"
-   "= b\n"
+   "= \n"
"  >> (aa);",
Style);
 }
 
+TEST_F(FormatTest, ExpressionIndentationStrictAlign) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  Style.AlignOperands = FormatStyle::OAS_AlignAfterOperator;
+
+  verifyFormat("bool value = a\n"
+   "   + a\n"
+   "   + a\n"
+   "  == a\n"
+   " * b\n"
+   " + b\n"
+   "  && a\n"
+   " * a\n"
+   " > c;",
+   Style);
+  verifyFormat("if (a\n"
+   "* \n"
+   "+ aa\n"
+   "== bbb) {\n}",
+   Style);
+  verifyFormat("if (a\n"
+   "+ \n"
+   "  * aa\n"
+   "== bbb) {\n}",
+   Style);
+  verifyFormat("if (a\n"
+   "== \n"
+   "   * aa\n"
+   "   + bbb) {\n}",
+   Style);
+  verifyFormat("if () {\n"
+   "} else if (a\n"
+   "   && b // break\n"
+   "  > c) {\n"
+   "}",
+   Style);
+  verifyFormat("return a\n"
+   "&& bbb;",
+   Style);
+  verifyFormat("return (a)\n"
+   " // comment\n"
+   " + b;",
+   Style);
+  verifyFormat(
+  "int aa = aa\n"
+  "   * bbb\n"
+  "   + cc;",
+  Style);
+
+  verifyFormat("a\n"
+   "=  + ;",
+   Style);
+
+  verifyFormat("return boost::fusion::at_c<0>().second\n"
+   "== boost::fusion::at_c<1>().second;",
+   Style);
+
+  Style.ColumnLimit = 60;
+  verifyFormat("z\n"
+   "= \n"
+   "   >> (aaa

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 147079.
Typz added a comment.

rebase


Repository:
  rC Clang

https://reviews.llvm.org/D43183

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/UnwrappedLineParser.cpp
  lib/Format/UnwrappedLineParser.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1010,6 +1010,96 @@
"  return;\n"
"}",
getLLVMStyleWithColumns(34));
+
+  FormatStyle indentClosingBrace = getLLVMStyle();
+  indentClosingBrace.CaseBlockIndent = FormatStyle::CBIS_ClosingBrace;
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "  f();\n"
+   "  break;\n"
+   "}",
+   indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "  f();\n"
+   "  break;\n"
+   "  }\n"
+   "case 2: {\n"
+   "  break;\n"
+   "  }\n"
+   "}",
+   indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "  f();\n"
+   "  } break;\n"
+   "case 2: {\n"
+   "  } break;\n"
+   "}",
+   indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "  f();\n"
+   "  }\n"
+   "  g();\n"
+   "  break;\n"
+   "}",
+   indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "  f();\n"
+   "  {\n"
+   "g();\n"
+   "h();\n"
+   "  }\n"
+   "  break;\n"
+   "}",
+   indentClosingBrace);
+
+  FormatStyle indentBlock = getLLVMStyle();
+  indentBlock.CaseBlockIndent = FormatStyle::CBIS_Block;
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "  f();\n"
+   "  break;\n"
+   "}",
+   indentBlock);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "f();\n"
+   "break;\n"
+   "  }\n"
+   "case 2: {\n"
+   "break;\n"
+   "  }\n"
+   "}",
+   indentBlock);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "f();\n"
+   "  } break;\n"
+   "case 2: {\n"
+   "  } break;\n"
+   "}",
+   indentBlock);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "f();\n"
+   "  }\n"
+   "  g();\n"
+   "  break;\n"
+   "}",
+   indentBlock);
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "  f();\n"
+   "  {\n"
+   "g();\n"
+   "h();\n"
+   "  }\n"
+   "  break;\n"
+   "}",
+   indentBlock);
 }
 
 TEST_F(FormatTest, CaseRanges) {
@@ -10639,6 +10729,13 @@
   CHECK_PARSE("AllowShortFunctionsOnASingleLine: true",
   AllowShortFunctionsOnASingleLine, FormatStyle::SFS_All);
 
+  Style.CaseBlockIndent = FormatStyle::CBIS_Block;
+  CHECK_PARSE("CaseBlockIndent: None", CaseBlockIndent, FormatStyle::CBIS_None);
+  CHECK_PARSE("CaseBlockIndent: ClosingBrace", CaseBlockIndent,
+  FormatStyle::CBIS_ClosingBrace);
+  CHECK_PARSE("CaseBlockIndent: Block", CaseBlockIndent,
+  FormatStyle::CBIS_Block);
+
   Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
   CHECK_PARSE("SpaceBeforeParens: Never", SpaceBeforeParens,
   FormatStyle::SBPO_Never);
Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -88,6 +88,10 @@
   void parseFile();
   void parseLevel(bool HasOpeningBrace);
   void parseBlock(bool MustBeDeclaration, bool AddLevel = true,
+  bool MunchSemi = true) {
+parseBlock(MustBeDeclaration, AddLevel ? 1 : 0, MunchSemi);
+  }
+  void parseBlock(bool MustBeDeclaration, int AddLevel,
   bool MunchSemi = true);
   void parseChildBlock();
   void parsePPDirective();
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -515,7 +515,7 @@
   return h;
 }
 
-void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
+void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, int AddLevel,
  bool MunchSemi) {
   assert(FormatTok->isOneOf(tok::l_brace,

[PATCH] D33440: clang-format: better handle statement macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 147293.
Typz marked an inline comment as done.
Typz added a comment.

Address review comments


Repository:
  rC Clang

https://reviews.llvm.org/D33440

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/FormatTokenLexer.h
  lib/Format/UnwrappedLineParser.cpp
  lib/Format/UnwrappedLineParser.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2472,6 +2472,45 @@
getLLVMStyleWithColumns(40)));
 
   verifyFormat("MACRO(>)");
+
+  // Some macros contain an implicit semicolon.
+  FormatStyle Style = getLLVMStyle();
+  Style.StatementMacros.push_back("FOO");
+  verifyFormat("FOO(a) int b = 0;");
+  verifyFormat("FOO(a)\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO(a);\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO(argc, argv, \"4.0.2\")\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO()\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("void f() {\n"
+   "  FOO(a)\n"
+   "  return a;\n"
+   "}",
+   Style);
+  verifyFormat("FOO(a)\n"
+   "FOO(b)",
+   Style);
+  verifyFormat("int a = 0;\n"
+   "FOO(b)\n"
+   "int c = 0;",
+   Style);
+  verifyFormat("int a = 0;\n"
+   "int x = FOO(a)\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("void foo(int a) { FOO(a) }\n"
+   "uint32_t bar() {}",
+   Style);
 }
 
 TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) {
@@ -10728,6 +10767,12 @@
   CHECK_PARSE("ForEachMacros: [BOOST_FOREACH, Q_FOREACH]", ForEachMacros,
   BoostAndQForeach);
 
+  Style.StatementMacros.clear();
+  CHECK_PARSE("StatementMacros: [QUNUSED]", StatementMacros,
+  std::vector{"QUNUSED"});
+  CHECK_PARSE("StatementMacros: [QUNUSED, QT_REQUIRE_VERSION]", StatementMacros,
+  std::vector({"QUNUSED", "QT_REQUIRE_VERSION"}));
+
   Style.IncludeStyle.IncludeCategories.clear();
   std::vector ExpectedCategories = {
   {"abc/.*", 2}, {".*", 1}};
Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -125,6 +125,7 @@
   void parseObjCInterfaceOrImplementation();
   bool parseObjCProtocol();
   void parseJavaScriptEs6ImportExport();
+  void parseStatementMacro();
   bool tryToParseLambda();
   bool tryToParseLambdaIntroducer();
   void tryToParseJSFunction();
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -473,6 +473,10 @@
   }
   LBraceStack.pop_back();
   break;
+case tok::identifier:
+  if (!Tok->is(TT_StatementMacro))
+  break;
+  LLVM_FALLTHROUGH;
 case tok::at:
 case tok::semi:
 case tok::kw_if:
@@ -1098,6 +1102,10 @@
 return;
   }
 }
+if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  parseStatementMacro();
+  return;
+}
 // In all other cases, parse the declaration.
 break;
   default:
@@ -1297,6 +1305,11 @@
 return;
   }
 
+  if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+parseStatementMacro();
+return;
+  }
+
   // See if the following token should start a new unwrapped line.
   StringRef Text = FormatTok->TokenText;
   nextToken();
@@ -2295,6 +2308,16 @@
   }
 }
 
+void UnwrappedLineParser::parseStatementMacro()
+{
+  nextToken();
+  if (FormatTok->is(tok::l_paren))
+parseParens();
+  if (FormatTok->is(tok::semi))
+nextToken();
+  addUnwrappedLine();
+}
+
 LLVM_ATTRIBUTE_UNUSED static void printDebugInfo(const UnwrappedLine &Line,
  StringRef Prefix = "") {
   llvm::dbgs() << Prefix << "Line(" << Line.Level
Index: lib/Format/FormatTokenLexer.h
===
--- lib/Format/FormatTokenLexer.h
+++ lib/Format/FormatTokenLexer.h
@@ -22,6 +22,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Format/Format.h"
 #include "llvm/Support/Regex.h"
+#include "llvm/ADT/MapVector.h"
 
 #include 
 
@@ -99,7 +100,8 @@
   // Index (in 'Tokens') of the last token that starts a new line.
   unsigned FirstInLineIndex;
   SmallVector Tokens;
-  SmallVector ForEachMacros;
+
+  llvm::SmallMapVector Macros;
 
   bool FormattingDisabled;
 
Index: lib/Format/FormatTokenLexer.cpp

[PATCH] D33440: clang-format: better handle statement macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz marked an inline comment as done.
Typz added inline comments.



Comment at: lib/Format/Format.cpp:647-648
   LLVMStyle.SortUsingDeclarations = true;
+  LLVMStyle.StatementMacros.push_back("Q_UNUSED");
+  LLVMStyle.StatementMacros.push_back("QT_REQUIRE_VERSION");
 

alexfh wrote:
> What's the reason to have these in the LLVM style? The macros aren't used in 
> LLVM code.
This is similar to the default foreach macros (foreach, Q_FOREACH and 
BOOST_FOREACH) : the macros are added here so that they are the default values 
for any style, since LLVM style is also both the default style and the base 
style for any style.



Repository:
  rC Clang

https://reviews.llvm.org/D33440



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

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D37813#876227, @klimek wrote:

> I think instead of introducing more and more special cases of macros we might 
> want to handle, we should instead allow specifying macro productions globally.


what do you mean?
Do you mean to group all the macro configuration options into "Macros" field, 
still containing one field for each kind of macro? Or do you have something 
else in mind?


https://reviews.llvm.org/D37813



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

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


https://reviews.llvm.org/D32478



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

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

> My question is: if CanBreak is false, we currently don't call 
> breakProtrudingToken. So either we do something very wrong in that case 
> (which might be true, but I'd like to understand why) or we should be able  
> to just calculate the penalty by not breaking anything and go on.

CanBreak is currently very short, it only verifies some very broad conditions. 
I initiallly tried patching at this level, but it really does not work.

Most conditions are actually tested at the beginning of breakProtrudingToken. 
That part of the code actually takes different branches for different kind of 
tokens (strings, block commands, line comments...), and handles many different 
cases for each. This goal is to create the actual BreakableToken object, but it 
has a few other side effects: it returns immediately in various corner cases 
(javascript, preprocessor, formatting macros), and it tweaks some 
variables, e.g. ColumnLimit.

In addition, replacement is non trivial even in case we choose not to reflow: 
it is required in order to properly manage whitespaces. For this reason we 
really must pass through the loop in most cases.


https://reviews.llvm.org/D33589



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

[PATCH] D33440: clang-format: better handle statement macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 120084.
Typz added a comment.

rebase


https://reviews.llvm.org/D33440

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/FormatTokenLexer.h
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2420,6 +2420,42 @@
getLLVMStyleWithColumns(40)));
 
   verifyFormat("MACRO(>)");
+
+  // Some macros contain an implicit semicolon
+  FormatStyle Style = getLLVMStyle();
+  Style.StatementMacros.push_back("FOO");
+  verifyFormat("FOO(a) int b = 0;");
+  verifyFormat("FOO(a)\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO(a);\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO(argc, argv, \"4.0.2\")\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO()\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("void f() {\n"
+   "  FOO(a)\n"
+   "  return a;\n"
+   "}",
+   Style);
+  verifyFormat("FOO(a)\n"
+   "FOO(b)",
+   Style);
+  verifyFormat("int a = 0;\n"
+   "FOO(b)\n"
+   "int c = 0;",
+   Style);
+  verifyFormat("int a = 0;\n"
+   "int x = FOO(a)\n"
+   "int b = 0;",
+   Style);
 }
 
 TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) {
@@ -10244,6 +10280,12 @@
   CHECK_PARSE("ForEachMacros: [BOOST_FOREACH, Q_FOREACH]", ForEachMacros,
   BoostAndQForeach);
 
+  Style.StatementMacros.clear();
+  CHECK_PARSE("StatementMacros: [QUNUSED]", StatementMacros,
+  std::vector{"QUNUSED"});
+  CHECK_PARSE("StatementMacros: [QUNUSED, QT_REQUIRE_VERSION]", StatementMacros,
+  std::vector({"QUNUSED", "QT_REQUIRE_VERSION"}));
+
   Style.IncludeCategories.clear();
   std::vector ExpectedCategories = {{"abc/.*", 2},
   {".*", 1}};
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1093,6 +1093,15 @@
 return;
   }
 }
+if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  nextToken();
+  if (FormatTok->is(tok::l_paren))
+parseParens();
+  if (FormatTok->is(tok::semi))
+nextToken();
+  addUnwrappedLine();
+  return;
+}
 // In all other cases, parse the declaration.
 break;
   default:
@@ -1242,6 +1251,16 @@
 return;
   }
 
+  if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+nextToken();
+if (FormatTok->is(tok::l_paren))
+  parseParens();
+if (FormatTok->is(tok::semi))
+  nextToken();
+addUnwrappedLine();
+return;
+  }
+
   // See if the following token should start a new unwrapped line.
   StringRef Text = FormatTok->TokenText;
   nextToken();
Index: lib/Format/FormatTokenLexer.h
===
--- lib/Format/FormatTokenLexer.h
+++ lib/Format/FormatTokenLexer.h
@@ -22,6 +22,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Format/Format.h"
 #include "llvm/Support/Regex.h"
+#include 
 
 #include 
 
@@ -97,7 +98,8 @@
   // Index (in 'Tokens') of the last token that starts a new line.
   unsigned FirstInLineIndex;
   SmallVector Tokens;
-  SmallVector ForEachMacros;
+
+  llvm::SmallMapVector Macros;
 
   bool FormattingDisabled;
 
Index: lib/Format/FormatTokenLexer.cpp
===
--- lib/Format/FormatTokenLexer.cpp
+++ lib/Format/FormatTokenLexer.cpp
@@ -37,8 +37,9 @@
   Lex->SetKeepWhitespaceMode(true);
 
   for (const std::string &ForEachMacro : Style.ForEachMacros)
-ForEachMacros.push_back(&IdentTable.get(ForEachMacro));
-  std::sort(ForEachMacros.begin(), ForEachMacros.end());
+Macros.insert({&IdentTable.get(ForEachMacro), TT_ForEachMacro});
+  for (const std::string &StatementMacro : Style.StatementMacros)
+Macros.insert({&IdentTable.get(StatementMacro), TT_StatementMacro});
 }
 
 ArrayRef FormatTokenLexer::lex() {
@@ -626,12 +627,13 @@
   }
 
   if (Style.isCpp()) {
+decltype(Macros)::iterator it;
 if (!(Tokens.size() > 0 && Tokens.back()->Tok.getIdentifierInfo() &&
   Tokens.back()->Tok.getIdentifierInfo()->getPPKeywordID() ==
   tok::pp_define) &&
-std::find(ForEachMacros.begin(), ForEachMacros.end(),
-  FormatTok->Tok.getIdentifierInfo()) != ForEachMacros.end()) {
- 

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 120085.
Typz added a comment.

rebase


https://reviews.llvm.org/D37813

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -53,6 +53,7 @@
 "  int i;\n"
 "  int j;\n"
 "}"));
+
   EXPECT_EQ("namespace {\n"
 "  int i;\n"
 "  int j;\n"
@@ -249,6 +250,85 @@
 "// unrelated"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, AddsMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+
+  EXPECT_EQ("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A)",
+fixNamespaceEndComments("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(\"Test1\")",
+fixNamespaceEndComments("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+}
+
 TEST_F(NamespaceEndCommentsFixerTest, AddsNewlineIfNeeded) {
   EXPECT_EQ("namespace A {\n"
 "  int i;\n"
@@ -381,6 +461,54 @@
 "}; /* unnamed namespace */"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, KeepsValidMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"} // end anonymous TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"} // end anonymous TESTSUITE()",
+Style));
+  EXPECT_EQ("TESTSUITE(A) {\n"
+"  int i;\n"
+"} /* end of TESTSUITE(A) */",
+

[PATCH] D50147: clang-format: support external styles

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

clang-format does indeed support .clang-format, which is great for *isolated* 
projects, and which is not affected by this patch.

This patch addresses the issue of *centralizing* the definition of styles, e.g. 
allowing individual projects to reference externally defined styles. This is 
useful when deploying a coding styles compagny-wide, to avoid copying the 
configuration in each project (and later having problem issues to maintain the 
styles or check if the correct style is indeed used). Another way of seeing it 
is that it allows extending the styles which are hard-coded in clang-format 
(llvm, google, ...)


Repository:
  rC Clang

https://reviews.llvm.org/D50147



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

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 158302.
Typz added a comment.
Herald added a subscriber: acoomans.

rebase


Repository:
  rC Clang

https://reviews.llvm.org/D32478

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/ContinuationIndenter.h
  lib/Format/Format.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/FormatTestJS.cpp

Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -267,7 +267,7 @@
   verifyFormat("var x = a() in\n"
".aa.aa;");
   FormatStyle Style = getGoogleJSStyleWithColumns(80);
-  Style.AlignOperands = true;
+  Style.AlignOperands = FormatStyle::OAS_Align;
   verifyFormat("var x = a() in\n"
".aa.aa;",
Style);
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -3344,6 +3344,9 @@
"  > c) {\n"
"}",
Style);
+  verifyFormat("return a\n"
+   "   && bbb;",
+   Style);
   verifyFormat("return (a)\n"
"   // comment\n"
"   + b;",
@@ -3372,11 +3375,103 @@
 
   Style.ColumnLimit = 60;
   verifyFormat("zz\n"
-   "= b\n"
+   "= \n"
"  >> (aa);",
Style);
 }
 
+TEST_F(FormatTest, ExpressionIndentationStrictAlign) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  Style.AlignOperands = FormatStyle::OAS_AlignAfterOperator;
+
+  verifyFormat("bool value = a\n"
+   "   + a\n"
+   "   + a\n"
+   "  == a\n"
+   " * b\n"
+   " + b\n"
+   "  && a\n"
+   " * a\n"
+   " > c;",
+   Style);
+  verifyFormat("if (a\n"
+   "* \n"
+   "+ aa\n"
+   "== bbb) {\n}",
+   Style);
+  verifyFormat("if (a\n"
+   "+ \n"
+   "  * aa\n"
+   "== bbb) {\n}",
+   Style);
+  verifyFormat("if (a\n"
+   "== \n"
+   "   * aa\n"
+   "   + bbb) {\n}",
+   Style);
+  verifyFormat("if () {\n"
+   "} else if (a\n"
+   "   && b // break\n"
+   "  > c) {\n"
+   "}",
+   Style);
+  verifyFormat("return a\n"
+   "&& bbb;",
+   Style);
+  verifyFormat("return (a)\n"
+   " // comment\n"
+   " + b;",
+   Style);
+  verifyFormat(
+  "int aa = aa\n"
+  "   * bbb\n"
+  "   + cc;",
+  Style);
+
+  verifyFormat("a\n"
+   "=  + ;",
+   Style);
+
+  verifyFormat("return boost::fusion::at_c<0>().second\n"
+   "== boost::fusion::at_c<1>().second;",
+   Style);
+
+  Style.ColumnLimit = 60;
+  verifyFormat("z\n"
+   "= \n"
+ 

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Typz added reviewers: krasimir, djasper, klimek.
Herald added a subscriber: acoomans.

When multiple ternary operators are chained, e.g. like an if/else-if/
else-if/.../else sequence, clang-format will keep aligning the colon
with the question mark, which increases the indent for each
conditionals:

  int a = condition1 ? result1
 : condition2 ? result2
  : condition3 ? result3
   : result4;

This patch detects the situation (e.g. conditionals used in false branch
of another conditional), to avoid indenting in that case:

  int a = condition1 ? result1
: condition2 ? result2
: condition3 ? result3
 : result4;

When BreakBeforeTernaryOperators is false, this will format like this:

  int a = condition1 ? result1 :
  condition2 ? result2 :
  conditino3 ? result3 :
   result4;

This formatting style is referenced here:
https://www.fluentcpp.com/2018/02/27/replace-else-if-ternary-operator/
and here:
https://marcmutz.wordpress.com/2010/10/14/top-5-reasons-you-should-love-your-ternary-operator/


Repository:
  rC Clang

https://reviews.llvm.org/D50078

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/ContinuationIndenter.h
  lib/Format/WhitespaceManager.cpp
  lib/Format/WhitespaceManager.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -4801,9 +4801,9 @@
getLLVMStyleWithColumns(60));
   verifyFormat("bool aa = a //\n"
"  ? aaa\n"
-   "  : bbb //\n"
-   "? ccc\n"
-   ": ddd;");
+   "  : bbb //\n"
+   "  ? ccc\n"
+   "  : ddd;");
   verifyFormat("bool aa = a //\n"
"  ? aaa\n"
"  : (bbb //\n"
@@ -4865,6 +4865,113 @@
" // comment\n"
" ? a = b\n"
" : a;");
+
+  // Chained conditionals
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 70;
+  Style.AlignOperands = FormatStyle::OAS_AlignAfterOperator;
+  verifyFormat("return  ? \n"
+   " :  ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   " : bb   ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return aa   ? \n"
+   " :  ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   " :  ? 22\n"
+   ": 33;",
+   Style);
+  verifyFormat("return  ? \n"
+   " :  ? \n"
+   " :  ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? (aaa ? bbb : ccc)\n"
+   " :  ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   " :  ? \n"
+   ": (aaa ? bbb : ccc);",
+   Style);
+  verifyFormat("return  ? (a ? bb\n"
+   " : cc)\n"
+   " :  ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? (a ? bb\n"
+   " : cc)\n"
+   " :  ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? a = (a ? bb\n"
+   " : dd)\n"

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Notes:

- I choose not to add an option to enable this behavior, as I think of it as 
just another way clang-format can (better) format the code; but I can one if 
needed
- Currently, it relies on another patch (https://reviews.llvm.org/D32478), 
which supports aligning the wrapped operator slightly differently. If/since 
that other patch does not seem to make it, I can change this patch to either do 
the alignment in this specific case (e.g. for wrapped ternary operator only) or 
to keep the 'default' behavior of clang-format (e.g. the wrapped colon would be 
aligned with the first condition):

  // i.e. the better looking option, but which requires specific cases when it 
comes after assignment or return:
  int fooo =  ? 0
   :  ? 1
  : ;
  // or the option more consistent with current clang-format behavior:
  int fooo =    ? 0
 :  ? 1
: ;

The current patch supports both behaviors, as it relies on 
https://reviews.llvm.org/D32478 to specify the expected behavior.


Repository:
  rC Clang

https://reviews.llvm.org/D50078



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

[PATCH] D33440: clang-format: better handle statement macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 158308.
Typz marked an inline comment as done.
Typz added a comment.
Herald added a subscriber: acoomans.

Regenerate documentation


Repository:
  rC Clang

https://reviews.llvm.org/D33440

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/FormatTokenLexer.h
  lib/Format/UnwrappedLineParser.cpp
  lib/Format/UnwrappedLineParser.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2472,6 +2472,45 @@
getLLVMStyleWithColumns(40)));
 
   verifyFormat("MACRO(>)");
+
+  // Some macros contain an implicit semicolon.
+  FormatStyle Style = getLLVMStyle();
+  Style.StatementMacros.push_back("FOO");
+  verifyFormat("FOO(a) int b = 0;");
+  verifyFormat("FOO(a)\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO(a);\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO(argc, argv, \"4.0.2\")\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO()\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("void f() {\n"
+   "  FOO(a)\n"
+   "  return a;\n"
+   "}",
+   Style);
+  verifyFormat("FOO(a)\n"
+   "FOO(b)",
+   Style);
+  verifyFormat("int a = 0;\n"
+   "FOO(b)\n"
+   "int c = 0;",
+   Style);
+  verifyFormat("int a = 0;\n"
+   "int x = FOO(a)\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("void foo(int a) { FOO(a) }\n"
+   "uint32_t bar() {}",
+   Style);
 }
 
 TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) {
@@ -10728,6 +10767,12 @@
   CHECK_PARSE("ForEachMacros: [BOOST_FOREACH, Q_FOREACH]", ForEachMacros,
   BoostAndQForeach);
 
+  Style.StatementMacros.clear();
+  CHECK_PARSE("StatementMacros: [QUNUSED]", StatementMacros,
+  std::vector{"QUNUSED"});
+  CHECK_PARSE("StatementMacros: [QUNUSED, QT_REQUIRE_VERSION]", StatementMacros,
+  std::vector({"QUNUSED", "QT_REQUIRE_VERSION"}));
+
   Style.IncludeStyle.IncludeCategories.clear();
   std::vector ExpectedCategories = {
   {"abc/.*", 2}, {".*", 1}};
Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -125,6 +125,7 @@
   void parseObjCInterfaceOrImplementation();
   bool parseObjCProtocol();
   void parseJavaScriptEs6ImportExport();
+  void parseStatementMacro();
   bool tryToParseLambda();
   bool tryToParseLambdaIntroducer();
   void tryToParseJSFunction();
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -473,6 +473,10 @@
   }
   LBraceStack.pop_back();
   break;
+case tok::identifier:
+  if (!Tok->is(TT_StatementMacro))
+  break;
+  LLVM_FALLTHROUGH;
 case tok::at:
 case tok::semi:
 case tok::kw_if:
@@ -1098,6 +1102,10 @@
 return;
   }
 }
+if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  parseStatementMacro();
+  return;
+}
 // In all other cases, parse the declaration.
 break;
   default:
@@ -1297,6 +1305,11 @@
 return;
   }
 
+  if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+parseStatementMacro();
+return;
+  }
+
   // See if the following token should start a new unwrapped line.
   StringRef Text = FormatTok->TokenText;
   nextToken();
@@ -2295,6 +2308,16 @@
   }
 }
 
+void UnwrappedLineParser::parseStatementMacro()
+{
+  nextToken();
+  if (FormatTok->is(tok::l_paren))
+parseParens();
+  if (FormatTok->is(tok::semi))
+nextToken();
+  addUnwrappedLine();
+}
+
 LLVM_ATTRIBUTE_UNUSED static void printDebugInfo(const UnwrappedLine &Line,
  StringRef Prefix = "") {
   llvm::dbgs() << Prefix << "Line(" << Line.Level
Index: lib/Format/FormatTokenLexer.h
===
--- lib/Format/FormatTokenLexer.h
+++ lib/Format/FormatTokenLexer.h
@@ -22,6 +22,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Format/Format.h"
 #include "llvm/Support/Regex.h"
+#include "llvm/ADT/MapVector.h"
 
 #include 
 
@@ -99,7 +100,8 @@
   // Index (in 'Tokens') of the last token that starts a new line.
   unsigned FirstInLineIndex;
   SmallVector Tokens;
-  SmallVector ForEachMacros;
+
+  llvm::SmallMapVector Macros

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 158309.
Typz added a comment.
Herald added a subscriber: acoomans.

Regeneration documentation


Repository:
  rC Clang

https://reviews.llvm.org/D37813

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -53,6 +53,7 @@
 "  int i;\n"
 "  int j;\n"
 "}"));
+
   EXPECT_EQ("namespace {\n"
 "  int i;\n"
 "  int j;\n"
@@ -249,6 +250,85 @@
 "// unrelated"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, AddsMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+
+  EXPECT_EQ("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A)",
+fixNamespaceEndComments("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(\"Test1\")",
+fixNamespaceEndComments("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+}
+
 TEST_F(NamespaceEndCommentsFixerTest, AddsNewlineIfNeeded) {
   EXPECT_EQ("namespace A {\n"
 "  int i;\n"
@@ -381,6 +461,54 @@
 "}; /* unnamed namespace */"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, KeepsValidMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"} // end anonymous TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"} // end anonymous TESTSUITE()",
+

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 158313.
Typz added a comment.
Herald added a subscriber: acoomans.

Regenerate documentation


Repository:
  rC Clang

https://reviews.llvm.org/D43183

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/UnwrappedLineParser.cpp
  lib/Format/UnwrappedLineParser.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1010,6 +1010,96 @@
"  return;\n"
"}",
getLLVMStyleWithColumns(34));
+
+  FormatStyle indentClosingBrace = getLLVMStyle();
+  indentClosingBrace.CaseBlockIndent = FormatStyle::CBIS_ClosingBrace;
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "  f();\n"
+   "  break;\n"
+   "}",
+   indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "  f();\n"
+   "  break;\n"
+   "  }\n"
+   "case 2: {\n"
+   "  break;\n"
+   "  }\n"
+   "}",
+   indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "  f();\n"
+   "  } break;\n"
+   "case 2: {\n"
+   "  } break;\n"
+   "}",
+   indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "  f();\n"
+   "  }\n"
+   "  g();\n"
+   "  break;\n"
+   "}",
+   indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "  f();\n"
+   "  {\n"
+   "g();\n"
+   "h();\n"
+   "  }\n"
+   "  break;\n"
+   "}",
+   indentClosingBrace);
+
+  FormatStyle indentBlock = getLLVMStyle();
+  indentBlock.CaseBlockIndent = FormatStyle::CBIS_Block;
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "  f();\n"
+   "  break;\n"
+   "}",
+   indentBlock);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "f();\n"
+   "break;\n"
+   "  }\n"
+   "case 2: {\n"
+   "break;\n"
+   "  }\n"
+   "}",
+   indentBlock);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "f();\n"
+   "  } break;\n"
+   "case 2: {\n"
+   "  } break;\n"
+   "}",
+   indentBlock);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "f();\n"
+   "  }\n"
+   "  g();\n"
+   "  break;\n"
+   "}",
+   indentBlock);
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "  f();\n"
+   "  {\n"
+   "g();\n"
+   "h();\n"
+   "  }\n"
+   "  break;\n"
+   "}",
+   indentBlock);
 }
 
 TEST_F(FormatTest, CaseRanges) {
@@ -10639,6 +10729,13 @@
   CHECK_PARSE("AllowShortFunctionsOnASingleLine: true",
   AllowShortFunctionsOnASingleLine, FormatStyle::SFS_All);
 
+  Style.CaseBlockIndent = FormatStyle::CBIS_Block;
+  CHECK_PARSE("CaseBlockIndent: None", CaseBlockIndent, FormatStyle::CBIS_None);
+  CHECK_PARSE("CaseBlockIndent: ClosingBrace", CaseBlockIndent,
+  FormatStyle::CBIS_ClosingBrace);
+  CHECK_PARSE("CaseBlockIndent: Block", CaseBlockIndent,
+  FormatStyle::CBIS_Block);
+
   Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
   CHECK_PARSE("SpaceBeforeParens: Never", SpaceBeforeParens,
   FormatStyle::SBPO_Never);
Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -88,6 +88,10 @@
   void parseFile();
   void parseLevel(bool HasOpeningBrace);
   void parseBlock(bool MustBeDeclaration, bool AddLevel = true,
+  bool MunchSemi = true) {
+parseBlock(MustBeDeclaration, AddLevel ? 1 : 0, MunchSemi);
+  }
+  void parseBlock(bool MustBeDeclaration, int AddLevel,
   bool MunchSemi = true);
   void parseChildBlock();
   void parsePPDirective();
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -515,7 +515,7 @@
   return h;
 }
 
-void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
+void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, int AddLevel,

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Ping!

This has been stale for a while now. Last comment indicate this is not the 
right approach, but some prototyping has been done a more generic approach, 
with no timeline.
I understand this, but in the meantime the problem is still there, with no 
solution at the moment...

Can we move this forward in the mean time, possibly marking the API as 'beta', 
'unstable' or another way of saying it is subject to change. It can be removed 
when there is a better solution.
Or maybe there is now a timeline for the better solution? or the prototype is 
available somewhere, with some documentation on limits/works to be done, so 
that work can continue from there?


Repository:
  rC Clang

https://reviews.llvm.org/D37813



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D50078#1184015, @djasper wrote:

> - I'd be ok with the suggestion for BreakBeforeTernaryOperators=true


Just to be clear, which suggestion would you be ok with?

  int fooo =  ? 0
   :  ? 1
  : ;

or:

  int fooo =    ? 0
 :  ? 1
: ;



> - I don't think the alignment of "?" and ":" (in the WhitespaceManager) 
> should be always on. I think we'd need a flag for that part

No problem, I'll add the option.


Repository:
  rC Clang

https://reviews.llvm.org/D50078



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D50078#1184159, @krasimir wrote:

> Could you clarify how each piece is supposed to be aligned in these examples?
>  This is what makes me happy:
>
>   // column  limit V
>   int a = condition1 ? result1
> : conditio2 ? result2
> : loocondition 
>   ? result2
>   : dition3 ? resul3
> : resul4;
>
>
>
>
>   // column  limit V
>   int a = condition1 
>   ? loresult1
>   : conditio2 ? result2
>   : result4;
>


It gives the following:

  // column  limit V
  int a = condition1 ? result1
: conditio2 ? result2
: loocondition
? result2
: dition3 ? resul3
  : resul4;
  
  // column  limit V
  int a = condition1
? loresult1
: conditio2 ? result2
: result4;

i.e. the long result is wrapped and gets an extra indentation.
I have tried quite a bit to "fall back" to the old behavior when there is this 
kind of wrapping, but this always created other situations which got brocken 
because of this: so finally I choose to stay consistent, and apply the same 
behavior whenever there are chained conditionals.

> When BreakBeforeTernaryOperators is false:
> 
>   int a = condition1 ? result1 :
>   conditio2 ? result2 :
>   ditino3 ? resul3 :
> result4;

This ones is indeed aligned like this.


Repository:
  rC Clang

https://reviews.llvm.org/D50078



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

[PATCH] D50147: clang-format: support external styles

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Typz added reviewers: krasimir, djasper, klimek.
Herald added a subscriber: acoomans.

This patch allows defining external styles, which can be used exactly
like the embedded styles (llvm, google, mozilla...).

These styles are clang-format files installed either systemwide in
/usr/local/share/clang-format, or per-user in ~/.local/share/clang-
format. These can be specified by simply using the name of the file,
and clang-format will search the directories for the style:

  clang-format -style=foo-1.0

The patch also allows loading specifying a file name directly, either
relative or absolute:

  clang-format -style=/home/clang-format-styles/foo-1.0
  clang-format -style=styles/foo-1.0

This works also in `BaseOnStyle` field, which allows defining compagny-
wide (and possibly versionned) clang-format styles, without having to
maintain many copies in each repository: each repository will simply
need to store a short .clang-format, which simply references the
compagny-wide style.

The drawback is that these style need to be installed on each computer,
but this may be automated through an OS package. In any case, the error
cannot be ignored, as the user will be presented with an error message
if he does not have the style.

NOTE: this may be further improved by also allowing URL (http://,
git://...) in this field, which would allow clang-format to automatically
download the missing styles.


Repository:
  rC Clang

https://reviews.llvm.org/D50147

Files:
  include/clang/Format/Format.h
  lib/Basic/VirtualFileSystem.cpp
  lib/Format/Format.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12113,6 +12113,61 @@
   ASSERT_EQ(*Style1, getGoogleStyle());
 }
 
+TEST(FormatStyle, GetExternalStyle) {
+  vfs::InMemoryFileSystem FS;
+  // Test 1: format file in /usr/local/share/clang-format/
+  ASSERT_TRUE(
+  FS.addFile("/usr/local/share/clang-format/style1", 0,
+ llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google")));
+  auto Style1 = getStyle("style1", "", "LLVM", "", &FS);
+  ASSERT_TRUE((bool)Style1);
+  ASSERT_EQ(*Style1, getGoogleStyle());
+
+  // Test 2: format file in ~/.local/share/clang-format/
+  ASSERT_TRUE(
+  FS.addFile("~/.local/share/clang-format/style2", 0,
+ llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google")));
+  auto Style2 = getStyle("style2", "", "LLVM", "", &FS);
+  ASSERT_TRUE((bool)Style2);
+  ASSERT_EQ(*Style2, getGoogleStyle());
+
+  // Test 3: format file in absolute path
+  ASSERT_TRUE(
+  FS.addFile("/clang-format-styles/style3", 0,
+ llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google")));
+  auto Style3 = getStyle("/clang-format-styles/style3", "", "LLVM", "", &FS);
+  ASSERT_TRUE((bool)Style3);
+  ASSERT_EQ(*Style3, getGoogleStyle());
+
+  // Test 4: format file in relative path
+  ASSERT_TRUE(
+  FS.addFile("/home/clang-format-styles/style4", 0,
+ llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google")));
+  FS.setCurrentWorkingDirectory("/home/clang-format-styles");
+  auto Style4 = getStyle("style4", "", "LLVM", "", &FS);
+  ASSERT_TRUE((bool)Style4);
+  ASSERT_EQ(*Style4, getGoogleStyle());
+
+  // Test 5: file does not exist
+  auto Style5 = getStyle("style5", "", "LLVM", "", &FS);
+  ASSERT_FALSE((bool)Style5);
+  llvm::consumeError(Style5.takeError());
+
+  // Test 6: absolute file does not exist
+  auto Style6 = getStyle("/style6", "", "LLVM", "", &FS);
+  ASSERT_FALSE((bool)Style6);
+  llvm::consumeError(Style6.takeError());
+
+  // Test 7: file is not a format style
+  ASSERT_TRUE(
+  FS.addFile("/usr/local/share/clang-format/nostyle", 0,
+ llvm::MemoryBuffer::getMemBuffer("This is not a style...")));
+  FS.setCurrentWorkingDirectory("/home/clang-format-styles");
+  auto Style7 = getStyle("nostyle", "", "LLVM", "", &FS);
+  ASSERT_FALSE((bool)Style7);
+  llvm::consumeError(Style7.takeError());
+}
+
 TEST(FormatStyle, GetStyleOfFile) {
   vfs::InMemoryFileSystem FS;
   // Test 1: format file in the same directory.
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -928,8 +928,42 @@
   return NoStyle;
 }
 
+// Try loading config file from path, in argument to -style and BasedOnStyle.
+bool loadSystemStyle(StringRef Name, FormatStyle *Style,
+ vfs::FileSystem *FS = nullptr) {
+  if (!FS) {
+FS = vfs::getRealFileSystem().get();
+  }
+  const llvm::SmallVector paths = {
+{"/usr/local/share/clang-format/", Name},
+{"~/.local/share/clang-format/", Name},
+Name
+  };
+  for (Twine Path: paths) {
+llvm::SmallVector RealPath;
+Twine ConfigFile{!FS->getRealPath(Path, RealPath) ? RealPath : Path};
+if (FS->exists(ConfigFile)) {
+  l

[PATCH] D32525: [clang-format] Add SpaceBeforeColon option

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 132963.
Typz added a comment.

Split the option into 3 separate options: SpaceBeforeCtorInitializerColon, 
SpaceBeforeInheritanceColon and SpaceBeforeRangeBasedForLoopColon.
This makes each option clearer and more consistent, with no ambiguities due to 
interractions with other options.


https://reviews.llvm.org/D32525

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -7519,6 +7519,91 @@
   verifyFormat("a or_eq 8;", Spaces);
 }
 
+TEST_F(FormatTest, ConfigurableSpaceBeforeColon) {
+  verifyFormat("class Foo : public Bar {};");
+  verifyFormat("Foo::Foo() : foo(1) {}");
+  verifyFormat("for (auto a : b) {\n}");
+  verifyFormat("int x = a ? b : c;");
+  verifyFormat("{\n"
+   "label0:\n"
+   "  int x = 0;\n"
+   "}");
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "default:\n"
+   "}");
+
+  FormatStyle CtorInitializerStyle = getLLVMStyle();
+  CtorInitializerStyle.SpaceBeforeCtorInitializerColon = false;
+  verifyFormat("class Foo : public Bar {};", CtorInitializerStyle);
+  verifyFormat("Foo::Foo(): foo(1) {}", CtorInitializerStyle);
+  verifyFormat("for (auto a : b) {\n}", CtorInitializerStyle);
+  verifyFormat("int x = a ? b : c;", CtorInitializerStyle);
+  verifyFormat("{\n"
+   "label1:\n"
+   "  int x = 0;\n"
+   "}",
+   CtorInitializerStyle);
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "default:\n"
+   "}",
+   CtorInitializerStyle);
+
+  FormatStyle InheritanceStyle = getLLVMStyle();
+  InheritanceStyle.SpaceBeforeInheritanceColon = false;
+  verifyFormat("class Foo: public Bar {};", InheritanceStyle);
+  verifyFormat("Foo::Foo() : foo(1) {}", InheritanceStyle);
+  verifyFormat("for (auto a : b) {\n}", InheritanceStyle);
+  verifyFormat("int x = a ? b : c;", InheritanceStyle);
+  verifyFormat("{\n"
+   "label2:\n"
+   "  int x = 0;\n"
+   "}",
+   InheritanceStyle);
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "default:\n"
+   "}",
+   InheritanceStyle);
+
+  FormatStyle ForLoopStyle = getLLVMStyle();
+  ForLoopStyle.SpaceBeforeRangeBasedForLoopColon = false;
+  verifyFormat("class Foo : public Bar {};", ForLoopStyle);
+  verifyFormat("Foo::Foo() : foo(1) {}", ForLoopStyle);
+  verifyFormat("for (auto a: b) {\n}", ForLoopStyle);
+  verifyFormat("int x = a ? b : c;", ForLoopStyle);
+  verifyFormat("{\n"
+   "label2:\n"
+   "  int x = 0;\n"
+   "}",
+   ForLoopStyle);
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "default:\n"
+   "}",
+   ForLoopStyle);
+
+  FormatStyle NoSpaceStyle = getLLVMStyle();
+  NoSpaceStyle.SpaceBeforeCtorInitializerColon = false;
+  NoSpaceStyle.SpaceBeforeInheritanceColon = false;
+  NoSpaceStyle.SpaceBeforeRangeBasedForLoopColon = false;
+  verifyFormat("class Foo: public Bar {};", NoSpaceStyle);
+  verifyFormat("Foo::Foo(): foo(1) {}", NoSpaceStyle);
+  verifyFormat("for (auto a: b) {\n}", NoSpaceStyle);
+  verifyFormat("int x = a ? b : c;", NoSpaceStyle);
+  verifyFormat("{\n"
+   "label3:\n"
+   "  int x = 0;\n"
+   "}",
+   NoSpaceStyle);
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "default:\n"
+   "}",
+   NoSpaceStyle);
+}
+
 TEST_F(FormatTest, AlignConsecutiveAssignments) {
   FormatStyle Alignment = getLLVMStyle();
   Alignment.AlignConsecutiveAssignments = false;
@@ -8703,6 +8788,9 @@
   CHECK_PARSE_BOOL(SpaceAfterCStyleCast);
   CHECK_PARSE_BOOL(SpaceAfterTemplateKeyword);
   CHECK_PARSE_BOOL(SpaceBeforeAssignmentOperators);
+  CHECK_PARSE_BOOL(SpaceBeforeCtorInitializerColon);
+  CHECK_PARSE_BOOL(SpaceBeforeInheritanceColon);
+  CHECK_PARSE_BOOL(SpaceBeforeRangeBasedForLoopColon);
 
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterClass);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterControlStatement);
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2340,8 +2340,15 @@
 return true;
   if (Right.is(tok::comma))
 return false;
-  if (Right.isOneOf(TT_CtorInitializerColon, TT_ObjCBlockLParen))
+  if (Right.is(TT_ObjCBlockLParen))
 return true;
+  if (Right.is(TT_CtorInitializerColon))
+return Style.SpaceBeforeCtorInitializerColon;
+  if (Right.is(TT_InheritanceColon) && !Style.SpaceBeforeInheritanceColon)
+return false;
+  if (Right.is(TT_

[PATCH] D32525: [clang-format] Add SpaceBeforeColon option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Indeed, I have yet find more precisely documented coding rules which require 
this format, but I thought I could at least address the non-precise aspect of 
the patch itself in the mean-time.


https://reviews.llvm.org/D32525



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

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Typz added reviewers: djasper, krasimir, klimek.

This option replaces the BreakBeforeInheritanceComma option with an
enum, thus introducing a mode where the colon stays on the same line as
constructor declaration:

  // When it fits on line:
  class A : public B, public C {
...
  };
  
  // When it does not fit:
  class A :
  public B,
  public C {
...
  };

This matches the behavior of the `BreakConstructorInitializers` option,
introduced in https://reviews.llvm.org/D32479.


Repository:
  rC Clang

https://reviews.llvm.org/D43015

Files:
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/Format.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1286,15 +1286,40 @@
   verifyFormat("class ::A::B {};");
 }
 
-TEST_F(FormatTest, BreakBeforeInheritanceComma) {
-  FormatStyle StyleWithInheritanceBreak = getLLVMStyle();
-  StyleWithInheritanceBreak.BreakBeforeInheritanceComma = true;
-
-  verifyFormat("class MyClass : public X {};", StyleWithInheritanceBreak);
+TEST_F(FormatTest, BreakInheritanceStyle) {
+  FormatStyle StyleWithInheritanceBreakBeforeComma = getLLVMStyle();
+  StyleWithInheritanceBreakBeforeComma.BreakInheritanceList =
+  FormatStyle::BILS_BeforeComma;
+  verifyFormat("class MyClass : public X {};",
+   StyleWithInheritanceBreakBeforeComma);
   verifyFormat("class MyClass\n"
": public X\n"
", public Y {};",
-   StyleWithInheritanceBreak);
+   StyleWithInheritanceBreakBeforeComma);
+  verifyFormat("class AA\n"
+   ": public BB\n"
+   ", public CC {};",
+   StyleWithInheritanceBreakBeforeComma);
+  verifyFormat("struct a\n"
+   ": public aaa< // break\n"
+   "  > {};",
+   StyleWithInheritanceBreakBeforeComma);
+
+  FormatStyle StyleWithInheritanceBreakAfterColon = getLLVMStyle();
+  StyleWithInheritanceBreakAfterColon.BreakInheritanceList =
+  FormatStyle::BILS_AfterColon;
+  verifyFormat("class MyClass : public X {};",
+   StyleWithInheritanceBreakAfterColon);
+  verifyFormat("class MyClass : public X, public Y {};",
+   StyleWithInheritanceBreakAfterColon);
+  verifyFormat("class AA :\n"
+   "public BB,\n"
+   "public CC {};",
+   StyleWithInheritanceBreakAfterColon);
+  verifyFormat("struct a :\n"
+   "public aaa< // break\n"
+   "> {};",
+   StyleWithInheritanceBreakAfterColon);
 }
 
 TEST_F(FormatTest, FormatsVariableDeclarationsAfterStructOrClass) {
@@ -10172,7 +10197,6 @@
   CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
   CHECK_PARSE_BOOL(BreakBeforeTernaryOperators);
   CHECK_PARSE_BOOL(BreakStringLiterals);
-  CHECK_PARSE_BOOL(BreakBeforeInheritanceComma)
   CHECK_PARSE_BOOL(CompactNamespaces);
   CHECK_PARSE_BOOL(ConstructorInitializerAllOnOneLineOrOnePerLine);
   CHECK_PARSE_BOOL(DerivePointerAlignment);
@@ -10284,6 +10308,17 @@
   CHECK_PARSE("BreakConstructorInitializersBeforeComma: true",
   BreakConstructorInitializers, FormatStyle::BCIS_BeforeComma);
 
+  Style.BreakInheritanceList = FormatStyle::BILS_BeforeColon;
+  CHECK_PARSE("BreakInheritanceList: BeforeComma",
+  BreakInheritanceList, FormatStyle::BILS_BeforeComma);
+  CHECK_PARSE("BreakInheritanceList: AfterColon",
+  BreakInheritanceList, FormatStyle::BILS_AfterColon);
+  CHECK_PARSE("BreakInheritanceList: BeforeColon",
+  BreakInheritanceList, FormatStyle::BILS_BeforeColon);
+  // For backward compatibility:
+  CHECK_PARSE("BreakBeforeInheritanceComma: true",
+  BreakInheritanceList, FormatStyle::BILS_BeforeComma);
+
   Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
   CHECK_PARSE("AlignAfterOpenBracket: Align", AlignAfterOpenBracket,
   FormatStyle::BAS_Align);
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2652,7 +2652,8 @@
   !Style.ConstructorInitializerAllOnOneLineOrOnePerLine)
 return true;
   // Break only if we have multiple inheritance.
-  if (Style.BreakBeforeInheritanceComma && Right.is(TT_InheritanceComma))
+  if (Style.BreakInheritanceList == FormatStyle::BILS_BeforeComma &&
+  Right.is(TT_InheritanceComma))
 return true;
   if (Right.is(tok::string_literal) && Right.TokenTe

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 133204.
Typz added a comment.

Fix indentation of inheritance list, which is actually based on 
`ConstructorInitializerIndentWidth`.

Maybe a new option (`InheritanceListIndentWidth`) should be used instead?


Repository:
  rC Clang

https://reviews.llvm.org/D43015

Files:
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/Format.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1286,15 +1286,40 @@
   verifyFormat("class ::A::B {};");
 }
 
-TEST_F(FormatTest, BreakBeforeInheritanceComma) {
-  FormatStyle StyleWithInheritanceBreak = getLLVMStyle();
-  StyleWithInheritanceBreak.BreakBeforeInheritanceComma = true;
-
-  verifyFormat("class MyClass : public X {};", StyleWithInheritanceBreak);
+TEST_F(FormatTest, BreakInheritanceStyle) {
+  FormatStyle StyleWithInheritanceBreakBeforeComma = getLLVMStyle();
+  StyleWithInheritanceBreakBeforeComma.BreakInheritanceList =
+  FormatStyle::BILS_BeforeComma;
+  verifyFormat("class MyClass : public X {};",
+   StyleWithInheritanceBreakBeforeComma);
   verifyFormat("class MyClass\n"
": public X\n"
", public Y {};",
-   StyleWithInheritanceBreak);
+   StyleWithInheritanceBreakBeforeComma);
+  verifyFormat("class AA\n"
+   ": public BB\n"
+   ", public CC {};",
+   StyleWithInheritanceBreakBeforeComma);
+  verifyFormat("struct a\n"
+   ": public aaa< // break\n"
+   "  > {};",
+   StyleWithInheritanceBreakBeforeComma);
+
+  FormatStyle StyleWithInheritanceBreakAfterColon = getLLVMStyle();
+  StyleWithInheritanceBreakAfterColon.BreakInheritanceList =
+  FormatStyle::BILS_AfterColon;
+  verifyFormat("class MyClass : public X {};",
+   StyleWithInheritanceBreakAfterColon);
+  verifyFormat("class MyClass : public X, public Y {};",
+   StyleWithInheritanceBreakAfterColon);
+  verifyFormat("class AA :\n"
+   "public BB,\n"
+   "public CC {};",
+   StyleWithInheritanceBreakAfterColon);
+  verifyFormat("struct a :\n"
+   "public aaa< // break\n"
+   "> {};",
+   StyleWithInheritanceBreakAfterColon);
 }
 
 TEST_F(FormatTest, FormatsVariableDeclarationsAfterStructOrClass) {
@@ -3618,6 +3643,23 @@
"  aa,\n"
"  bb {}",
Style);
+
+  // `ConstructorInitializerIndentWidth` actually applies to InheritanceList as well
+  Style.BreakInheritanceList = FormatStyle::BILS_BeforeColon;
+  verifyFormat("class SomeClass\n"
+   "  : public aa,\n"
+   "public bb {};",
+   Style);
+  Style.BreakInheritanceList = FormatStyle::BILS_BeforeComma;
+  verifyFormat("class SomeClass\n"
+   "  : public aa\n"
+   "  , public bb {};",
+   Style);
+  Style.BreakInheritanceList = FormatStyle::BILS_AfterColon;
+  verifyFormat("class SomeClass :\n"
+   "  public aa,\n"
+   "  public bb {};",
+   Style);
 }
 
 #ifndef EXPENSIVE_CHECKS
@@ -10172,7 +10214,6 @@
   CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
   CHECK_PARSE_BOOL(BreakBeforeTernaryOperators);
   CHECK_PARSE_BOOL(BreakStringLiterals);
-  CHECK_PARSE_BOOL(BreakBeforeInheritanceComma)
   CHECK_PARSE_BOOL(CompactNamespaces);
   CHECK_PARSE_BOOL(ConstructorInitializerAllOnOneLineOrOnePerLine);
   CHECK_PARSE_BOOL(DerivePointerAlignment);
@@ -10284,6 +10325,17 @@
   CHECK_PARSE("BreakConstructorInitializersBeforeComma: true",
   BreakConstructorInitializers, FormatStyle::BCIS_BeforeComma);
 
+  Style.BreakInheritanceList = FormatStyle::BILS_BeforeColon;
+  CHECK_PARSE("BreakInheritanceList: BeforeComma",
+  BreakInheritanceList, FormatStyle::BILS_BeforeComma);
+  CHECK_PARSE("BreakInheritanceList: AfterColon",
+  BreakInheritanceList, FormatStyle::BILS_AfterColon);
+  CHECK_PARSE("BreakInheritanceList: BeforeColon",
+  BreakInheritanceList, FormatStyle::BI

[PATCH] D43114: clang-format: fix formatting of ObjC @synchronized blocks

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Typz added reviewers: krasimir, djasper, klimek.

The blocks used to be formatted using the "default" behavior, and would
thus be mistaken for function calls followed by blocks: this could lead
to unexpected inlining of the block and extra line-break before the
opening brace.

They are now formatted similarly to `@autoreleasepool` blocks, as
expected:

  @synchronized(self) {
  f();
  }


Repository:
  rC Clang

https://reviews.llvm.org/D43114

Files:
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -189,6 +189,24 @@
"}\n");
 }
 
+TEST_F(FormatTestObjC, FormatObjCSynchronized) {
+  verifyFormat("@synchronized(self) {\n"
+   "  f();\n"
+   "}\n"
+   "@synchronized(self) {\n"
+   "  f();\n"
+   "}\n");
+  Style.BreakBeforeBraces = FormatStyle::BS_Allman;
+  verifyFormat("@synchronized(self)\n"
+   "{\n"
+   "  f();\n"
+   "}\n"
+   "@synchronized(self)\n"
+   "{\n"
+   "  f();\n"
+   "}\n");
+}
+
 TEST_F(FormatTestObjC, FormatObjCInterface) {
   verifyFormat("@interface Foo : NSObject  {\n"
"@public\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1121,6 +1121,18 @@
 }
 addUnwrappedLine();
 return;
+  case tok::objc_synchronized:
+nextToken();
+if (FormatTok->Tok.is(tok::l_paren))
+   // Skip synchronization object
+   parseParens();
+if (FormatTok->Tok.is(tok::l_brace)) {
+  if (Style.BraceWrapping.AfterObjCDeclaration)
+addUnwrappedLine();
+  parseBlock(/*MustBeDeclaration=*/false);
+}
+addUnwrappedLine();
+return;
   case tok::objc_try:
 // This branch isn't strictly necessary (the kw_try case below would
 // do this too after the tok::at is parsed above).  But be explicit.


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -189,6 +189,24 @@
"}\n");
 }
 
+TEST_F(FormatTestObjC, FormatObjCSynchronized) {
+  verifyFormat("@synchronized(self) {\n"
+   "  f();\n"
+   "}\n"
+   "@synchronized(self) {\n"
+   "  f();\n"
+   "}\n");
+  Style.BreakBeforeBraces = FormatStyle::BS_Allman;
+  verifyFormat("@synchronized(self)\n"
+   "{\n"
+   "  f();\n"
+   "}\n"
+   "@synchronized(self)\n"
+   "{\n"
+   "  f();\n"
+   "}\n");
+}
+
 TEST_F(FormatTestObjC, FormatObjCInterface) {
   verifyFormat("@interface Foo : NSObject  {\n"
"@public\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1121,6 +1121,18 @@
 }
 addUnwrappedLine();
 return;
+  case tok::objc_synchronized:
+nextToken();
+if (FormatTok->Tok.is(tok::l_paren))
+   // Skip synchronization object
+   parseParens();
+if (FormatTok->Tok.is(tok::l_brace)) {
+  if (Style.BraceWrapping.AfterObjCDeclaration)
+addUnwrappedLine();
+  parseBlock(/*MustBeDeclaration=*/false);
+}
+addUnwrappedLine();
+return;
   case tok::objc_try:
 // This branch isn't strictly necessary (the kw_try case below would
 // do this too after the tok::at is parsed above).  But be explicit.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43114: clang-format: fix formatting of ObjC @synchronized blocks

[Francois Ferrand via Phabricator via cfe-commits]

Typz added inline comments.



Comment at: lib/Format/UnwrappedLineParser.cpp:1130
+if (FormatTok->Tok.is(tok::l_brace)) {
+  if (Style.BraceWrapping.AfterObjCDeclaration)
+addUnwrappedLine();

Wondering if formatting with this style is appropriate: the clang-format doc 
points in that direction, but it seems to me both `@synchronized` and 
`@autoreleasepool` are more akin to "control statements".

For consistency (esp. in ObjC++ code), as a user, I would tend to have these 
blocks indented like control statements while interfaces/implementations blocks 
would be indented like classes/structs.

So maybe it would be better to introduce a new BraceWrapping style 
'AfterObjCSpecialBlock` to control these cases, matching the possibilities that 
are given for control statements vs classes. What do you think?


Repository:
  rC Clang

https://reviews.llvm.org/D43114



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

[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


Repository:
  rC Clang

https://reviews.llvm.org/D42684



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

[PATCH] D42787: clang-format: do not add extra indent when wrapping last parameter

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D42787#1000687, @krasimir wrote:

> We could adapt the single-argument version instead, turning:
>
>   foo(bb +
>   c);
>
>
> into:
>
>   foo(bb +
>   c);
>


We could indeed, but that would probably make neither djasper or me happy :-)


Repository:
  rC Clang

https://reviews.llvm.org/D42787



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

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Typz added reviewers: krasimir, djasper, klimek.

When the target object expression is short and the first selector name is
long, clang-format used to break the colon alignment:

  [I performSelectorOnMainThread:@selector(loadAccessories)
   withObject:nil
waitUntilDone:false];

This happens because the colon is placed at `ContinuationIndent +
LongestObjCSelectorName`, so that any selector can be wrapped. This is
however not needed in case the longest selector is the first one, and not
wrapped.

To overcome this, this patch does not include the first selector in
`LongestObjCSelectorName` computation (in TokenAnnotator), and lets
ContinuationIndenter account decide how to account for the first selector
when wrapping. (Note this was already partly the case, see line 521 of
ContinuationIndenter.cpp)

This way, the code gets properly aligned whenever possible without
breaking the continuation indent.

  [I performSelectorOnMainThread:@selector(loadAccessories)
  withObject:nil
   waitUntilDone:false];
  [I // force break
  performSelectorOnMainThread:@selector(loadAccessories)
   withObject:nil
waitUntilDone:false];
  [I perform:@selector(loadAccessories)
  withSelectorOnMainThread:true
 waitUntilDone:false];


Repository:
  rC Clang

https://reviews.llvm.org/D43121

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -652,8 +652,8 @@
   // Formats pair-parameters.
   verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
   verifyFormat("[I drawRectOn:surface //\n"
-   "ofSize:aa:bbb\n"
-   "  atOrigin:cc:dd];");
+   "   ofSize:aa:bbb\n"
+   " atOrigin:cc:dd];");
 
   Style.ColumnLimit = 70;
   verifyFormat(
@@ -686,7 +686,19 @@
   "  backing:NSBackingStoreBuffered\n"
   "defer:NO]);\n"
   "}");
-}
+
+  // Respect continuation indent and colon alignment (e.g. when object name is
+  // short, and first selector is the longest one)
+  Style = getLLVMStyle();
+  Style.Language = FormatStyle::LK_ObjC;
+  Style.ContinuationIndentWidth = 8;
+  verifyFormat("[self performSelectorOnMainThread:@selector(loadAccessories)\n"
+   "   withObject:nil\n"
+   "waitUntilDone:false];");
+  verifyFormat("[self performSelector:@selector(loadAccessories)\n"
+   "withObjectOnMainThread:nil\n"
+   " waitUntilDone:false];");
+  }
 
 TEST_F(FormatTestObjC, ObjCAt) {
   verifyFormat("@autoreleasepool");
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -571,12 +571,12 @@
 BeforePrevious->is(tok::r_square) ||
 Contexts.back().LongestObjCSelectorName == 0) {
   Tok->Previous->Type = TT_SelectorName;
-  if (Tok->Previous->ColumnWidth >
-  Contexts.back().LongestObjCSelectorName)
-Contexts.back().LongestObjCSelectorName =
-Tok->Previous->ColumnWidth;
   if (!Contexts.back().FirstObjCSelectorName)
 Contexts.back().FirstObjCSelectorName = Tok->Previous;
+  else if (Tok->Previous->ColumnWidth >
+   Contexts.back().LongestObjCSelectorName)
+Contexts.back().LongestObjCSelectorName =
+Tok->Previous->ColumnWidth;
 }
   } else if (Contexts.back().ColonIsForRangeExpr) {
 Tok->Type = TT_RangeBasedForLoopColon;
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -696,7 +696,8 @@
  ? std::max(State.Stack.back().Indent,
 State.FirstIndent + Style.ContinuationIndentWidth)
  : State.Stack.back().Indent) +
-NextNonComment->LongestObjCSelectorName;
+std::max(NextNonComment->LongestObjCSelectorName,
+ unsigned(NextNonComment->TokenText.size()));
   }
 } else if (State.Stack.back().AlignColons &&
State.Stack.back().ColonPos <= NextNonComment->ColumnWidth) {
@@ -895,7 +896,8 @@
   ? std::max(State.Stack.back().Indent,
  State.FirstIndent + Style.ContinuationIndentWidth)
   : State.Stack.back().Indent) +
- NextNonComment->LongestObjCSelectorName -
+ std::max(Next

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 133589.
Typz added a comment.

fix type in commit message


Repository:
  rC Clang

https://reviews.llvm.org/D43121

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -652,8 +652,8 @@
   // Formats pair-parameters.
   verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
   verifyFormat("[I drawRectOn:surface //\n"
-   "ofSize:aa:bbb\n"
-   "  atOrigin:cc:dd];");
+   "   ofSize:aa:bbb\n"
+   " atOrigin:cc:dd];");
 
   Style.ColumnLimit = 70;
   verifyFormat(
@@ -686,7 +686,19 @@
   "  backing:NSBackingStoreBuffered\n"
   "defer:NO]);\n"
   "}");
-}
+
+  // Respect continuation indent and colon alignment (e.g. when object name is
+  // short, and first selector is the longest one)
+  Style = getLLVMStyle();
+  Style.Language = FormatStyle::LK_ObjC;
+  Style.ContinuationIndentWidth = 8;
+  verifyFormat("[self performSelectorOnMainThread:@selector(loadAccessories)\n"
+   "   withObject:nil\n"
+   "waitUntilDone:false];");
+  verifyFormat("[self performSelector:@selector(loadAccessories)\n"
+   "withObjectOnMainThread:nil\n"
+   " waitUntilDone:false];");
+  }
 
 TEST_F(FormatTestObjC, ObjCAt) {
   verifyFormat("@autoreleasepool");
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -571,12 +571,12 @@
 BeforePrevious->is(tok::r_square) ||
 Contexts.back().LongestObjCSelectorName == 0) {
   Tok->Previous->Type = TT_SelectorName;
-  if (Tok->Previous->ColumnWidth >
-  Contexts.back().LongestObjCSelectorName)
-Contexts.back().LongestObjCSelectorName =
-Tok->Previous->ColumnWidth;
   if (!Contexts.back().FirstObjCSelectorName)
 Contexts.back().FirstObjCSelectorName = Tok->Previous;
+  else if (Tok->Previous->ColumnWidth >
+   Contexts.back().LongestObjCSelectorName)
+Contexts.back().LongestObjCSelectorName =
+Tok->Previous->ColumnWidth;
 }
   } else if (Contexts.back().ColonIsForRangeExpr) {
 Tok->Type = TT_RangeBasedForLoopColon;
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -696,7 +696,8 @@
  ? std::max(State.Stack.back().Indent,
 State.FirstIndent + Style.ContinuationIndentWidth)
  : State.Stack.back().Indent) +
-NextNonComment->LongestObjCSelectorName;
+std::max(NextNonComment->LongestObjCSelectorName,
+ unsigned(NextNonComment->TokenText.size()));
   }
 } else if (State.Stack.back().AlignColons &&
State.Stack.back().ColonPos <= NextNonComment->ColumnWidth) {
@@ -895,7 +896,8 @@
   ? std::max(State.Stack.back().Indent,
  State.FirstIndent + Style.ContinuationIndentWidth)
   : State.Stack.back().Indent) +
- NextNonComment->LongestObjCSelectorName -
+ std::max(NextNonComment->LongestObjCSelectorName,
+  unsigned(NextNonComment->TokenText.size())) -
  NextNonComment->ColumnWidth;
 }
 if (!State.Stack.back().AlignColons)


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -652,8 +652,8 @@
   // Formats pair-parameters.
   verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
   verifyFormat("[I drawRectOn:surface //\n"
-   "ofSize:aa:bbb\n"
-   "  atOrigin:cc:dd];");
+   "   ofSize:aa:bbb\n"
+   " atOrigin:cc:dd];");
 
   Style.ColumnLimit = 70;
   verifyFormat(
@@ -686,7 +686,19 @@
   "  backing:NSBackingStoreBuffered\n"
   "defer:NO]);\n"
   "}");
-}
+
+  // Respect continuation indent and colon alignment (e.g. when object name is
+  // short, and first selector is the longest one)
+  Style = getLLVMStyle();
+  Style.Language = FormatStyle::LK_ObjC;
+  Style.ContinuationIndentWidth = 8;
+  verifyFormat("[self performSelectorOnMainThread:@selector(loadAccessories)\n"
+   "   withObject:nil\n"
+ 

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 133596.
Typz marked 2 inline comments as done.
Typz added a comment.

Address review comments


Repository:
  rC Clang

https://reviews.llvm.org/D43121

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -652,8 +652,8 @@
   // Formats pair-parameters.
   verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
   verifyFormat("[I drawRectOn:surface //\n"
-   "ofSize:aa:bbb\n"
-   "  atOrigin:cc:dd];");
+   "   ofSize:aa:bbb\n"
+   " atOrigin:cc:dd];");
 
   Style.ColumnLimit = 70;
   verifyFormat(
@@ -686,6 +686,26 @@
   "  backing:NSBackingStoreBuffered\n"
   "defer:NO]);\n"
   "}");
+
+  // Respect continuation indent and colon alignment (e.g. when object name is
+  // short, and first selector is the longest one)
+  Style = getLLVMStyle();
+  Style.Language = FormatStyle::LK_ObjC;
+  Style.ContinuationIndentWidth = 8;
+  verifyFormat("[self performSelectorOnMainThread:@selector(loadAccessories)\n"
+   "   withObject:nil\n"
+   "waitUntilDone:false];");
+  verifyFormat("[self performSelector:@selector(loadAccessories)\n"
+   "withObjectOnMainThread:nil\n"
+   " waitUntilDone:false];");
+  verifyFormat("[a\n"
+   "
performSelectorOnMainThread:@selector(loadAccessories)\n"
+   " withObject:nil\n"
+   "  waitUntilDone:false];");
+  verifyFormat("[self // force wrapping\n"
+   "
performSelectorOnMainThread:@selector(loadAccessories)\n"
+   " withObject:nil\n"
+   "  waitUntilDone:false];");
 }
 
 TEST_F(FormatTestObjC, ObjCAt) {
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -571,12 +571,12 @@
 BeforePrevious->is(tok::r_square) ||
 Contexts.back().LongestObjCSelectorName == 0) {
   Tok->Previous->Type = TT_SelectorName;
-  if (Tok->Previous->ColumnWidth >
-  Contexts.back().LongestObjCSelectorName)
-Contexts.back().LongestObjCSelectorName =
-Tok->Previous->ColumnWidth;
   if (!Contexts.back().FirstObjCSelectorName)
 Contexts.back().FirstObjCSelectorName = Tok->Previous;
+  else if (Tok->Previous->ColumnWidth >
+   Contexts.back().LongestObjCSelectorName)
+Contexts.back().LongestObjCSelectorName =
+Tok->Previous->ColumnWidth;
 }
   } else if (Contexts.back().ColonIsForRangeExpr) {
 Tok->Type = TT_RangeBasedForLoopColon;
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -696,7 +696,8 @@
  ? std::max(State.Stack.back().Indent,
 State.FirstIndent + Style.ContinuationIndentWidth)
  : State.Stack.back().Indent) +
-NextNonComment->LongestObjCSelectorName;
+std::max(NextNonComment->LongestObjCSelectorName,
+ NextNonComment->ColumnWidth);
   }
 } else if (State.Stack.back().AlignColons &&
State.Stack.back().ColonPos <= NextNonComment->ColumnWidth) {
@@ -895,7 +896,8 @@
   ? std::max(State.Stack.back().Indent,
  State.FirstIndent + Style.ContinuationIndentWidth)
   : State.Stack.back().Indent) +
- NextNonComment->LongestObjCSelectorName -
+ std::max(NextNonComment->LongestObjCSelectorName,
+  NextNonComment->ColumnWidth) -
  NextNonComment->ColumnWidth;
 }
 if (!State.Stack.back().AlignColons)


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -652,8 +652,8 @@
   // Formats pair-parameters.
   verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
   verifyFormat("[I drawRectOn:surface //\n"
-   "ofSize:aa:bbb\n"
-   "  atOrigin:cc:dd];");
+   "   ofSize:aa:bbb\n"
+   " atOrigin:cc:dd];");
 
   Style.ColumnLimit = 70;
   verifyFormat(
@@ -686,6 +686,26 @@
   "  backing:

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

[Francois Ferrand via Phabricator via cfe-commits]

Typz added inline comments.



Comment at: lib/Format/ContinuationIndenter.cpp:900
+ std::max(NextNonComment->LongestObjCSelectorName,
+  unsigned(NextNonComment->TokenText.size())) -
  NextNonComment->ColumnWidth;

djasper wrote:
> I'd prefer to use std::max( .. )
> 
> (and we generally don't use c-style casts)
this is C++-style C-style cast :-)

anyway changed to use ColumnWidth, as this is also what is used for 
initializing LongestObjCSelectorName.


Repository:
  rC Clang

https://reviews.llvm.org/D43121



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

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 133619.
Typz added a comment.

rebase on latest master.


Repository:
  rC Clang

https://reviews.llvm.org/D43121

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -693,8 +693,8 @@
   // Formats pair-parameters.
   verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
   verifyFormat("[I drawRectOn:surface //\n"
-   "ofSize:aa:bbb\n"
-   "  atOrigin:cc:dd];");
+   "   ofSize:aa:bbb\n"
+   " atOrigin:cc:dd];");
 
   // Inline block as a first argument.
   verifyFormat("[object justBlock:^{\n"
@@ -760,6 +760,26 @@
   "  backing:NSBackingStoreBuffered\n"
   "defer:NO]);\n"
   "}");
+
+  // Respect continuation indent and colon alignment (e.g. when object name is
+  // short, and first selector is the longest one)
+  Style = getLLVMStyle();
+  Style.Language = FormatStyle::LK_ObjC;
+  Style.ContinuationIndentWidth = 8;
+  verifyFormat("[self performSelectorOnMainThread:@selector(loadAccessories)\n"
+   "   withObject:nil\n"
+   "waitUntilDone:false];");
+  verifyFormat("[self performSelector:@selector(loadAccessories)\n"
+   "withObjectOnMainThread:nil\n"
+   " waitUntilDone:false];");
+  verifyFormat("[a\n"
+   "
performSelectorOnMainThread:@selector(loadAccessories)\n"
+   " withObject:nil\n"
+   "  waitUntilDone:false];");
+  verifyFormat("[self // force wrapping\n"
+   "
performSelectorOnMainThread:@selector(loadAccessories)\n"
+   " withObject:nil\n"
+   "  waitUntilDone:false];");
 }
 
 TEST_F(FormatTestObjC, ObjCAt) {
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -591,12 +591,12 @@
 BeforePrevious->is(tok::r_square) ||
 Contexts.back().LongestObjCSelectorName == 0) {
   Tok->Previous->Type = TT_SelectorName;
-  if (Tok->Previous->ColumnWidth >
-  Contexts.back().LongestObjCSelectorName)
-Contexts.back().LongestObjCSelectorName =
-Tok->Previous->ColumnWidth;
   if (!Contexts.back().FirstObjCSelectorName)
 Contexts.back().FirstObjCSelectorName = Tok->Previous;
+  else if (Tok->Previous->ColumnWidth >
+   Contexts.back().LongestObjCSelectorName)
+Contexts.back().LongestObjCSelectorName =
+Tok->Previous->ColumnWidth;
 }
   } else if (Contexts.back().ColonIsForRangeExpr) {
 Tok->Type = TT_RangeBasedForLoopColon;
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -701,7 +701,8 @@
  ? std::max(State.Stack.back().Indent,
 State.FirstIndent + Style.ContinuationIndentWidth)
  : State.Stack.back().Indent) +
-NextNonComment->LongestObjCSelectorName;
+std::max(NextNonComment->LongestObjCSelectorName,
+ NextNonComment->ColumnWidth);
   }
 } else if (State.Stack.back().AlignColons &&
State.Stack.back().ColonPos <= NextNonComment->ColumnWidth) {
@@ -900,7 +901,8 @@
   ? std::max(State.Stack.back().Indent,
  State.FirstIndent + Style.ContinuationIndentWidth)
   : State.Stack.back().Indent) +
- NextNonComment->LongestObjCSelectorName -
+ std::max(NextNonComment->LongestObjCSelectorName,
+  NextNonComment->ColumnWidth) -
  NextNonComment->ColumnWidth;
 }
 if (!State.Stack.back().AlignColons)


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -693,8 +693,8 @@
   // Formats pair-parameters.
   verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
   verifyFormat("[I drawRectOn:surface //\n"
-   "ofSize:aa:bbb\n"
-   "  atOrigin:cc:dd];");
+   "   ofSize:aa:bbb\n"
+   " atOrigin:cc:dd];");
 
   // Inline block as a first argument.
   verifyFormat("[object justBlock:^{\n"
@@ -760,6 +760,26 @

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC324741: clang-format: keep ObjC colon alignment with short 
object name (authored by Typz, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D43121?vs=133619&id=133620#toc

Repository:
  rC Clang

https://reviews.llvm.org/D43121

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -693,8 +693,8 @@
   // Formats pair-parameters.
   verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
   verifyFormat("[I drawRectOn:surface //\n"
-   "ofSize:aa:bbb\n"
-   "  atOrigin:cc:dd];");
+   "   ofSize:aa:bbb\n"
+   " atOrigin:cc:dd];");
 
   // Inline block as a first argument.
   verifyFormat("[object justBlock:^{\n"
@@ -760,6 +760,26 @@
   "  backing:NSBackingStoreBuffered\n"
   "defer:NO]);\n"
   "}");
+
+  // Respect continuation indent and colon alignment (e.g. when object name is
+  // short, and first selector is the longest one)
+  Style = getLLVMStyle();
+  Style.Language = FormatStyle::LK_ObjC;
+  Style.ContinuationIndentWidth = 8;
+  verifyFormat("[self performSelectorOnMainThread:@selector(loadAccessories)\n"
+   "   withObject:nil\n"
+   "waitUntilDone:false];");
+  verifyFormat("[self performSelector:@selector(loadAccessories)\n"
+   "withObjectOnMainThread:nil\n"
+   " waitUntilDone:false];");
+  verifyFormat("[a\n"
+   "
performSelectorOnMainThread:@selector(loadAccessories)\n"
+   " withObject:nil\n"
+   "  waitUntilDone:false];");
+  verifyFormat("[self // force wrapping\n"
+   "
performSelectorOnMainThread:@selector(loadAccessories)\n"
+   " withObject:nil\n"
+   "  waitUntilDone:false];");
 }
 
 TEST_F(FormatTestObjC, ObjCAt) {
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -591,12 +591,12 @@
 BeforePrevious->is(tok::r_square) ||
 Contexts.back().LongestObjCSelectorName == 0) {
   Tok->Previous->Type = TT_SelectorName;
-  if (Tok->Previous->ColumnWidth >
-  Contexts.back().LongestObjCSelectorName)
-Contexts.back().LongestObjCSelectorName =
-Tok->Previous->ColumnWidth;
   if (!Contexts.back().FirstObjCSelectorName)
 Contexts.back().FirstObjCSelectorName = Tok->Previous;
+  else if (Tok->Previous->ColumnWidth >
+   Contexts.back().LongestObjCSelectorName)
+Contexts.back().LongestObjCSelectorName =
+Tok->Previous->ColumnWidth;
 }
   } else if (Contexts.back().ColonIsForRangeExpr) {
 Tok->Type = TT_RangeBasedForLoopColon;
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -701,7 +701,8 @@
  ? std::max(State.Stack.back().Indent,
 State.FirstIndent + Style.ContinuationIndentWidth)
  : State.Stack.back().Indent) +
-NextNonComment->LongestObjCSelectorName;
+std::max(NextNonComment->LongestObjCSelectorName,
+ NextNonComment->ColumnWidth);
   }
 } else if (State.Stack.back().AlignColons &&
State.Stack.back().ColonPos <= NextNonComment->ColumnWidth) {
@@ -900,7 +901,8 @@
   ? std::max(State.Stack.back().Indent,
  State.FirstIndent + Style.ContinuationIndentWidth)
   : State.Stack.back().Indent) +
- NextNonComment->LongestObjCSelectorName -
+ std::max(NextNonComment->LongestObjCSelectorName,
+  NextNonComment->ColumnWidth) -
  NextNonComment->ColumnWidth;
 }
 if (!State.Stack.back().AlignColons)


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -693,8 +693,8 @@
   // Formats pair-parameters.
   verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
   verifyFormat("[I drawRectOn:surface //\n"
-   "ofSize:aa:bbb\n"
-   "  atOrigin:cc:d

[PATCH] D43114: clang-format: fix formatting of ObjC @synchronized blocks

[Francois Ferrand via Phabricator via cfe-commits]

Typz added inline comments.



Comment at: lib/Format/UnwrappedLineParser.cpp:1130
+if (FormatTok->Tok.is(tok::l_brace)) {
+  if (Style.BraceWrapping.AfterObjCDeclaration)
+addUnwrappedLine();

benhamilton wrote:
> Typz wrote:
> > Wondering if formatting with this style is appropriate: the clang-format 
> > doc points in that direction, but it seems to me both `@synchronized` and 
> > `@autoreleasepool` are more akin to "control statements".
> > 
> > For consistency (esp. in ObjC++ code), as a user, I would tend to have 
> > these blocks indented like control statements while 
> > interfaces/implementations blocks would be indented like classes/structs.
> > 
> > So maybe it would be better to introduce a new BraceWrapping style 
> > 'AfterObjCSpecialBlock` to control these cases, matching the possibilities 
> > that are given for control statements vs classes. What do you think?
> Hmm, I definitely agree with that logic. I don't see them acting as 
> declarations in any way, they are definitely like control statements.
> 
Ok, i can change this. Two questions though:
* Should I do this in this patch, or a separate patch? (won't be a big change 
in any case, but it can still be seen as 2 separate issues/changes)
* Should I introduce a new BraceWrapping flag (`AfterObjCSpecialBlock`), or 
simply apply `AfterControlStatement` to these blocks?


Repository:
  rC Clang

https://reviews.llvm.org/D43114



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

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Typz added reviewers: krasimir, djasper, klimek.

When a block is started after a case label, clang-format does add extra
indent to the content of this block: the block content is indented one
level (with respect to the switch) while the closing brace is not
indented, and closes at switch level.

This gives the following code:

  switch (x) {
  case A: {
stuff();
  } break;
  }

This makes it easy to confuse the closing brace of the 'case' with that
the whole 'switch', especially if more code is added after the brace:

  switch (x) {
  case A: {
stuff();
  }
moreStuff();
break;
  }

This patch introduces a new CaseBlockIndent switch, which provides
alternative formatting for these cases:

- `CaseBlockIndent = None` : default behavior, same behavior as before

- `CaseBlockIndent = ClosingBrace` : indent the closing brace to the

same level as its content.

  switch (x) {
  case A: {
stuff();
} break;
  }

- `CaseBlockIndent = Block` : add an extra level of indent for the

content of the block.

  switch (x) {
  case A: {
  stuff();
} break;
  }


Repository:
  rC Clang

https://reviews.llvm.org/D43183

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/UnwrappedLineParser.cpp
  lib/Format/UnwrappedLineParser.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -980,6 +980,96 @@
"  return;\n"
"}",
getLLVMStyleWithColumns(34));
+
+  FormatStyle indentClosingBrace = getLLVMStyle();
+  indentClosingBrace.CaseBlockIndent = FormatStyle::CBIS_ClosingBrace;
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "  f();\n"
+   "  break;\n"
+   "}",
+   indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "  f();\n"
+   "  break;\n"
+   "  }\n"
+   "case 2: {\n"
+   "  break;\n"
+   "  }\n"
+   "}",
+   indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "  f();\n"
+   "  } break;\n"
+   "case 2: {\n"
+   "  } break;\n"
+   "}",
+   indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "  f();\n"
+   "  }\n"
+   "  g();\n"
+   "  break;\n"
+   "}",
+   indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "  f();\n"
+   "  {\n"
+   "g();\n"
+   "h();\n"
+   "  }\n"
+   "  break;\n"
+   "}",
+   indentClosingBrace);
+
+  FormatStyle indentBlock = getLLVMStyle();
+  indentBlock.CaseBlockIndent = FormatStyle::CBIS_Block;
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "  f();\n"
+   "  break;\n"
+   "}",
+   indentBlock);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "f();\n"
+   "break;\n"
+   "  }\n"
+   "case 2: {\n"
+   "break;\n"
+   "  }\n"
+   "}",
+   indentBlock);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "f();\n"
+   "  } break;\n"
+   "case 2: {\n"
+   "  } break;\n"
+   "}",
+   indentBlock);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "f();\n"
+   "  }\n"
+   "  g();\n"
+   "  break;\n"
+   "}",
+   indentBlock);
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "  f();\n"
+   "  {\n"
+   "g();\n"
+   "h();\n"
+   "  }\n"
+   "  break;\n"
+   "}",
+   indentBlock);
 }
 
 TEST_F(FormatTest, CaseRanges) {
@@ -10413,6 +10503,13 @@
   CHECK_PARSE("AllowShortFunctionsOnASingleLine: true",
   AllowShortFunctionsOnASingleLine, FormatStyle::SFS_All);
 
+  Style.CaseBlockIndent = FormatStyle::CBIS_Block;
+  CHECK_PARSE("CaseBlockIndent: None", CaseBlockIndent, FormatStyle::CBIS_None);
+  CHECK_PARSE("CaseBlockIndent: ClosingBrace", CaseBlockIndent,
+  FormatStyle::CBIS_ClosingBrace);
+  CHECK_PARSE("CaseBlockIndent: Block", CaseBlockIndent,
+  FormatStyle::CBIS_Block);
+
   Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
   CHECK_PARSE("SpaceBeforeParens: Never", SpaceBeforeParens,
   FormatStyle::SBPO_Never);
Index: lib/Format/UnwrappedLin

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

> Do you have a reference to style guides recommending any of this?

Unfortunately not... I think this is a really advanced topic, and often not 
recommended way to format code at all (e.g. "if you need a block, you may as 
well use a function"). I can find the current implementation documented in 
google style guide, where there is an extra indent inside switch [e.g. 
`IndentCaseLabels`], which alleviates the ambiguity. But it is not documented 
(or shown) in the LLVM coding style, where it makes the code IMO really 
difficult to follow.

I believe the 2 extra modes I introduce should provide for most cases, leaving 
the decision to user:

- `None` mode matches google-style, and is perfectly fine when 
`IndentCaseLabels == true`
- `ClosingBrace` mode is "logical" with what people may want (e.g. have a scope 
for the whole `case` block), but is not syntaxically correct and may thus 
mislead
- `Block` mode is syntaxically correct, but somewhat breaks the switch layout 
by moving break to different indentation levels depending on the use of braces

Which mode is use is then left to the user, depending on the "goals" of their 
coding style.

Also, maybe this is a small-enough detail that it would be better moved to the 
`BraceWrapping` field, to allow finely tuning the expected behavior (e.g. in 
Custom brace mode) while keeping the 'main' options simple (even though it is 
not technically a way to wrap the braces).


Repository:
  rC Clang

https://reviews.llvm.org/D43183



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

[PATCH] D43232: clang-format: use AfterControlStatement to format ObjC control blocks

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Typz added reviewers: krasimir, djasper, klimek, benhamilton.

ObjC defines `@autoreleasepool` and `@synchronized` control blocks. These
used to be formatted according to the `AfterObjCDeclaration` brace-
wrapping flag, which is not very consistent.

This patch changes the behavior to use the `AfterControlStatement` flag
instead. This should not affect the behavior unless a custom brace
wrapping mode is used.


Repository:
  rC Clang

https://reviews.llvm.org/D43232

Files:
  include/clang/Format/Format.h
  lib/Format/UnwrappedLineParser.cpp


Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1115,7 +1115,7 @@
   case tok::objc_autoreleasepool:
 nextToken();
 if (FormatTok->Tok.is(tok::l_brace)) {
-  if (Style.BraceWrapping.AfterObjCDeclaration)
+  if (Style.BraceWrapping.AfterControlStatement)
 addUnwrappedLine();
   parseBlock(/*MustBeDeclaration=*/false);
 }
@@ -1127,7 +1127,7 @@
// Skip synchronization object
parseParens();
 if (FormatTok->Tok.is(tok::l_brace)) {
-  if (Style.BraceWrapping.AfterObjCDeclaration)
+  if (Style.BraceWrapping.AfterControlStatement)
 addUnwrappedLine();
   parseBlock(/*MustBeDeclaration=*/false);
 }
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -651,7 +651,9 @@
 ///   }
 /// \endcode
 bool AfterNamespace;
-/// \brief Wrap ObjC definitions (``@autoreleasepool``, interfaces, ..).
+/// \brief Wrap ObjC definitions (interfaces, implementations...).
+/// \note @autoreleasepool and @synchronized blocks are wrapped
+/// according to `AfterControlStatement` flag.
 bool AfterObjCDeclaration;
 /// \brief Wrap struct definitions.
 /// \code


Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1115,7 +1115,7 @@
   case tok::objc_autoreleasepool:
 nextToken();
 if (FormatTok->Tok.is(tok::l_brace)) {
-  if (Style.BraceWrapping.AfterObjCDeclaration)
+  if (Style.BraceWrapping.AfterControlStatement)
 addUnwrappedLine();
   parseBlock(/*MustBeDeclaration=*/false);
 }
@@ -1127,7 +1127,7 @@
// Skip synchronization object
parseParens();
 if (FormatTok->Tok.is(tok::l_brace)) {
-  if (Style.BraceWrapping.AfterObjCDeclaration)
+  if (Style.BraceWrapping.AfterControlStatement)
 addUnwrappedLine();
   parseBlock(/*MustBeDeclaration=*/false);
 }
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -651,7 +651,9 @@
 ///   }
 /// \endcode
 bool AfterNamespace;
-/// \brief Wrap ObjC definitions (``@autoreleasepool``, interfaces, ..).
+/// \brief Wrap ObjC definitions (interfaces, implementations...).
+/// \note @autoreleasepool and @synchronized blocks are wrapped
+/// according to `AfterControlStatement` flag.
 bool AfterObjCDeclaration;
 /// \brief Wrap struct definitions.
 /// \code
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43232: clang-format: use AfterControlStatement to format ObjC control blocks

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

This is done as discussed in https://reviews.llvm.org/D43114.
But I can instead add a new `AfterObjCSpecialBlock` brace wrapping flag.


Repository:
  rC Clang

https://reviews.llvm.org/D43232



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

[PATCH] D43114: clang-format: fix formatting of ObjC @synchronized blocks

[Francois Ferrand via Phabricator via cfe-commits]

Typz marked 4 inline comments as done.
Typz added inline comments.



Comment at: lib/Format/UnwrappedLineParser.cpp:1130
+if (FormatTok->Tok.is(tok::l_brace)) {
+  if (Style.BraceWrapping.AfterObjCDeclaration)
+addUnwrappedLine();

benhamilton wrote:
> Typz wrote:
> > benhamilton wrote:
> > > Typz wrote:
> > > > Wondering if formatting with this style is appropriate: the 
> > > > clang-format doc points in that direction, but it seems to me both 
> > > > `@synchronized` and `@autoreleasepool` are more akin to "control 
> > > > statements".
> > > > 
> > > > For consistency (esp. in ObjC++ code), as a user, I would tend to have 
> > > > these blocks indented like control statements while 
> > > > interfaces/implementations blocks would be indented like 
> > > > classes/structs.
> > > > 
> > > > So maybe it would be better to introduce a new BraceWrapping style 
> > > > 'AfterObjCSpecialBlock` to control these cases, matching the 
> > > > possibilities that are given for control statements vs classes. What do 
> > > > you think?
> > > Hmm, I definitely agree with that logic. I don't see them acting as 
> > > declarations in any way, they are definitely like control statements.
> > > 
> > Ok, i can change this. Two questions though:
> > * Should I do this in this patch, or a separate patch? (won't be a big 
> > change in any case, but it can still be seen as 2 separate issues/changes)
> > * Should I introduce a new BraceWrapping flag (`AfterObjCSpecialBlock`), or 
> > simply apply `AfterControlStatement` to these blocks?
> Personally, I feel `AfterControlStatement` applies to these. I'm not an 
> expert on this, though, so I will defer to others on the review.
I created another diff to change the field that is used to control brace 
wrapping after these blocks: https://reviews.llvm.org/D43232

This patch here should now only relate to the parsing of `@synchronized` blocks.


Repository:
  rC Clang

https://reviews.llvm.org/D43114



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

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

> We'll just format those cases in a somewhat weird way and users can either 
> accept that or change their code to not need it.

I think we have a really diverging opinion on this. From my experience, people 
will mostly accept the output of the formatter without question : therefore I 
think we should strive to have the formatter format things as "correctly" as 
possible. And looking at the existing options and various complex formatting 
algorithms implemented I think this reasoning has been applied to some extent 
:-)
So I personnally would be willing to add some code/options even for such kind 
of corner cases; but then I am not the one maintaining the clang-format project.

> I don't particularly care which of these options we go with, but I would be 
> really hesitant to introduce an style flag for it. This is so rare, that 
> almost no one needs this flag (or should need this flag). Thus, the cost of 
> the flag (discoverability of flags for users, increased maintenance cost, 
> etc.) strongly outweigh the benefit.

Any change will still require a new flag somewhere, since the currently 
implemented behavior is :
1- the documented way to format in Google style
2- the expected format in LLVM style, according to the clang-format unit test

It should thus probably not be changed.

> IMO, for the same reason, this specific issue will not become part of the 
> style guide of projects.

I definitely agree on this, but it is actually part of some styles (e.g. 
Google's)

> If you want to change something, I'd vote for making clang fall back to this 
> behavior:
> 
>   case A:
> {
>   stuff();
> }
> moreStuff();
> break;
>   }
>
> 
> i.e. not let it put the "{" on the same line as the "case ..." if there is a 
> trailing statement (other than "break;" maybe).

I am fine with that formatting (though I did not implement it since it requires 
tweaking the code in more places, instead of essentially modifying a single 
function like I did).


Repository:
  rC Clang

https://reviews.llvm.org/D43183



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

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

It is explicitly documented in google style guide: 
https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements :

> case blocks in switch statements can have curly braces or not, depending on 
> your preference. If you do include curly braces they should be placed as 
> shown below.
> 
> If not conditional on an enumerated value, switch statements should always 
> have a default case (in the case of an enumerated value, the compiler will 
> warn you if any values are not handled). If the default case should never 
> execute, simply assert:
> 
>   switch (var) {
> case 0: {  // 2 space indent
>   ...  // 4 space indent
>   break;
> }
> case 1: {
>   ...
>   break;
> }
> default: {
>   assert(false);
> }
>   }

So IMHO we cannot just change the current (or default) behaviour.


Repository:
  rC Clang

https://reviews.llvm.org/D43183



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

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Hum, not sure I fully got your proposal. So you actually mean that clang-format 
you format like this (with 4-space indent for clarity):

  switch (x) {
  case 0:
  break;
  case 1: {
  foo();
  break;
  }
  case 2: {
  foo();
  } break;
  case 3:
  {
  foo();
  }
  bar();
  break;
  }

Is this right?

If so it does not really help me: I don't care so much how it is formatted, but 
I think the current way is way too error prone (and I cannot change the style 
to indent the case blocks themself). So i'll have to keep a patch in my fork :-(
Or maybe the behavior should be dependant on `IndentCaseLabels` (though this 
would change LLVM style formatting) ?


Repository:
  rC Clang

https://reviews.llvm.org/D43183



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

[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Typz added reviewers: krasimir, djasper, klimek.

This patch changes the behavior of PenaltyBreakBeforeFirstCallParameter
so that is does not apply when the brace comes after an assignment.

This way, variable initialization is wrapped more like an initializer
than like a function call, which seems more consistent with user
expectations.

With PenaltyBreakBeforeFirstCallParameter=200, this gives the following
code: (with Cpp11BracedListStyle=false)

Before :

  const std::unordered_map Something::MyHashTable =
  { { "a", 0 },
{ "b", 1 },
{ "c", 2 } };

After :

  const std::unordered_set Something::MyUnorderedSet = {
{ "a", 0 },
{ "b", 1 },
{ "c", 2 }
  };


Repository:
  rC Clang

https://reviews.llvm.org/D43290

Files:
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6650,6 +6650,15 @@
"};");
   verifyFormat("#define A {a, a},");
 
+  // Avoid breaking between equal sign and opening brace
+  FormatStyle avoidBreakingFirstArgument = getLLVMStyle();
+  avoidBreakingFirstArgument.PenaltyBreakBeforeFirstCallParameter = 200;
+  verifyFormat("const std::unordered_map MyHashTable = {\n"
+   "{\"a\", 0},\n"
+   "{\"b\", 1},\n"
+   "{\"c\", 2}};",
+   avoidBreakingFirstArgument);
+
   // Binpacking only if there is no trailing comma
   verifyFormat("const Aa a = {aa, bb,\n"
"  cc, dd};",
@@ -6806,6 +6815,15 @@
   verifyFormat("vector foo = { ::SomeGlobalFunction() };", ExtraSpaces);
   verifyFormat("const struct A a = { .a = 1, .b = 2 };", ExtraSpaces);
   verifyFormat("const struct A a = { [0] = 1, [1] = 2 };", ExtraSpaces);
+
+  // Avoid breaking between equal sign and opening brace
+  ExtraSpaces.PenaltyBreakBeforeFirstCallParameter = 200;
+  verifyFormat("const std::unordered_map MyHashTable = {\n"
+   "  { \"a\", 0 },\n"
+   "  { \"b\", 1 },\n"
+   "  { \"c\", 2 }\n"
+   "};",
+   ExtraSpaces);
 }
 
 TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
@@ -6867,6 +6885,14 @@
" 3, cc};",
getLLVMStyleWithColumns(60));
 
+  // Do not break between equal and opening brace.
+  FormatStyle avoidBreakingFirstArgument = getLLVMStyleWithColumns(43);
+  avoidBreakingFirstArgument.PenaltyBreakBeforeFirstCallParameter = 200;
+  avoidBreakingFirstArgument.ColumnLimit = 43;
+  verifyFormat("vector aa = {\n"
+   "1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1};",
+   avoidBreakingFirstArgument);
+
   // Trailing commas.
   verifyFormat("vector x = {\n"
"1, 1, 1, 1, 1, 1, 1, 1,\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2180,6 +2180,8 @@
   if (Left.opensScope()) {
 if (Style.AlignAfterOpenBracket == FormatStyle::BAS_DontAlign)
   return 0;
+if (Left.Previous && Left.Previous->is(tok::equal))
+  return 19;
 return Left.ParameterCount > 1 ? Style.PenaltyBreakBeforeFirstCallParameter
: 19;
   }


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6650,6 +6650,15 @@
"};");
   verifyFormat("#define A {a, a},");
 
+  // Avoid breaking between equal sign and opening brace
+  FormatStyle avoidBreakingFirstArgument = getLLVMStyle();
+  avoidBreakingFirstArgument.PenaltyBreakBeforeFirstCallParameter = 200;
+  verifyFormat("const std::unordered_map MyHashTable = {\n"
+   "{\"a\", 0},\n"
+   "{\"b\", 1},\n"
+   "{\"c\", 2}};",
+   avoidBreakingFirstArgument);
+
   // Binpacking only if there is no trailing comma
   verifyFormat("const Aa a = {aa, bb,\n"
"  cc, dd};",
@@ -6806,6 +6815,15 @@
   verifyFormat("vector foo = { ::SomeGlobalFunction() };", ExtraSpaces);
   verifyFormat("const struct A a = { .a = 1, .b = 2 };", ExtraSpaces);
   verifyFormat("const struct A a = { [0] = 1, [1] = 2 };", ExtraSpaces);
+
+  // Avoid breaking between equal sign and opening brace
+  ExtraSpaces.Pe

[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Tweaking the penalty handling would still be needed in any case, since the 
problem happens also when Cpp11BracedListStyle is true (though the effect is 
more subtle)

Generally, I think it is better to just rely on penalties, since it gives a way 
to compare and weight each solution. Then each style can decide what should 
break first: e.g. a style may also have a lower `PenaltyBreakAssignment`, thus 
wrapping after the equal sign would be expected...


Repository:
  rC Clang

https://reviews.llvm.org/D43290



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

[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 134411.
Typz marked an inline comment as done.
Typz added a comment.

Address review comments.


Repository:
  rC Clang

https://reviews.llvm.org/D43290

Files:
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6650,6 +6650,15 @@
"};");
   verifyFormat("#define A {a, a},");
 
+  // Avoid breaking between equal sign and opening brace
+  FormatStyle AvoidBreakingFirstArgument = getLLVMStyle();
+  AvoidBreakingFirstArgument.PenaltyBreakBeforeFirstCallParameter = 200;
+  verifyFormat("const std::unordered_map MyHashTable = {\n"
+   "{\"a\", 0},\n"
+   "{\"b\", 1},\n"
+   "{\"c\", 2}};",
+   AvoidBreakingFirstArgument);
+
   // Binpacking only if there is no trailing comma
   verifyFormat("const Aa a = {aa, bb,\n"
"  cc, dd};",
@@ -6806,6 +6815,15 @@
   verifyFormat("vector foo = { ::SomeGlobalFunction() };", ExtraSpaces);
   verifyFormat("const struct A a = { .a = 1, .b = 2 };", ExtraSpaces);
   verifyFormat("const struct A a = { [0] = 1, [1] = 2 };", ExtraSpaces);
+
+  // Avoid breaking between equal sign and opening brace
+  ExtraSpaces.PenaltyBreakBeforeFirstCallParameter = 200;
+  verifyFormat("const std::unordered_map MyHashTable = {\n"
+   "  { \"a\", 0 },\n"
+   "  { \"b\", 1 },\n"
+   "  { \"c\", 2 }\n"
+   "};",
+   ExtraSpaces);
 }
 
 TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
@@ -6867,6 +6885,14 @@
" 3, cc};",
getLLVMStyleWithColumns(60));
 
+  // Do not break between equal and opening brace.
+  FormatStyle AvoidBreakingFirstArgument = getLLVMStyleWithColumns(43);
+  AvoidBreakingFirstArgument.PenaltyBreakBeforeFirstCallParameter = 200;
+  AvoidBreakingFirstArgument.ColumnLimit = 43;
+  verifyFormat("vector aa = {\n"
+   "1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1};",
+   AvoidBreakingFirstArgument);
+
   // Trailing commas.
   verifyFormat("vector x = {\n"
"1, 1, 1, 1, 1, 1, 1, 1,\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2180,6 +2180,8 @@
   if (Left.opensScope()) {
 if (Style.AlignAfterOpenBracket == FormatStyle::BAS_DontAlign)
   return 0;
+if (Left.Previous && Left.Previous->is(tok::equal))
+  return 19;
 return Left.ParameterCount > 1 ? Style.PenaltyBreakBeforeFirstCallParameter
: 19;
   }


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6650,6 +6650,15 @@
"};");
   verifyFormat("#define A {a, a},");
 
+  // Avoid breaking between equal sign and opening brace
+  FormatStyle AvoidBreakingFirstArgument = getLLVMStyle();
+  AvoidBreakingFirstArgument.PenaltyBreakBeforeFirstCallParameter = 200;
+  verifyFormat("const std::unordered_map MyHashTable = {\n"
+   "{\"a\", 0},\n"
+   "{\"b\", 1},\n"
+   "{\"c\", 2}};",
+   AvoidBreakingFirstArgument);
+
   // Binpacking only if there is no trailing comma
   verifyFormat("const Aa a = {aa, bb,\n"
"  cc, dd};",
@@ -6806,6 +6815,15 @@
   verifyFormat("vector foo = { ::SomeGlobalFunction() };", ExtraSpaces);
   verifyFormat("const struct A a = { .a = 1, .b = 2 };", ExtraSpaces);
   verifyFormat("const struct A a = { [0] = 1, [1] = 2 };", ExtraSpaces);
+
+  // Avoid breaking between equal sign and opening brace
+  ExtraSpaces.PenaltyBreakBeforeFirstCallParameter = 200;
+  verifyFormat("const std::unordered_map MyHashTable = {\n"
+   "  { \"a\", 0 },\n"
+   "  { \"b\", 1 },\n"
+   "  { \"c\", 2 }\n"
+   "};",
+   ExtraSpaces);
 }
 
 TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
@@ -6867,6 +6885,14 @@
" 3, cc};",
getLLVMStyleWithColumns(60));
 
+  // Do not break between equal and opening brace.
+  FormatStyle AvoidBreakingFirstArgument = getLLVMStyleWithColumns(43);
+  AvoidBreakingFirstArgument.PenaltyBreakBeforeFirstCallParameter = 200

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D43183#1008632, @djasper wrote:

> Yes, that's what I mean. What do you mean, the style is too error prone?


When `IndentCaseLabels` is false, the code gets formatted in a way that "hides" 
the structure of the code, by indenting the end of the case in the exact same 
way as the end of a switch, which is error prone IMHO.
Indentation should "highlight" the structure of the code, to make it easier to 
understand, and thus avoid this kind of confusion.


Repository:
  rC Clang

https://reviews.llvm.org/D43183



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

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

A user can create an error by reasoning like this, after the code has been 
formatted this way (a long time ago) : "oh, I need to make an extra function 
call, but in all cases ah, here is the end of the switch, let's put my 
function call here".

I am not saying it happens often, I am just saying it breaks indentation : i.e. 
that two nested blocks should not close at the same depth. Breaking such things 
makes code harder to read/understand, and when you don't properly get the code 
you can more easily make a mistake. Obviously it should be caught in testing, 
but it is not always.

That said, I am not trying to say in any way that my approach is better. I 
think both `CaseBlockIndent = Block` or your variant (with the extra line break 
before opening brace; but applying it all the time) will indeed be consistent 
with the code and not break indentation; keeping the current behavior when 
`IndentCaseLabels = true` is also not a problem IMHO.

> And to me (but this is obviously objective), your suggestions hide the 
> structure of the code even more as they lead to a state where the closing 
> brace is not aligned with the start of the line/statement that contains the 
> opening braces. That looks like a bug to me more than anything else and I 
> don't think there is another situation where clang-format would do that.

The exact same thing happens for functions blocks, class blocks and control 
statements, depending on BraceWrapping modes. Actually, IMO wrapping the 
opening brace should probably be done according to 
`BraceWrapping.AfterControlStatement` flag.

  // BraceWrapping.AfterControlStatement = false
  switch (x) {
  case 0: {
  foo();
  break;
}
  }
  // BraceWrapping.AfterControlStatement = true
  switch (x)
  {
  case 0:
{
  foo();
  break;
}
  }

But I agree the `CaseBlockIndent = ClosingBrace` mode is definitely not 
consistent with the code. I think it is clearer this way, but that is 
definitely my own subjective opinion: in this case, I accept to lose the extra 
indent for the sake of compactness and to somehow mimic the "regular" case 
blocks (e.g. which do not include an actual block), but that is indeed really 
my personnal way to read code.


Repository:
  rC Clang

https://reviews.llvm.org/D43183



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

[PATCH] D43232: clang-format: use AfterControlStatement to format ObjC control blocks

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D43232#1008004, @benhamilton wrote:

> Thanks! Can you add a test for this, please?


There are actually tests, though they are performed by activating Allman brace 
wrapping mode.
I am trying to change them, but this highlights another issue: when only 
`AfterControlStatement` is set, the block gets inlined, like this:

  @autoreleasepool
  { f(); }

This used to happen before as well, and would require fixing 
UnwrappedLineFormatter.cpp to understand that these blocks are linked to the 
previous ObjC keyword.


Repository:
  rC Clang

https://reviews.llvm.org/D43232



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

[PATCH] D43232: clang-format: use AfterControlStatement to format ObjC control blocks

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

I'll try to fix this.

Can you please review D43114  in the mean 
time? This patch depends on it, so it must be merged first.


Repository:
  rC Clang

https://reviews.llvm.org/D43232



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

[PATCH] D43114: clang-format: fix formatting of ObjC @synchronized blocks

[Francois Ferrand via Phabricator via cfe-commits]

Typz marked an inline comment as done.
Typz added inline comments.



Comment at: unittests/Format/FormatTestObjC.cpp:193-198
+  verifyFormat("@synchronized(self) {\n"
+   "  f();\n"
+   "}\n"
+   "@synchronized(self) {\n"
+   "  f();\n"
+   "}\n");

benhamilton wrote:
> Why is the block repeated twice?
not sure why, I just copied the @autoreleasepool test code.
this may be to detect some more 'internal' error, which would lead to any 
visible effect on the first statement itself...


Repository:
  rC Clang

https://reviews.llvm.org/D43114



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

[PATCH] D43232: clang-format: use AfterControlStatement to format ObjC control blocks

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 134641.
Typz added a comment.

Fix bug which was lingering: prevent inlining of ObjC special control blocks.


Repository:
  rC Clang

https://reviews.llvm.org/D43232

Files:
  include/clang/Format/Format.h
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -178,7 +178,8 @@
"@autoreleasepool {\n"
"  f();\n"
"}\n");
-  Style.BreakBeforeBraces = FormatStyle::BS_Allman;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = true;
   verifyFormat("@autoreleasepool\n"
"{\n"
"  f();\n"
@@ -196,7 +197,8 @@
"@synchronized(self) {\n"
"  f();\n"
"}\n");
-  Style.BreakBeforeBraces = FormatStyle::BS_Allman;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = true;
   verifyFormat("@synchronized(self)\n"
"{\n"
"  f();\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1115,7 +1115,7 @@
   case tok::objc_autoreleasepool:
 nextToken();
 if (FormatTok->Tok.is(tok::l_brace)) {
-  if (Style.BraceWrapping.AfterObjCDeclaration)
+  if (Style.BraceWrapping.AfterControlStatement)
 addUnwrappedLine();
   parseBlock(/*MustBeDeclaration=*/false);
 }
@@ -1127,7 +1127,7 @@
// Skip synchronization object
parseParens();
 if (FormatTok->Tok.is(tok::l_brace)) {
-  if (Style.BraceWrapping.AfterObjCDeclaration)
+  if (Style.BraceWrapping.AfterControlStatement)
 addUnwrappedLine();
   parseBlock(/*MustBeDeclaration=*/false);
 }
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -314,6 +314,14 @@
   }
   return MergedLines;
 }
+// Don't merge block with left brace wrapped after ObjC special blocks
+if (TheLine->First->is(tok::l_brace) && I != AnnotatedLines.begin() &&
+I[-1]->First->is(tok::at) && I[-1]->First->Next) {
+  tok::ObjCKeywordKind kwId = I[-1]->First->Next->Tok.getObjCKeywordID();
+  if (kwId == clang::tok::objc_autoreleasepool ||
+  kwId == clang::tok::objc_synchronized)
+return 0;
+}
 // Try to merge a block with left brace wrapped that wasn't yet covered
 if (TheLine->Last->is(tok::l_brace)) {
   return !Style.BraceWrapping.AfterFunction ||
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -651,7 +651,9 @@
 ///   }
 /// \endcode
 bool AfterNamespace;
-/// \brief Wrap ObjC definitions (``@autoreleasepool``, interfaces, ..).
+/// \brief Wrap ObjC definitions (interfaces, implementations...).
+/// \note @autoreleasepool and @synchronized blocks are wrapped
+/// according to `AfterControlStatement` flag.
 bool AfterObjCDeclaration;
 /// \brief Wrap struct definitions.
 /// \code


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -178,7 +178,8 @@
"@autoreleasepool {\n"
"  f();\n"
"}\n");
-  Style.BreakBeforeBraces = FormatStyle::BS_Allman;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = true;
   verifyFormat("@autoreleasepool\n"
"{\n"
"  f();\n"
@@ -196,7 +197,8 @@
"@synchronized(self) {\n"
"  f();\n"
"}\n");
-  Style.BreakBeforeBraces = FormatStyle::BS_Allman;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = true;
   verifyFormat("@synchronized(self)\n"
"{\n"
"  f();\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1115,7 +1115,7 @@
   case tok::objc_autoreleasepool:
 nextToken();
 if (FormatTok->Tok.is(tok::l_brace)) {
-  if (Style.BraceWrapping.AfterObjCDeclaration)
+  if (Style.BraceWrapping.AfterControlStatement)
 addUnwrappedLine();
   

[PATCH] D33447: clang-format: add option to merge empty function body

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 100053.
Typz added a comment.

Fix SFS_Empty & SFS_Inline merging of empty function when 
BraceWrapping.AfterFunction, and add missing test


https://reviews.llvm.org/D33447

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6006,6 +6006,36 @@
getLLVMStyleWithColumns(23));
 }
 
+TEST_F(FormatTest, PullEmptyFunctionDefinitionsIntoSingleLine) {
+  FormatStyle MergeEmptyOnly = getLLVMStyle();
+  MergeEmptyOnly.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeEmptyOnly);
+  verifyFormat("class C {\n"
+   "  int f() {\n"
+			   "return 42;\n"
+			   "  }\n"
+   "};",
+   MergeEmptyOnly);
+  verifyFormat("int f() {}",
+   MergeEmptyOnly);
+  verifyFormat("int f() {\n"
+   "  return 42;\n"
+   "}",
+   MergeEmptyOnly);
+
+  // Also verify behavior when BraceWrapping.AfterFunction = true
+  MergeEmptyOnly.BreakBeforeBraces = FormatStyle::BS_Custom;
+  MergeEmptyOnly.BraceWrapping.AfterFunction = true;
+  verifyFormat("int f() {}", MergeEmptyOnly);
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeEmptyOnly);
+}
+
 TEST_F(FormatTest, PullInlineFunctionDefinitionsIntoSingleLine) {
   FormatStyle MergeInlineOnly = getLLVMStyle();
   MergeInlineOnly.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
@@ -6017,6 +6047,104 @@
"  return 42;\n"
"}",
MergeInlineOnly);
+
+  // SFS_Inline implies SFS_Empty
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f() {}",
+   MergeInlineOnly);
+
+  // Also verify behavior when BraceWrapping.AfterFunction = true
+  MergeInlineOnly.BreakBeforeBraces = FormatStyle::BS_Custom;
+  MergeInlineOnly.BraceWrapping.AfterFunction = true;
+  verifyFormat("class C {\n"
+   "  int f() { return 42; }\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f()\n"
+			   "{\n"
+   "  return 42;\n"
+   "}",
+   MergeInlineOnly);
+
+  // SFS_Inline implies SFS_Empty
+  verifyFormat("int f() {}", MergeInlineOnly);
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeInlineOnly);
+}
+
+TEST_F(FormatTest, AllowEmptyFunctionBodyOnASingleLine) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+  Style.AllowEmptyFunctionBodyOnASingleLine = true;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterFunction = true;
+  Style.ColumnLimit = 40;
+
+  verifyFormat("int f()\n"
+   "{}",
+   Style);
+  verifyFormat("int f()\n"
+			   "{\n"
+   "  return 42;\n"
+   "}",
+   Style);
+  verifyFormat("int f()\n"
+			   "{\n"
+   "  // some comment\n"
+   "}",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
+  verifyFormat("int f() {}", Style);
+  verifyFormat("int aa(int bb)\n"
+			   "{}",
+   Style);
+  verifyFormat("int f()\n"
+			   "{\n"
+			   "  return 0;\n"
+			   "}",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
+  verifyFormat("class Foo {\n"
+			   "  int f() {}\n"
+			   "};\n",
+   Style);
+verifyFormat("class Foo {\n"
+			   "  int f() { return 0; }\n"
+			   "};\n",
+   Style);
+  verifyFormat("class Foo {\n"
+			   "  int aa(int bb)\n"
+			   "  {}\n"
+			   "};\n",
+   Style);
+  verifyFormat("class Foo {\n"
+			   "  int aa(int bb)\n"
+			   "  {\n"
+			   "return 0;\n"
+			   "  }\n"
+			   "};\n",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
+  verifyFormat("int f() {}",
+   Style);
+  verifyFormat("int f() { return 0; }",
+   Style);
+  verifyFormat("int aa(int bb)\n"
+			   "{}",
+   Style);
+  verifyFormat("int aa(int bb)\n"
+			   "{\n"
+			   "  return 0;\n"
+			   "}",
+   Style);
 }
 
 TEST_F(FormatTest, UnderstandContextOfRecordTypeKeywords) {
@@ -8670,6 +8798,7 @@
   CHECK_PARSE_BOOL(AlignConsecutiveAssignments);
   CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOn

[PATCH] D33447: clang-format: add option to merge empty function body

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D33447#763045, @djasper wrote:

> Does anything speak against making this behavior happen with 
> AllowShortFunctionsOnASingleLine = SFS_Empty and 
> MergeEmptyOnly.BraceWrapping.AfterFunction = true? I mean without the extra 
> style option?


That is fine with me (with `AllowShortFunctionsOnASingleLine >= SFS_Empty` 
condition), but I see two things:

- it does not match the "OnASingleLine" part of that option's name, since in 
that case we get the function definition on one line, and the body on the next 
line
- some (existing) coding style may use `BraceWrapping.AfterFunction` and 
`AllowShortFunctionsOnASingleLine` (for exemple Mozilla), but may not want to 
change the behavior when the function cannot be merged on a single line


https://reviews.llvm.org/D33447



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

[PATCH] D33447: clang-format: add option to merge empty function body

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 100060.
Typz added a comment.

fix indent


https://reviews.llvm.org/D33447

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6006,6 +6006,36 @@
getLLVMStyleWithColumns(23));
 }
 
+TEST_F(FormatTest, PullEmptyFunctionDefinitionsIntoSingleLine) {
+  FormatStyle MergeEmptyOnly = getLLVMStyle();
+  MergeEmptyOnly.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeEmptyOnly);
+  verifyFormat("class C {\n"
+   "  int f() {\n"
+			   "return 42;\n"
+			   "  }\n"
+   "};",
+   MergeEmptyOnly);
+  verifyFormat("int f() {}",
+   MergeEmptyOnly);
+  verifyFormat("int f() {\n"
+   "  return 42;\n"
+   "}",
+   MergeEmptyOnly);
+
+  // Also verify behavior when BraceWrapping.AfterFunction = true
+  MergeEmptyOnly.BreakBeforeBraces = FormatStyle::BS_Custom;
+  MergeEmptyOnly.BraceWrapping.AfterFunction = true;
+  verifyFormat("int f() {}", MergeEmptyOnly);
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeEmptyOnly);
+}
+
 TEST_F(FormatTest, PullInlineFunctionDefinitionsIntoSingleLine) {
   FormatStyle MergeInlineOnly = getLLVMStyle();
   MergeInlineOnly.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
@@ -6017,6 +6047,104 @@
"  return 42;\n"
"}",
MergeInlineOnly);
+
+  // SFS_Inline implies SFS_Empty
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f() {}",
+   MergeInlineOnly);
+
+  // Also verify behavior when BraceWrapping.AfterFunction = true
+  MergeInlineOnly.BreakBeforeBraces = FormatStyle::BS_Custom;
+  MergeInlineOnly.BraceWrapping.AfterFunction = true;
+  verifyFormat("class C {\n"
+   "  int f() { return 42; }\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f()\n"
+   "{\n"
+   "  return 42;\n"
+   "}",
+   MergeInlineOnly);
+
+  // SFS_Inline implies SFS_Empty
+  verifyFormat("int f() {}", MergeInlineOnly);
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeInlineOnly);
+}
+
+TEST_F(FormatTest, AllowEmptyFunctionBodyOnASingleLine) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+  Style.AllowEmptyFunctionBodyOnASingleLine = true;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterFunction = true;
+  Style.ColumnLimit = 40;
+
+  verifyFormat("int f()\n"
+   "{}",
+   Style);
+  verifyFormat("int f()\n"
+			   "{\n"
+   "  return 42;\n"
+   "}",
+   Style);
+  verifyFormat("int f()\n"
+			   "{\n"
+   "  // some comment\n"
+   "}",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
+  verifyFormat("int f() {}", Style);
+  verifyFormat("int aa(int bb)\n"
+			   "{}",
+   Style);
+  verifyFormat("int f()\n"
+			   "{\n"
+			   "  return 0;\n"
+			   "}",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
+  verifyFormat("class Foo {\n"
+			   "  int f() {}\n"
+			   "};\n",
+   Style);
+verifyFormat("class Foo {\n"
+			   "  int f() { return 0; }\n"
+			   "};\n",
+   Style);
+  verifyFormat("class Foo {\n"
+			   "  int aa(int bb)\n"
+			   "  {}\n"
+			   "};\n",
+   Style);
+  verifyFormat("class Foo {\n"
+			   "  int aa(int bb)\n"
+			   "  {\n"
+			   "return 0;\n"
+			   "  }\n"
+			   "};\n",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
+  verifyFormat("int f() {}",
+   Style);
+  verifyFormat("int f() { return 0; }",
+   Style);
+  verifyFormat("int aa(int bb)\n"
+			   "{}",
+   Style);
+  verifyFormat("int aa(int bb)\n"
+			   "{\n"
+			   "  return 0;\n"
+			   "}",
+   Style);
 }
 
 TEST_F(FormatTest, UnderstandContextOfRecordTypeKeywords) {
@@ -8670,6 +8798,7 @@
   CHECK_PARSE_BOOL(AlignConsecutiveAssignments);
   CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
+  CHECK_PARSE_BOOL(AllowEmptyFunctionBodyOnASingleLine);
   CHECK_PARSE_BOOL(

[PATCH] D33447: clang-format: add option to merge empty function body

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Facebook's HHVM seems to use this for empty constructors: 
https://github.com/facebook/hhvm/blob/master/hphp/doc/coding-conventions.md
This is also done systematically in Qt and QtCreator code.

Personally, I don't care if this is an option or not; but I fear it would break 
existing styles.


https://reviews.llvm.org/D33447



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

[PATCH] D32479: clang-format: Introduce BreakConstructorInitializers option

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL303739: clang-format: Introduce BreakConstructorInitializers 
option (authored by Typz).

Changed prior to commit:
  https://reviews.llvm.org/D32479?vs=99887&id=100068#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D32479

Files:
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/ContinuationIndenter.cpp
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp

Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -710,16 +710,35 @@
   /// \endcode
   bool BreakBeforeTernaryOperators;
 
-  /// \brief Always break constructor initializers before commas and align
-  /// the commas with the colon.
-  /// \code
-  ///true:  false:
-  ///SomeClass::Constructor()   vs. SomeClass::Constructor() : a(a),
-  ///: a(a)   b(b),
-  ///, b(b)   c(c) {}
-  ///, c(c) {}
-  /// \endcode
-  bool BreakConstructorInitializersBeforeComma;
+  /// \brief Different ways to break initializers.
+  enum BreakConstructorInitializersStyle
+  {
+/// Break constructor initializers before the colon and after the commas.
+/// \code
+/// Constructor()
+/// : initializer1(),
+///   initializer2()
+/// \endcode
+BCIS_BeforeColon,
+/// Break constructor initializers before the colon and commas, and align
+/// the commas with the colon.
+/// \code
+/// Constructor()
+/// : initializer1()
+/// , initializer2()
+/// \endcode
+BCIS_BeforeComma,
+/// Break constructor initializers after the colon and commas.
+/// \code
+/// Constructor() :
+/// initializer1(),
+/// initializer2()
+/// \endcode
+BCIS_AfterColon
+  };
+
+  /// \brief The constructor initializers style to use..
+  BreakConstructorInitializersStyle BreakConstructorInitializers;
 
   /// \brief Break after each annotation on a field in Java files.
   /// \code{.java}
@@ -1390,8 +1409,7 @@
BreakBeforeBinaryOperators == R.BreakBeforeBinaryOperators &&
BreakBeforeBraces == R.BreakBeforeBraces &&
BreakBeforeTernaryOperators == R.BreakBeforeTernaryOperators &&
-   BreakConstructorInitializersBeforeComma ==
-   R.BreakConstructorInitializersBeforeComma &&
+   BreakConstructorInitializers == R.BreakConstructorInitializers &&
BreakAfterJavaFieldAnnotations == R.BreakAfterJavaFieldAnnotations &&
BreakStringLiterals == R.BreakStringLiterals &&
ColumnLimit == R.ColumnLimit && CommentPragmas == R.CommentPragmas &&
Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp
===
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp
@@ -54,13 +54,14 @@
 const FormatStyle &Style) {
   const FormatToken &Previous = *Current.Previous;
   if (Current.is(TT_CtorInitializerComma) &&
-  Style.BreakConstructorInitializersBeforeComma)
+  Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeComma)
 return true;
   return Previous.is(tok::comma) && !Current.isTrailingComment() &&
  ((Previous.isNot(TT_CtorInitializerComma) ||
-  !Style.BreakConstructorInitializersBeforeComma) &&
+   Style.BreakConstructorInitializers !=
+   FormatStyle::BCIS_BeforeComma) &&
   (Previous.isNot(TT_InheritanceComma) ||
-  !Style.BreakBeforeInheritanceComma));
+   !Style.BreakBeforeInheritanceComma));
 }
 
 ContinuationIndenter::ContinuationIndenter(const FormatStyle &Style,
@@ -178,13 +179,20 @@
   getLengthToMatchingParen(Previous) + State.Column - 1 >
   getColumnLimit(State))
 return true;
-  if (Current.is(TT_CtorInitializerColon) &&
-  (State.Column + State.Line->Last->TotalLength - Current.TotalLength + 2 >
+
+  const FormatToken &BreakConstructorInitializersToken =
+  Style.BreakConstructorInitializers == FormatStyle::BCIS_AfterColon
+  ? Previous
+  : Current;
+  if (BreakConstructorInitializersToken.is(TT_CtorInitializerColon) &&
+  (State.Column + State.Line->Last->TotalLength - Previous.TotalLength >
getColumnLimit(State) ||
State.Stack.back().BreakBeforeParameter) &&
-  ((Style.AllowShortFunctionsOnASingleLine != FormatStyle::SFS_All) ||
-   Style.BreakConstructorInitializersBeforeComma || Style.ColumnLimit != 0))
+  (Style.AllowShortFunctionsOnASingleLine != FormatStyle::SFS_All ||
+   Style.BreakConstructorInitializers != FormatStyl

[PATCH] D33440: clang-format: properly handle Q_UNUSED and QT_REQUIRE_VERSION

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Without this patch, macros with no trailing semicolon _in the body of a 
function_ are not handled properly, so I get:

  int foo(int a, int b) {
Q_UNUSED(a) return b;
  }
  
  class Foo {
void bar(int a, int b) { Q_UNUSED(a) Q_UNUSED(b) }
  }


https://reviews.llvm.org/D33440



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

[PATCH] D33447: clang-format: add option to merge empty function body

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

I am fine with "removing" the options: I only fear it would incorrectly affect 
existing users/styles, which would possibly end up in the patch being 
reverted...
I could not find the info in Mozilla coding style, but looking at the code it 
seems it should be done indeed, but with an extra space in between, like this:

  void foo()
  { }

So if you tell me it's fine I will go ahead and replace the option with the 
`BraceWrapping.AfterFunction && AllowShortFunctionsOnASingleLine >= SFS_Empty` 
condition.


https://reviews.llvm.org/D33447



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

[PATCH] D33440: clang-format: properly handle Q_UNUSED and QT_REQUIRE_VERSION

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Digging back, it seem the issue was that I had this code:

  void foo(..) { Q_UNUSED(a) Q_UNUSED(b) }

which got wrapped because the line was too long:

  void foo(..) {
Q_UNUSED(a) Q_UNUSED(b)
  }

I definitely understand your concern about introduceing special handling for 
some specific/hard-coded macro-DSLs, but would it be acceptable to add a 
configuration option to specify a list of such macros? e.g. a StatementMacros 
option, similar to the ForEachMacros option?

That would allow:

- Systematically reformatting the first example
- If  such a code is reformatted, ensure the 2 macros are not on the same line


https://reviews.llvm.org/D33440



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

[PATCH] D33447: clang-format: add option to merge empty function body

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Right, I was not clear enough: this is indeed done only if everything does not 
fit on one line.

There is an exemple in facebook's coding style:

  MyClass::MyClass(const Class* cls, const Func* func, const Class* ctx)
: m_cls(cls)
, m_func(func)
, m_ctx(ctx)
, m_isMyConditionMet(false)
  {}

And this happens (with the extra space) in the same kind of situation in 
Mozilla's code:

  
nsSameProcessAsyncMessageBase::nsSameProcessAsyncMessageBase(JS::RootingContext*
 aRootingCx,
   
JS::Handle aCpows)
: mRootingCx(aRootingCx)
, mCpows(aRootingCx, aCpows)
  { }


https://reviews.llvm.org/D33447



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

[PATCH] D33447: clang-format: add option to merge empty function body

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Merging empty blocks is not allowed in webkit style: 
https://webkit.org/code-style-guidelines/#punctuation-member-init

So it seems changing the behavior would actually fix mozilla style and break 
webkit style (as integrated in clang-format).


https://reviews.llvm.org/D33447



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

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 100095.
Typz added a comment.

add new value to AlignOperands to support this mode.
fix some corner cases.


https://reviews.llvm.org/D32478

Files:
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/ContinuationIndenter.h
  lib/Format/Format.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/FormatTestJS.cpp

Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -102,7 +102,7 @@
   verifyFormat("var x = a() in\n"
".aa.aa;");
   FormatStyle Style = getGoogleJSStyleWithColumns(80);
-  Style.AlignOperands = true;
+  Style.AlignOperands = FormatStyle::OAS_Align;
   verifyFormat("var x = a() in\n"
".aa.aa;",
Style);
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2503,6 +2503,9 @@
"  > c) {\n"
"}",
Style);
+  verifyFormat("return a\n"
+   "   && bbb;",
+   Style);
   verifyFormat("return (a)\n"
"   // comment\n"
"   + b;",
@@ -2531,11 +2534,104 @@
 
   Style.ColumnLimit = 60;
   verifyFormat("zz\n"
-   "= b\n"
+   "= \n"
"  >> (aa);",
Style);
 }
 
+TEST_F(FormatTest, ExpressionIndentationStrictAlign) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  Style.AlignOperands = FormatStyle::OAS_StrictAlign;
+
+  verifyFormat(
+  "bool value = a\n"
+  "   + a\n"
+  "   + a\n"
+  "  == a\n"
+  " * b\n"
+  " + b\n"
+  "  && a\n"
+  " * a\n"
+  " > c;",
+  Style);
+  verifyFormat("if (a\n"
+   "* \n"
+   "+ aa\n"
+   "== bbb) {\n}",
+   Style);
+  verifyFormat("if (a\n"
+   "+ \n"
+   "  * aa\n"
+   "== bbb) {\n}",
+   Style);
+  verifyFormat("if (a\n"
+   "== \n"
+   "   * aa\n"
+   "   + bbb) {\n}",
+   Style);
+  verifyFormat("if () {\n"
+   "} else if (a\n"
+   "   && b // break\n"
+   "  > c) {\n"
+   "}",
+   Style);
+  verifyFormat("return a\n"
+   "&& bbb;",
+   Style);
+  verifyFormat("return (a)\n"
+   "   // comment\n"
+   " + b;",
+   Style);
+  verifyFormat(
+  "int aa = aa\n"
+  "   * bbb\n"
+  "   + cc;",
+  Style);
+
+  verifyFormat("a\n"
+   "=  + ;",
+   Style);
+
+  verifyFormat("return boost::fusion::at_c<0>().second\n"
+   "== boost::fusion::at_c<1>().second;",
+   Style);
+
+  Style.ColumnLimit = 60;
+  verifyFormat("z\n"
+   "= \n"
+   "   >> (aa);",
+   Style);
+
+  // Forced by co

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

It indeed does not happens inside any parenthesis (it would actually make 
things completely unreadable if there are imbricated parenthesis), and may get 
indented less than the ContinuationIndentWidth.


https://reviews.llvm.org/D32478



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

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz added inline comments.



Comment at: lib/Format/ContinuationIndenter.cpp:949
+ Previous->is(tok::kw_return)))
+  NewParenState.UnindentOperator = true;
 

djasper wrote:
> I am not yet convinced you need a new flag in ParenState here. I guess you 
> need it because the operators can have varying length and so you cannot just 
> change LastSpace here?
exactly. This is set when passing through the equal/return sign, but indent 
must be adjusted for each line individually based on the actual size of that 
line's leading operator.


https://reviews.llvm.org/D32478



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

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Herald added a subscriber: klimek.

This patch tries to improve the optimizer a bit, to avoid splitting
tokens (e.g. comments/strings) if only there are only few characters
beyond the ColumnLimit.

Previously, comments/strings would be split whenever they went beyond
the ColumnLimit, without consideration for the PenaltyBreakComment/
String and PenaltyExcessCharacter. With this patch, the
OptimizingLineFormatter will also consider not splitting each token,
so that these 'small' offenders get a chance:

  // With ColumnLimit=20
  int a; // the
 // comment
  
  // With ColumnLimit=20 and PenaltyExcessCharacter = 10
  int a; // the comment

This patch does not fully optimize the reflowing, as it will only try
reflowing the whole comment or not reflowing it at all (instead of
trying each split, to allow some lines of overflow ColumnLimit even
when reflowing).


https://reviews.llvm.org/D33589

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/ContinuationIndenter.h
  lib/Format/FormatToken.cpp
  lib/Format/FormatToken.h
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8542,6 +8542,36 @@
   EXPECT_EQ("#pragma option -C -A", format("#pragmaoption   -C   -A"));
 }
 
+TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 20;
+
+  verifyFormat("int a; // the\n"
+			   "   // comment", Style);
+  verifyFormat("int a; /* first line\n"
+   "* second\n"
+			   "* line third\n"
+   "* line\n"
+   "*/", Style);
+
+  Style.PenaltyExcessCharacter = 30;
+  verifyFormat("int a; // the comment", Style);
+  EXPECT_EQ("int a; // the\n"
+"   // comment aa",
+format("int a; // the comment aa", Style));
+  verifyFormat("int a; /* first line\n"
+   "* second line\n"
+			   "* third line\n"
+   "*/", Style);
+  EXPECT_EQ("int a; /* first line\n"
+"* second\n"
+			"* line third\n"
+"* line\n"
+"*/",
+format("int a; /* first line second line third line */",
+   Style));
+}
+
 #define EXPECT_ALL_STYLES_EQUAL(Styles)\
   for (size_t i = 1; i < Styles.size(); ++i)   \
   EXPECT_EQ(Styles[0], Styles[i]) << "Style #" << i << " of " << Styles.size() \
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -590,7 +590,8 @@
   (Indenter->canBreak(State) && State.NextToken->NewlinesBefore > 0);
   unsigned Penalty = 0;
   formatChildren(State, Newline, /*DryRun=*/false, Penalty);
-  Indenter->addTokenToState(State, Newline, /*DryRun=*/false);
+  Indenter->addTokenToState(State, Newline, /*BreakToken=*/true,
+/*DryRun=*/false);
 }
 return 0;
   }
@@ -611,7 +612,8 @@
 LineState State = Indenter->getInitialState(FirstIndent, &Line, DryRun);
 while (State.NextToken) {
   formatChildren(State, /*Newline=*/false, DryRun, Penalty);
-  Indenter->addTokenToState(State, /*Newline=*/false, DryRun);
+  Indenter->addTokenToState(State, /*Newline=*/false, /*BreakToken=*/true,
+DryRun);
 }
 return Penalty;
   }
@@ -658,10 +660,11 @@
   /// \brief An edge in the solution space from \c Previous->State to \c State,
   /// inserting a newline dependent on the \c NewLine.
   struct StateNode {
-StateNode(const LineState &State, bool NewLine, StateNode *Previous)
-: State(State), NewLine(NewLine), Previous(Previous) {}
+StateNode(const LineState &State, bool NewLine, bool BreakToken, StateNode *Previous)
+: State(State), NewLine(NewLine), BreakToken(BreakToken), Previous(Previous) {}
 LineState State;
 bool NewLine;
+bool BreakToken;
 StateNode *Previous;
   };
 
@@ -691,7 +694,7 @@
 
 // Insert start element into queue.
 StateNode *Node =
-new (Allocator.Allocate()) StateNode(InitialState, false, nullptr);
+new (Allocator.Allocate()) StateNode(InitialState, false, true, nullptr);
 Queue.push(QueueItem(OrderedPenalty(0, Count), Node));
 ++Count;
 
@@ -718,9 +721,17 @@
 
   FormatDecision LastFormat = Node->State.NextToken->Decision;
   if (LastFormat == FD_Unformatted || LastFormat == FD_Continue)
-addNextStateToQueue(Penalty, Node, /*NewLine=*/false, &Count, &Queue);
+addNextStateToQueue(Penalty, Node, /*NewLine=*/false,
+/*BreakToken=*/true, &Count, &Queue);
   if 

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 100379.
Typz added a comment.

fix indentation issues


https://reviews.llvm.org/D33589

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/ContinuationIndenter.h
  lib/Format/FormatToken.cpp
  lib/Format/FormatToken.h
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8542,6 +8542,35 @@
   EXPECT_EQ("#pragma option -C -A", format("#pragmaoption   -C   -A"));
 }
 
+TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 20;
+
+  verifyFormat("int a; // the\n"
+   "   // comment", Style);
+  verifyFormat("int a; /* first line\n"
+   "* second\n"
+   "* line third\n"
+   "* line\n"
+   "*/", Style);
+
+  Style.PenaltyExcessCharacter = 30;
+  verifyFormat("int a; // the comment", Style);
+  EXPECT_EQ("int a; // the\n"
+"   // comment aa",
+format("int a; // the comment aa", Style));
+  verifyFormat("int a; /* first line\n"
+   "* second line\n"
+   "* third line\n"
+   "*/", Style);
+  EXPECT_EQ("int a; /* first line\n"
+"* second\n"
+"* line third\n"
+"* line\n"
+"*/",
+format("int a; /* first line second line third line */", Style));
+}
+
 #define EXPECT_ALL_STYLES_EQUAL(Styles)\
   for (size_t i = 1; i < Styles.size(); ++i)   \
   EXPECT_EQ(Styles[0], Styles[i]) << "Style #" << i << " of " << Styles.size() \
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -590,7 +590,8 @@
   (Indenter->canBreak(State) && State.NextToken->NewlinesBefore > 0);
   unsigned Penalty = 0;
   formatChildren(State, Newline, /*DryRun=*/false, Penalty);
-  Indenter->addTokenToState(State, Newline, /*DryRun=*/false);
+  Indenter->addTokenToState(State, Newline, /*BreakToken=*/true,
+/*DryRun=*/false);
 }
 return 0;
   }
@@ -611,7 +612,8 @@
 LineState State = Indenter->getInitialState(FirstIndent, &Line, DryRun);
 while (State.NextToken) {
   formatChildren(State, /*Newline=*/false, DryRun, Penalty);
-  Indenter->addTokenToState(State, /*Newline=*/false, DryRun);
+  Indenter->addTokenToState(State, /*Newline=*/false, /*BreakToken=*/true,
+DryRun);
 }
 return Penalty;
   }
@@ -658,10 +660,11 @@
   /// \brief An edge in the solution space from \c Previous->State to \c State,
   /// inserting a newline dependent on the \c NewLine.
   struct StateNode {
-StateNode(const LineState &State, bool NewLine, StateNode *Previous)
-: State(State), NewLine(NewLine), Previous(Previous) {}
+StateNode(const LineState &State, bool NewLine, bool BreakToken, StateNode *Previous)
+: State(State), NewLine(NewLine), BreakToken(BreakToken), Previous(Previous) {}
 LineState State;
 bool NewLine;
+bool BreakToken;
 StateNode *Previous;
   };
 
@@ -691,7 +694,7 @@
 
 // Insert start element into queue.
 StateNode *Node =
-new (Allocator.Allocate()) StateNode(InitialState, false, nullptr);
+new (Allocator.Allocate()) StateNode(InitialState, false, true, nullptr);
 Queue.push(QueueItem(OrderedPenalty(0, Count), Node));
 ++Count;
 
@@ -718,9 +721,17 @@
 
   FormatDecision LastFormat = Node->State.NextToken->Decision;
   if (LastFormat == FD_Unformatted || LastFormat == FD_Continue)
-addNextStateToQueue(Penalty, Node, /*NewLine=*/false, &Count, &Queue);
+addNextStateToQueue(Penalty, Node, /*NewLine=*/false,
+/*BreakToken=*/true, &Count, &Queue);
   if (LastFormat == FD_Unformatted || LastFormat == FD_Break)
-addNextStateToQueue(Penalty, Node, /*NewLine=*/true, &Count, &Queue);
+addNextStateToQueue(Penalty, Node, /*NewLine=*/true,
+/*BreakToken=*/true, &Count, &Queue);
+  if (LastFormat == FD_Unformatted || LastFormat == FD_Continue)
+addNextStateToQueue(Penalty, Node, /*NewLine=*/false,
+/*BreakToken=*/false, &Count, &Queue);
+  if (LastFormat == FD_Unformatted || LastFormat == FD_Break)
+addNextStateToQueue(Penalty, Node, /*NewLine=*/true,
+/*BreakToken=*/false, &Count, &Queue);
 }
 
 if (Queue.empty()) {
@@ -745,18 +756,20 @@
   /// Assume the current state is \p PreviousNode and has been rea

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Still some issues with the patch, I would need some feedback first:

- Is this approach desirable, as a relatively easy fix?
- Or should this be fixed with a complete refactoring of the way the 
strings/comments are split, making multiple tokens out of them to let them be 
reflown 'directly' by the optimizing line formatter? (keep the optimizer, but 
changes the whole way comments are handled, and need to move all the 
information about the comment block and its relfowing into LineState)
- Or should the breakProtrudingToken() actually perform a "local" optimization 
of the splits? (keeps the same overall structure, but requires writing another 
optimizer)




Comment at: unittests/Format/FormatTest.cpp:8571
+"*/",
+format("int a; /* first line second line third line */", Style));
+}

This is not working as expected, format return:

  int a; /* first line
* second *
line third
* line */

Any clue how things could go so wrong?


https://reviews.llvm.org/D33589



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

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 100384.
Typz added a comment.

fix style


https://reviews.llvm.org/D32478

Files:
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/ContinuationIndenter.h
  lib/Format/Format.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/FormatTestJS.cpp

Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -102,7 +102,7 @@
   verifyFormat("var x = a() in\n"
".aa.aa;");
   FormatStyle Style = getGoogleJSStyleWithColumns(80);
-  Style.AlignOperands = true;
+  Style.AlignOperands = FormatStyle::OAS_Align;
   verifyFormat("var x = a() in\n"
".aa.aa;",
Style);
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2503,6 +2503,9 @@
"  > c) {\n"
"}",
Style);
+  verifyFormat("return a\n"
+   "   && bbb;",
+   Style);
   verifyFormat("return (a)\n"
"   // comment\n"
"   + b;",
@@ -2531,11 +2534,103 @@
 
   Style.ColumnLimit = 60;
   verifyFormat("zz\n"
-   "= b\n"
+   "= \n"
"  >> (aa);",
Style);
 }
 
+TEST_F(FormatTest, ExpressionIndentationStrictAlign) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  Style.AlignOperands = FormatStyle::OAS_StrictAlign;
+
+  verifyFormat("bool value = a\n"
+   "   + a\n"
+   "   + a\n"
+   "  == a\n"
+   " * b\n"
+   " + b\n"
+   "  && a\n"
+   " * a\n"
+   " > c;",
+   Style);
+  verifyFormat("if (a\n"
+   "* \n"
+   "+ aa\n"
+   "== bbb) {\n}",
+   Style);
+  verifyFormat("if (a\n"
+   "+ \n"
+   "  * aa\n"
+   "== bbb) {\n}",
+   Style);
+  verifyFormat("if (a\n"
+   "== \n"
+   "   * aa\n"
+   "   + bbb) {\n}",
+   Style);
+  verifyFormat("if () {\n"
+   "} else if (a\n"
+   "   && b // break\n"
+   "  > c) {\n"
+   "}",
+   Style);
+  verifyFormat("return a\n"
+   "&& bbb;",
+   Style);
+  verifyFormat("return (a)\n"
+   "   // comment\n"
+   " + b;",
+   Style);
+  verifyFormat(
+  "int aa = aa\n"
+  "   * bbb\n"
+  "   + cc;",
+  Style);
+
+  verifyFormat("a\n"
+   "=  + ;",
+   Style);
+
+  verifyFormat("return boost::fusion::at_c<0>().second\n"
+   "== boost::fusion::at_c<1>().second;",
+   Style);
+
+  Style.ColumnLimit = 60;
+  verifyFormat("z\n"
+   "= \n"
+   "   >> (aa);",
+   Style);
+
+  // Force

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D32478#765537, @djasper wrote:

> In all honesty, I think this style isn't thought out well enough. It really 
> is a special case for only "=" and "return" and even there, it has many cases 
> where it simply doesn't make sense. And then you have cases like this:
>
>   bool = aa //
>   ==  //
>   && c;
>   
>
> Where the syntactic structure is lost entirely.




  bool a = aa   //
==  //
&& c;

> On top of that it has runtime downsides for all clang-format users because 
> ParenState gets larger and more costly compare. As such, I am against moving 
> forward with this. Can you remind me again, which coding style suggests this 
> format?

This is just a single extra bit (and there are still less than 16 such bits), 
so it does change the size of ParenState. As for the compare cost, I think it 
is within reach of the compiler's optimization, but it may indeed have a slight 
impact.


https://reviews.llvm.org/D32478



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

[PATCH] D33447: clang-format: add option to merge empty function body

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Webkit inherits AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All from 
LLVM style, so merging the options would introduce these explicitely-forbidden 
empty blocks.
But the empty blocks should actually be used in code following Mozilla or Qt 
style.


https://reviews.llvm.org/D33447



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

[PATCH] D33447: clang-format: add option to merge empty function body

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 100393.
Typz added a comment.

update mozilla style to use empty function blocks


https://reviews.llvm.org/D33447

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6006,6 +6006,36 @@
getLLVMStyleWithColumns(23));
 }
 
+TEST_F(FormatTest, PullEmptyFunctionDefinitionsIntoSingleLine) {
+  FormatStyle MergeEmptyOnly = getLLVMStyle();
+  MergeEmptyOnly.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeEmptyOnly);
+  verifyFormat("class C {\n"
+   "  int f() {\n"
+			   "return 42;\n"
+			   "  }\n"
+   "};",
+   MergeEmptyOnly);
+  verifyFormat("int f() {}",
+   MergeEmptyOnly);
+  verifyFormat("int f() {\n"
+   "  return 42;\n"
+   "}",
+   MergeEmptyOnly);
+
+  // Also verify behavior when BraceWrapping.AfterFunction = true
+  MergeEmptyOnly.BreakBeforeBraces = FormatStyle::BS_Custom;
+  MergeEmptyOnly.BraceWrapping.AfterFunction = true;
+  verifyFormat("int f() {}", MergeEmptyOnly);
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeEmptyOnly);
+}
+
 TEST_F(FormatTest, PullInlineFunctionDefinitionsIntoSingleLine) {
   FormatStyle MergeInlineOnly = getLLVMStyle();
   MergeInlineOnly.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
@@ -6017,6 +6047,104 @@
"  return 42;\n"
"}",
MergeInlineOnly);
+
+  // SFS_Inline implies SFS_Empty
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f() {}",
+   MergeInlineOnly);
+
+  // Also verify behavior when BraceWrapping.AfterFunction = true
+  MergeInlineOnly.BreakBeforeBraces = FormatStyle::BS_Custom;
+  MergeInlineOnly.BraceWrapping.AfterFunction = true;
+  verifyFormat("class C {\n"
+   "  int f() { return 42; }\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f()\n"
+   "{\n"
+   "  return 42;\n"
+   "}",
+   MergeInlineOnly);
+
+  // SFS_Inline implies SFS_Empty
+  verifyFormat("int f() {}", MergeInlineOnly);
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeInlineOnly);
+}
+
+TEST_F(FormatTest, AllowEmptyFunctionBodyOnASingleLine) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+  Style.AllowEmptyFunctionBodyOnASingleLine = true;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterFunction = true;
+  Style.ColumnLimit = 40;
+
+  verifyFormat("int f()\n"
+   "{}",
+   Style);
+  verifyFormat("int f()\n"
+			   "{\n"
+   "  return 42;\n"
+   "}",
+   Style);
+  verifyFormat("int f()\n"
+			   "{\n"
+   "  // some comment\n"
+   "}",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
+  verifyFormat("int f() {}", Style);
+  verifyFormat("int aa(int bb)\n"
+			   "{}",
+   Style);
+  verifyFormat("int f()\n"
+			   "{\n"
+			   "  return 0;\n"
+			   "}",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
+  verifyFormat("class Foo {\n"
+			   "  int f() {}\n"
+			   "};\n",
+   Style);
+verifyFormat("class Foo {\n"
+			   "  int f() { return 0; }\n"
+			   "};\n",
+   Style);
+  verifyFormat("class Foo {\n"
+			   "  int aa(int bb)\n"
+			   "  {}\n"
+			   "};\n",
+   Style);
+  verifyFormat("class Foo {\n"
+			   "  int aa(int bb)\n"
+			   "  {\n"
+			   "return 0;\n"
+			   "  }\n"
+			   "};\n",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
+  verifyFormat("int f() {}",
+   Style);
+  verifyFormat("int f() { return 0; }",
+   Style);
+  verifyFormat("int aa(int bb)\n"
+			   "{}",
+   Style);
+  verifyFormat("int aa(int bb)\n"
+			   "{\n"
+			   "  return 0;\n"
+			   "}",
+   Style);
 }
 
 TEST_F(FormatTest, UnderstandContextOfRecordTypeKeywords) {
@@ -8670,6 +8798,7 @@
   CHECK_PARSE_BOOL(AlignConsecutiveAssignments);
   CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
+  CHECK_PARSE_BOOL(AllowEmptyFunctionB

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

I actually don't know how, but it still manages somehow : I rebuilt this exact 
patch to ensure I gave you the correct output.
And the same behavior can be seen in the test cases, where the operator with 
highest precedence is aligned with the equal sign.


https://reviews.llvm.org/D32478



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

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Nop, it's formatted like this:

  bool a = aa   //
==  //
&& c;
  
  bool a = aa //
==    //
   + c;


https://reviews.llvm.org/D32478



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

[PATCH] D33447: clang-format: add option to merge empty function body

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D33447#765610, @djasper wrote:

> I think it's just wrong that WebKit inherits this. The style guide explicitly 
> says that this is wrong:
>
>   MyOtherClass::MyOtherClass() : MySuperClass() {}


I think this exemple applies to constructors only.
Looking at webkit code, there are many shortl functions which are indeed 
formatted on a single line (at least 22097 occurences, actually)

> So I still think we can make this work with the existing options without 
> regressing a behavior that anyone is relying on.


https://reviews.llvm.org/D33447



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

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

> Ah damn. Didn't think about the precedences. What I wanted was for the 
> highest level to have a one char operator, e.g.
> 
>   bool a = aa //
>  <<    //
>  | c;
>

Almost, but not quite:

  bool a = aa   //
<<  //
 | c;

i.e. the identifiers is indented by 4 characters, then the operator is 
'unindented.

> I would still be interested in a coding style that recommends this format.

We are using this at our compagny, but this is neither public nor open source.
I am sure I saw this style somewhere else, but I cannot remember where...
(This is also the style recommended for webkit's when returning booleans : 
https://webkit.org/code-style-guidelines/#indentation-wrap-bool-op, but 
probably because the continuation indent is 4...
and I am not sure this guide in Boost's Spirit library counts either: 
http://www.boost.org/doc/libs/1_56_0/libs/spirit/doc/html/spirit/notes/style_guide.html
 )


https://reviews.llvm.org/D32478



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

[PATCH] D33447: clang-format: add option to merge empty function body

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 100412.
Typz added a comment.

move option to BraceWrapping


https://reviews.llvm.org/D33447

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6006,6 +6006,36 @@
getLLVMStyleWithColumns(23));
 }
 
+TEST_F(FormatTest, PullEmptyFunctionDefinitionsIntoSingleLine) {
+  FormatStyle MergeEmptyOnly = getLLVMStyle();
+  MergeEmptyOnly.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeEmptyOnly);
+  verifyFormat("class C {\n"
+   "  int f() {\n"
+			   "return 42;\n"
+			   "  }\n"
+   "};",
+   MergeEmptyOnly);
+  verifyFormat("int f() {}",
+   MergeEmptyOnly);
+  verifyFormat("int f() {\n"
+   "  return 42;\n"
+   "}",
+   MergeEmptyOnly);
+
+  // Also verify behavior when BraceWrapping.AfterFunction = true
+  MergeEmptyOnly.BreakBeforeBraces = FormatStyle::BS_Custom;
+  MergeEmptyOnly.BraceWrapping.AfterFunction = true;
+  verifyFormat("int f() {}", MergeEmptyOnly);
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeEmptyOnly);
+}
+
 TEST_F(FormatTest, PullInlineFunctionDefinitionsIntoSingleLine) {
   FormatStyle MergeInlineOnly = getLLVMStyle();
   MergeInlineOnly.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
@@ -6017,6 +6047,104 @@
"  return 42;\n"
"}",
MergeInlineOnly);
+
+  // SFS_Inline implies SFS_Empty
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f() {}",
+   MergeInlineOnly);
+
+  // Also verify behavior when BraceWrapping.AfterFunction = true
+  MergeInlineOnly.BreakBeforeBraces = FormatStyle::BS_Custom;
+  MergeInlineOnly.BraceWrapping.AfterFunction = true;
+  verifyFormat("class C {\n"
+   "  int f() { return 42; }\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f()\n"
+   "{\n"
+   "  return 42;\n"
+   "}",
+   MergeInlineOnly);
+
+  // SFS_Inline implies SFS_Empty
+  verifyFormat("int f() {}", MergeInlineOnly);
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeInlineOnly);
+}
+
+TEST_F(FormatTest, AllowEmptyFunctionBody) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterFunction = true;
+  Style.BraceWrapping.AllowEmptyFunctionBody = true;
+  Style.ColumnLimit = 40;
+
+  verifyFormat("int f()\n"
+   "{}",
+   Style);
+  verifyFormat("int f()\n"
+			   "{\n"
+   "  return 42;\n"
+   "}",
+   Style);
+  verifyFormat("int f()\n"
+			   "{\n"
+   "  // some comment\n"
+   "}",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
+  verifyFormat("int f() {}", Style);
+  verifyFormat("int aa(int bb)\n"
+			   "{}",
+   Style);
+  verifyFormat("int f()\n"
+			   "{\n"
+			   "  return 0;\n"
+			   "}",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
+  verifyFormat("class Foo {\n"
+			   "  int f() {}\n"
+			   "};\n",
+   Style);
+verifyFormat("class Foo {\n"
+			   "  int f() { return 0; }\n"
+			   "};\n",
+   Style);
+  verifyFormat("class Foo {\n"
+			   "  int aa(int bb)\n"
+			   "  {}\n"
+			   "};\n",
+   Style);
+  verifyFormat("class Foo {\n"
+			   "  int aa(int bb)\n"
+			   "  {\n"
+			   "return 0;\n"
+			   "  }\n"
+			   "};\n",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
+  verifyFormat("int f() {}",
+   Style);
+  verifyFormat("int f() { return 0; }",
+   Style);
+  verifyFormat("int aa(int bb)\n"
+			   "{}",
+   Style);
+  verifyFormat("int aa(int bb)\n"
+			   "{\n"
+			   "  return 0;\n"
+			   "}",
+   Style);
 }
 
 TEST_F(FormatTest, UnderstandContextOfRecordTypeKeywords) {
@@ -8712,6 +8840,7 @@
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterObjCDeclaration);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterStruct);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterUnion);
+  CHECK_PARSE_NESTED_BOOL(BraceWrapping, AllowEmptyFunctionB

[PATCH] D33447: clang-format: add option to merge empty function body

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 100420.
Typz marked 2 inline comments as done.
Typz added a comment.

fix indent & rename option to SplitEmptyFunctionBody


https://reviews.llvm.org/D33447

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6006,6 +6006,35 @@
getLLVMStyleWithColumns(23));
 }
 
+TEST_F(FormatTest, PullEmptyFunctionDefinitionsIntoSingleLine) {
+  FormatStyle MergeEmptyOnly = getLLVMStyle();
+  MergeEmptyOnly.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeEmptyOnly);
+  verifyFormat("class C {\n"
+   "  int f() {\n"
+   "return 42;\n"
+   "  }\n"
+   "};",
+   MergeEmptyOnly);
+  verifyFormat("int f() {}", MergeEmptyOnly);
+  verifyFormat("int f() {\n"
+   "  return 42;\n"
+   "}",
+   MergeEmptyOnly);
+
+  // Also verify behavior when BraceWrapping.AfterFunction = true
+  MergeEmptyOnly.BreakBeforeBraces = FormatStyle::BS_Custom;
+  MergeEmptyOnly.BraceWrapping.AfterFunction = true;
+  verifyFormat("int f() {}", MergeEmptyOnly);
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeEmptyOnly);
+}
+
 TEST_F(FormatTest, PullInlineFunctionDefinitionsIntoSingleLine) {
   FormatStyle MergeInlineOnly = getLLVMStyle();
   MergeInlineOnly.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
@@ -6017,6 +6046,101 @@
"  return 42;\n"
"}",
MergeInlineOnly);
+
+  // SFS_Inline implies SFS_Empty
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f() {}", MergeInlineOnly);
+
+  // Also verify behavior when BraceWrapping.AfterFunction = true
+  MergeInlineOnly.BreakBeforeBraces = FormatStyle::BS_Custom;
+  MergeInlineOnly.BraceWrapping.AfterFunction = true;
+  verifyFormat("class C {\n"
+   "  int f() { return 42; }\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f()\n"
+   "{\n"
+   "  return 42;\n"
+   "}",
+   MergeInlineOnly);
+
+  // SFS_Inline implies SFS_Empty
+  verifyFormat("int f() {}", MergeInlineOnly);
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeInlineOnly);
+}
+
+TEST_F(FormatTest, SplitEmptyFunctionBody) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterFunction = true;
+  Style.BraceWrapping.SplitEmptyFunctionBody = false;
+  Style.ColumnLimit = 40;
+
+  verifyFormat("int f()\n"
+   "{}",
+   Style);
+  verifyFormat("int f()\n"
+   "{\n"
+   "  return 42;\n"
+   "}",
+   Style);
+  verifyFormat("int f()\n"
+   "{\n"
+   "  // some comment\n"
+   "}",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
+  verifyFormat("int f() {}", Style);
+  verifyFormat("int aa(int bb)\n"
+   "{}",
+   Style);
+  verifyFormat("int f()\n"
+   "{\n"
+   "  return 0;\n"
+   "}",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
+  verifyFormat("class Foo {\n"
+   "  int f() {}\n"
+   "};\n",
+   Style);
+  verifyFormat("class Foo {\n"
+   "  int f() { return 0; }\n"
+   "};\n",
+   Style);
+  verifyFormat("class Foo {\n"
+   "  int aa(int bb)\n"
+   "  {}\n"
+   "};\n",
+   Style);
+  verifyFormat("class Foo {\n"
+   "  int aa(int bb)\n"
+   "  {\n"
+   "return 0;\n"
+   "  }\n"
+   "};\n",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
+  verifyFormat("int f() {}", Style);
+  verifyFormat("int f() { return 0; }", Style);
+  verifyFormat("int aa(int bb)\n"
+   "{}",
+   Style);
+  verifyFormat("int aa(int bb)\n"
+   "{\n"
+   "  return 0;\n"
+   "}",
+   Style);
 }
 
 TEST_F(FormatTest, UnderstandContextOfRecordTypeKeywords) {
@@ -8715,6 +8839,7 @@
   CHECK_PARSE_NESTED_B

[PATCH] D32480: clang-format: Add CompactNamespaces option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


https://reviews.llvm.org/D32480



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

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

Typz marked an inline comment as done.
Typz added inline comments.



Comment at: unittests/Format/FormatTest.cpp:8571
+"*/",
+format("int a; /* first line second line third line */", Style));
+}

Typz wrote:
> This is not working as expected, format return:
> 
>   int a; /* first line
> * second *
> line third
> * line */
> 
> Any clue how things could go so wrong?
This was a bug in tests: should not use test::messUp() with multi-line comments.


https://reviews.llvm.org/D33589



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

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 100698.
Typz marked an inline comment as done.
Typz added a comment.

fix code & tests


https://reviews.llvm.org/D33589

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/ContinuationIndenter.h
  lib/Format/FormatToken.cpp
  lib/Format/FormatToken.h
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8542,6 +8542,45 @@
   EXPECT_EQ("#pragma option -C -A", format("#pragmaoption   -C   -A"));
 }
 
+TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 20;
+
+  verifyFormat("int a; // the\n"
+   "   // comment", Style);
+  EXPECT_EQ("int a; /* first line\n"
+"* second\n"
+"* line third\n"
+"* line\n"
+"*/",
+			format("int a; /* first line\n"
+   "* second\n"
+   "* line third\n"
+   "* line\n"
+   "*/", Style));
+
+  Style.PenaltyExcessCharacter = 90;
+  verifyFormat("int a; // the comment", Style);
+  EXPECT_EQ("int a; // the\n"
+"   // comment aa",
+format("int a; // the comment aa", Style));
+  EXPECT_EQ("int a; /* first line\n"
+"* second line\n"
+"* third line\n"
+"*/",
+			format("int a; /* first line\n"
+	   "* second line\n"
+			   "* third line\n"
+	   "*/", Style));
+  EXPECT_EQ("int a; /* first line\n"
+"* second\n"
+"* line third\n"
+"* line\n"
+"*/",
+format("int a; /* first line second line third line"
+   "\n*/", Style));
+}
+
 #define EXPECT_ALL_STYLES_EQUAL(Styles)\
   for (size_t i = 1; i < Styles.size(); ++i)   \
   EXPECT_EQ(Styles[0], Styles[i]) << "Style #" << i << " of " << Styles.size() \
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -590,7 +590,8 @@
   (Indenter->canBreak(State) && State.NextToken->NewlinesBefore > 0);
   unsigned Penalty = 0;
   formatChildren(State, Newline, /*DryRun=*/false, Penalty);
-  Indenter->addTokenToState(State, Newline, /*DryRun=*/false);
+  Indenter->addTokenToState(State, Newline, /*BreakToken=*/true,
+/*DryRun=*/false);
 }
 return 0;
   }
@@ -611,7 +612,8 @@
 LineState State = Indenter->getInitialState(FirstIndent, &Line, DryRun);
 while (State.NextToken) {
   formatChildren(State, /*Newline=*/false, DryRun, Penalty);
-  Indenter->addTokenToState(State, /*Newline=*/false, DryRun);
+  Indenter->addTokenToState(State, /*Newline=*/false, /*BreakToken=*/true,
+DryRun);
 }
 return Penalty;
   }
@@ -658,10 +660,11 @@
   /// \brief An edge in the solution space from \c Previous->State to \c State,
   /// inserting a newline dependent on the \c NewLine.
   struct StateNode {
-StateNode(const LineState &State, bool NewLine, StateNode *Previous)
-: State(State), NewLine(NewLine), Previous(Previous) {}
+StateNode(const LineState &State, bool NewLine, bool BreakToken, StateNode *Previous)
+: State(State), NewLine(NewLine), BreakToken(BreakToken), Previous(Previous) {}
 LineState State;
 bool NewLine;
+bool BreakToken;
 StateNode *Previous;
   };
 
@@ -691,7 +694,7 @@
 
 // Insert start element into queue.
 StateNode *Node =
-new (Allocator.Allocate()) StateNode(InitialState, false, nullptr);
+new (Allocator.Allocate()) StateNode(InitialState, false, true, nullptr);
 Queue.push(QueueItem(OrderedPenalty(0, Count), Node));
 ++Count;
 
@@ -718,9 +721,17 @@
 
   FormatDecision LastFormat = Node->State.NextToken->Decision;
   if (LastFormat == FD_Unformatted || LastFormat == FD_Continue)
-addNextStateToQueue(Penalty, Node, /*NewLine=*/false, &Count, &Queue);
+addNextStateToQueue(Penalty, Node, /*NewLine=*/false,
+/*BreakToken=*/true, &Count, &Queue);
   if (LastFormat == FD_Unformatted || LastFormat == FD_Break)
-addNextStateToQueue(Penalty, Node, /*NewLine=*/true, &Count, &Queue);
+addNextStateToQueue(Penalty, Node, /*NewLine=*/true,
+/*BreakToken=*/true, &Count, &Queue);
+  if (LastFormat == FD_Unformatted || LastFormat == FD_Continue)
+addNextStateToQueue(Penalty, Node, /*NewLine=*/false,
+   

[PATCH] D32480: clang-format: Add CompactNamespaces option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

That will be slightly more complicated to check, esp. we will need to keep 
track of the matching-closing-brace (currently only the matching-opening-brace) 
is stored.
But I can try to update the patch in that direction, if that is the consensus.


https://reviews.llvm.org/D32480



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

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D33589#769893, @krasimir wrote:

> I think that what you're trying to solve is not practically that important, 
> is unlikely to improve the handling of comments, and will add a lot of 
> complexity.


Not sure the 'approach' I have in this patch is nice, but it does solve my 
problem: it is quite simple, and avoids reflowing (and actually, sometimes 
breaking, since comments are not so structured...) the comments.
e.g. if I have a line followed by a relatively long comment, with my patch it 
will really consider the penaltys properly, and not split a line which is 
slightly longer [with ColumnLimit=30] :

  int a; //< a very long comment
   

vs

  int a; //< a very long
 //< comment

> From a usability perspective, I think that people are happy enough when their 
> comments don't exceed the line limit. I personally wouldn't want the opposite 
> to happen. I've even seen style guides that have 80 columns limit for 
> comments and 100 for code.

That is a matter of taste and coding style: I prefer overflowing by a few 
characters instead of introducing an extra line... I see no reason to allow 
PenaltyExcessCharacter

> Comments are treated in a somewhat ad-hoc style in clang-format, which is 
> because they are inherently free text and don't have a specific format. 
> People just expect comments to be handled quite differently than source code. 
> There are at least three separate parts in clang-format that make up the 
> formatting of comments: the normal parsing and optimization pipeline, the 
> BreakableLineCommentSection / BreakableBlockComment classes that are 
> responsible for breaking and reflowing inside comment sections, and the 
> consecutive trailing comment alignment in the WhitespaceManager. This patch 
> takes into account the first aspect but not the consequences for the other 
> aspects: for example it allows for the first line comment token in a 
> multiline line comment section to get out of the column limit, but will 
> reflow the rest of the lines. A WhitespaceManager-related issue is that 
> because a trailing line comment for some declaration might not get split, it 
> might not be aligned with the surrounding trailing line comments, which I 
> think is a less desirable effect.

Not sure I understand the case you are speaking about, can you please provide 
an exemple?

(on a separate topic, I could indeed not find a penalty for breaking alignment; 
I think the optimizer should account for that as well... any hint on where to 
look for adding this?)

> Optimizing the layout in comment sections by using the optimizing formatter 
> sounds good, but because comments mostly contain free text that can be 
> structured in unexpected ways, I think that the ad-hoc approach works better. 
> Think of not destroying ASCII art and supporting bulleted and numbered lists 
> for example. We don't really want to leak lots of comment-related formatting 
> tweaks into the general pipeline itself.

Good, one less choice :-)

> I see you point that PenaltyBreakComment and PenaltyExcessCharacter are not 
> taken into account while comment placement, but they are taken into account 
> at a higher level by treating the whole comment section as a unit. For 
> example, if you have a long declaration that has multiple potential split 
> points followed by a long trailing comment section, the penalty induced by 
> the number of lines that are needed and the number of unbroken characters for 
> the formatting of the comment section is taken into account while optimizing 
> the layout of the whole code fragment.

If you reduce PenaltyExcessCharacter you see that comments can never overflow: 
the optimizer will consider splitting the comments or splitting the remaining 
of the code, but will (currently) not allow the comment to overflow just a bit.

> The formatted currently supports somewhat limited version of allowing comment 
> lines exceeding the column limit, like long hyperlinks for example. I think 
> that if there are some other examples which are both widespread and super 
> annoying, we may consider them separately.

This patch does not try to address these special cases, and should not change 
the behavior in this case.


https://reviews.llvm.org/D33589



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

[PATCH] D32480: clang-format: Add CompactNamespaces option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

So how do I proceed?

1. Keep the CompactNamespace option, and make "compacted" namespaces always add 
at most one level of indentation
2. Or assume that this can only ever usefully work with the behavior of NI_None 
and add an additional enum value NI_Compact.

And should we limit 'compacting' to namespace which start and end at 
consecutive lines [e.g. exactly the same conditions as C++17 nested 
namespaces], or is the current implementation enough [merge all adjacent 
beginning and all adjacent endings] ?


https://reviews.llvm.org/D32480



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

[PATCH] D33447: clang-format: add option to merge empty function body

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping


https://reviews.llvm.org/D33447



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

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


https://reviews.llvm.org/D33589



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

[PATCH] D33447: clang-format: add option to merge empty function body

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


https://reviews.llvm.org/D33447



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

[PATCH] D32480: clang-format: Add CompactNamespaces option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


https://reviews.llvm.org/D32480



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

[PATCH] D33447: clang-format: add option to merge empty function body

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL305272: clang-format: add option to merge empty function 
body (authored by Typz).

Changed prior to commit:
  https://reviews.llvm.org/D33447?vs=100420&id=102294#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33447

Files:
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp

Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -688,6 +688,18 @@
 bool BeforeElse;
 /// \brief Indent the wrapped braces themselves.
 bool IndentBraces;
+/// \brief If ``false``, empty function body can be put on a single line.
+/// This option is used only if the opening brace of the function has
+/// already been wrapped, i.e. the `AfterFunction` brace wrapping mode is
+/// set, and the function could/should not be put on a single line (as per
+/// `AllowShortFunctionsOnASingleLine` and constructor formatting options).
+/// \code
+///   int f()   vs.   inf f()
+///   {}  {
+///   }
+/// \endcode
+///
+bool SplitEmptyFunctionBody;
   };
 
   /// \brief Control of individual brace wrapping cases.
Index: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
@@ -186,6 +186,12 @@
 ? 0
 : Limit - TheLine->Last->TotalLength;
 
+if (TheLine->Last->is(TT_FunctionLBrace) &&
+TheLine->First == TheLine->Last &&
+!Style.BraceWrapping.SplitEmptyFunctionBody &&
+I[1]->First->is(tok::r_brace))
+  return tryMergeSimpleBlock(I, E, Limit);
+
 // FIXME: TheLine->Level != 0 might or might not be the right check to do.
 // If necessary, change to something smarter.
 bool MergeShortFunctions =
@@ -215,7 +221,10 @@
   Limit -= 2;
 
   unsigned MergedLines = 0;
-  if (MergeShortFunctions) {
+  if (MergeShortFunctions ||
+  (Style.AllowShortFunctionsOnASingleLine >= FormatStyle::SFS_Empty &&
+   I[1]->First == I[1]->Last && I + 2 != E &&
+   I[2]->First->is(tok::r_brace))) {
 MergedLines = tryMergeSimpleBlock(I + 1, E, Limit);
 // If we managed to merge the block, count the function header, which is
 // on a separate line.
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -410,6 +410,7 @@
 IO.mapOptional("BeforeCatch", Wrapping.BeforeCatch);
 IO.mapOptional("BeforeElse", Wrapping.BeforeElse);
 IO.mapOptional("IndentBraces", Wrapping.IndentBraces);
+IO.mapOptional("SplitEmptyFunctionBody", Wrapping.SplitEmptyFunctionBody);
   }
 };
 
@@ -485,7 +486,7 @@
 return Style;
   FormatStyle Expanded = Style;
   Expanded.BraceWrapping = {false, false, false, false, false, false,
-false, false, false, false, false};
+false, false, false, false, false, true};
   switch (Style.BreakBeforeBraces) {
   case FormatStyle::BS_Linux:
 Expanded.BraceWrapping.AfterClass = true;
@@ -498,6 +499,7 @@
 Expanded.BraceWrapping.AfterFunction = true;
 Expanded.BraceWrapping.AfterStruct = true;
 Expanded.BraceWrapping.AfterUnion = true;
+Expanded.BraceWrapping.SplitEmptyFunctionBody = false;
 break;
   case FormatStyle::BS_Stroustrup:
 Expanded.BraceWrapping.AfterFunction = true;
@@ -517,7 +519,7 @@
 break;
   case FormatStyle::BS_GNU:
 Expanded.BraceWrapping = {true, true, true, true, true, true,
-  true, true, true, true, true};
+  true, true, true, true, true, true};
 break;
   case FormatStyle::BS_WebKit:
 Expanded.BraceWrapping.AfterFunction = true;
@@ -554,7 +556,7 @@
   LLVMStyle.BreakBeforeTernaryOperators = true;
   LLVMStyle.BreakBeforeBraces = FormatStyle::BS_Attach;
   LLVMStyle.BraceWrapping = {false, false, false, false, false, false,
- false, false, false, false, false};
+ false, false, false, false, false, true};
   LLVMStyle.BreakAfterJavaFieldAnnotations = false;
   LLVMStyle.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon;
   LLVMStyle.BreakBeforeInheritanceComma = false;
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -6239,6 +6239,35 @@
getLLVMStyleWithColumns(23))

[PATCH] D32480: clang-format: Add CompactNamespaces option

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 102346.
Typz added a comment.

- make "compacted" namespaces always add at most one level of indentation
- compact only namespaces which both start and end on consecutive lines


https://reviews.llvm.org/D32480

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1309,6 +1309,164 @@
Style));
 }
 
+TEST_F(FormatTest, FormatsCompactNamespaces) {
+  FormatStyle Style = getLLVMStyle();
+  Style.CompactNamespaces = true;
+
+  verifyFormat("namespace A { namespace B {\n"
+			   "}} // namespace A::B",
+			   Style);
+
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out {\n"
+   "namespace in {\n"
+   "} // namespace in\n"
+   "} // namespace out",
+   Style));
+
+  // Only namespaces which have both consecutive opening and end get compacted
+  EXPECT_EQ("namespace out {\n"
+"namespace in1 {\n"
+"} // namespace in1\n"
+"namespace in2 {\n"
+"} // namespace in2\n"
+"} // namespace out",
+format("namespace out {\n"
+   "namespace in1 {\n"
+   "} // namespace in1\n"
+   "namespace in2 {\n"
+   "} // namespace in2\n"
+   "} // namespace out",
+   Style));
+
+  EXPECT_EQ("namespace out {\n"
+"int i;\n"
+"namespace in {\n"
+"int j;\n"
+"} // namespace in\n"
+"int k;\n"
+"} // namespace out",
+format("namespace out { int i;\n"
+   "namespace in { int j; } // namespace in\n"
+   "int k; } // namespace out",
+   Style));
+
+  EXPECT_EQ("namespace A { namespace B { namespace C {\n"
+"}}} // namespace A::B::C\n",
+format("namespace A { namespace B {\n"
+   "namespace C {\n"
+   "}} // namespace B::C\n"
+   "} // namespace A\n",
+   Style));
+
+  EXPECT_EQ("namespace  {\n"
+			"namespace  {\n"
+"}} // namespace ::",
+format("namespace  {\n"
+   "namespace  {\n"
+   "} // namespace \n"
+   "} // namespace ",
+   Style));
+
+  EXPECT_EQ("namespace a { namespace b {\n"
+			"namespace c {\n"
+"}}} // namespace a::b::c",
+format("namespace a {\n"
+   "namespace b {\n"
+   "namespace c {\n"
+   "} // namespace c\n"
+   "} // namespace b\n"
+   "} // namespace a",
+   Style));
+
+  // Missing comments are added
+  EXPECT_EQ("namespace out { namespace in {\n"
+			"int i;\n"
+			"int j;\n"
+"}} // namespace out::in",
+format("namespace out {\n"
+   "namespace in {\n"
+   "int i;\n"
+   "int j;\n"
+   "}\n"
+   "}",
+   Style));
+
+  // Incorrect comments are fixed
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out { namespace in {\n"
+   "}} // namespace out",
+   Style));
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out { namespace in {\n"
+   "}} // namespace in",
+   Style));
+
+  // Extra semicolon after 'inner' closing brace prevents merging
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}; } // namespace out::in",
+format("namespace out {\n"
+   "namespace in {\n"
+   "}; // namespace in\n"
+   "} // namespace out",
+   Style));
+
+  // Extra semicolon after 'outer' closing brace is conserved
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}}; // namespace out::in",
+format("namespace out {\n"
+   "namespace in {\n"
+   "} // namespace in\n"
+   

[PATCH] D32480: clang-format: Add CompactNamespaces option

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 102525.
Typz added a comment.

Make tests more readable


https://reviews.llvm.org/D32480

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1309,6 +1309,166 @@
Style));
 }
 
+TEST_F(FormatTest, FormatsCompactNamespaces) {
+  FormatStyle Style = getLLVMStyle();
+  Style.CompactNamespaces = true;
+
+  verifyFormat("namespace A { namespace B {\n"
+			   "}} // namespace A::B",
+			   Style);
+
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out {\n"
+   "namespace in {\n"
+   "} // namespace in\n"
+   "} // namespace out",
+   Style));
+
+  // Only namespaces which have both consecutive opening and end get compacted
+  EXPECT_EQ("namespace out {\n"
+"namespace in1 {\n"
+"} // namespace in1\n"
+"namespace in2 {\n"
+"} // namespace in2\n"
+"} // namespace out",
+format("namespace out {\n"
+   "namespace in1 {\n"
+   "} // namespace in1\n"
+   "namespace in2 {\n"
+   "} // namespace in2\n"
+   "} // namespace out",
+   Style));
+
+  EXPECT_EQ("namespace out {\n"
+"int i;\n"
+"namespace in {\n"
+"int j;\n"
+"} // namespace in\n"
+"int k;\n"
+"} // namespace out",
+format("namespace out { int i;\n"
+   "namespace in { int j; } // namespace in\n"
+   "int k; } // namespace out",
+   Style));
+
+  EXPECT_EQ("namespace A { namespace B { namespace C {\n"
+"}}} // namespace A::B::C\n",
+format("namespace A { namespace B {\n"
+   "namespace C {\n"
+   "}} // namespace B::C\n"
+   "} // namespace A\n",
+   Style));
+
+  Style.ColumnLimit = 40;
+  EXPECT_EQ("namespace aa {\n"
+"namespace bb {\n"
+"}} // namespace aa::bb",
+format("namespace aa {\n"
+   "namespace bb {\n"
+   "} // namespace bb\n"
+   "} // namespace aa",
+   Style));
+
+  EXPECT_EQ("namespace aa { namespace bb {\n"
+"namespace cc {\n"
+"}}} // namespace aa::bb::cc",
+format("namespace aa {\n"
+   "namespace bb {\n"
+   "namespace cc {\n"
+   "} // namespace cc\n"
+   "} // namespace bb\n"
+   "} // namespace aa",
+   Style));
+  Style.ColumnLimit = 80;
+
+  // Missing comments are added
+  EXPECT_EQ("namespace out { namespace in {\n"
+			"int i;\n"
+			"int j;\n"
+"}} // namespace out::in",
+format("namespace out {\n"
+   "namespace in {\n"
+   "int i;\n"
+   "int j;\n"
+   "}\n"
+   "}",
+   Style));
+
+  // Incorrect comments are fixed
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out { namespace in {\n"
+   "}} // namespace out",
+   Style));
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out { namespace in {\n"
+   "}} // namespace in",
+   Style));
+
+  // Extra semicolon after 'inner' closing brace prevents merging
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}; } // namespace out::in",
+format("namespace out {\n"
+   "namespace in {\n"
+   "}; // namespace in\n"
+   "} // namespace out",
+   Style));
+
+  // Extra semicolon after 'outer' closing brace is conserved
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}}; // namespace out::in",
+format("namespace out {\n"
+   "namespace in {\n"
+   "} // namespace in\n"
+   "}; // namespace out",
+   Style));
+
+  Style.NamespaceIndentation = FormatStyle::NI_All;
+  EXPECT_EQ("namespace out { namespace in {\n"
+"  int i;\n"
+"}} // namespace out::in",
+format("namespace out {\n"
+   "namespace in {\n"
+