[PATCH] D60726: Fixed -Wconversion-null warning in GCC.

2019-04-15 Thread Reuben Thomas via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL358441: [clang-format] Fix -Wconversion-null warning in GCC 
(authored by reuk, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60726?vs=195224=195240#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60726/new/

https://reviews.llvm.org/D60726

Files:
  cfe/trunk/unittests/Format/FormatTest.cpp


Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -11393,6 +11393,7 @@
   CHECK_PARSE_BOOL(SpacesInCStyleCastParentheses);
   CHECK_PARSE_BOOL(SpaceAfterCStyleCast);
   CHECK_PARSE_BOOL(SpaceAfterTemplateKeyword);
+  CHECK_PARSE_BOOL(SpaceAfterLogicalNot);
   CHECK_PARSE_BOOL(SpaceBeforeAssignmentOperators);
   CHECK_PARSE_BOOL(SpaceBeforeCpp11BracedList);
   CHECK_PARSE_BOOL(SpaceBeforeCtorInitializerColon);
@@ -11566,12 +11567,6 @@
   CHECK_PARSE("SpaceAfterControlStatementKeyword: true", SpaceBeforeParens,
   FormatStyle::SBPO_ControlStatements);
 
-  Style.SpaceAfterLogicalNot = false;
-  CHECK_PARSE("SpaceAfterLogicalNot: true", SpaceAfterLogicalNot,
-  true);
-  CHECK_PARSE("SpaceAfterLogicalNot: false", SpaceAfterLogicalNot,
-  false);
-
   Style.ColumnLimit = 123;
   FormatStyle BaseStyle = getLLVMStyle();
   CHECK_PARSE("BasedOnStyle: LLVM", ColumnLimit, BaseStyle.ColumnLimit);


Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -11393,6 +11393,7 @@
   CHECK_PARSE_BOOL(SpacesInCStyleCastParentheses);
   CHECK_PARSE_BOOL(SpaceAfterCStyleCast);
   CHECK_PARSE_BOOL(SpaceAfterTemplateKeyword);
+  CHECK_PARSE_BOOL(SpaceAfterLogicalNot);
   CHECK_PARSE_BOOL(SpaceBeforeAssignmentOperators);
   CHECK_PARSE_BOOL(SpaceBeforeCpp11BracedList);
   CHECK_PARSE_BOOL(SpaceBeforeCtorInitializerColon);
@@ -11566,12 +11567,6 @@
   CHECK_PARSE("SpaceAfterControlStatementKeyword: true", SpaceBeforeParens,
   FormatStyle::SBPO_ControlStatements);
 
-  Style.SpaceAfterLogicalNot = false;
-  CHECK_PARSE("SpaceAfterLogicalNot: true", SpaceAfterLogicalNot,
-  true);
-  CHECK_PARSE("SpaceAfterLogicalNot: false", SpaceAfterLogicalNot,
-  false);
-
   Style.ColumnLimit = 123;
   FormatStyle BaseStyle = getLLVMStyle();
   CHECK_PARSE("BasedOnStyle: LLVM", ColumnLimit, BaseStyle.ColumnLimit);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D60726: Fixed -Wconversion-null warning in GCC.

2019-04-15 Thread Reuben Thomas via Phabricator via cfe-commits
reuk accepted this revision.
reuk added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60726/new/

https://reviews.llvm.org/D60726



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


[PATCH] D60225: [clang-format] [PR19056] Add support for indenting class members and methods one level under the modifiers

2019-04-09 Thread Reuben Thomas via Phabricator via cfe-commits
reuk added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2009
+  // classes case
+  if (Style.AccessModifierIndentation && Line->Level % 2 == 0)
+--Line->Level;

klimek wrote:
> What if the class starts at level 1? (for example, inside a function or due 
> to namespace indentation)
> 
> namespace A {
>   class B {
> public:
>   ..
>   };
> }
Probably worth adding a test or two that format nested classes, classes inside 
namespaces, classes defined inside functions etc.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2253
   parseBlock(/*MustBeDeclaration=*/true, /*AddLevel=*/true,
- /*MunchSemi=*/false);
+ /*MunchSemi=*/false,
+ /*IndentedAccessModifiers=*/ContainsAccessModifier);

Yeah, this would look way nicer using a settings struct.



Comment at: clang/lib/Format/UnwrappedLineParser.h:89
   void parseBlock(bool MustBeDeclaration, bool AddLevel = true,
-  bool MunchSemi = true);
+  bool MunchSemi = true, bool IndentedAccessModifiers = false);
   void parseChildBlock();

Adjacent `bool` parameters is a bit nasty. Maybe pass a struct of settings 
instead? 
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i4-make-interfaces-precisely-and-strongly-typed


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60225/new/

https://reviews.llvm.org/D60225



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


[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

2019-04-08 Thread Reuben Thomas via Phabricator via cfe-commits
reuk closed this revision.
reuk added a comment.

Closed by https://reviews.llvm.org/rG91f60b44958f, 
https://reviews.llvm.org/rL357908, https://reviews.llvm.org/rC357908 (sorry, I 
forgot to update the body of the commit message to close this automatically)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60320/new/

https://reviews.llvm.org/D60320



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


[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

2019-04-07 Thread Reuben Thomas via Phabricator via cfe-commits
reuk updated this revision to Diff 194057.
reuk added a comment.

Fixed formatting nit.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60320/new/

https://reviews.llvm.org/D60320

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9719,6 +9719,17 @@
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 }
 
+TEST_F(FormatTest, SpaceAfterLogicalNot) {
+  FormatStyle Spaces = getLLVMStyle();
+  Spaces.SpaceAfterLogicalNot = true;
+
+  verifyFormat("bool x = ! y", Spaces);
+  verifyFormat("if (! isFailure())", Spaces);
+  verifyFormat("if (! (a && b))", Spaces);
+  verifyFormat("\"Error!\"", Spaces);
+  verifyFormat("! ! x", Spaces);
+}
+
 TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
   FormatStyle Spaces = getLLVMStyle();
 
@@ -11511,6 +11522,12 @@
   CHECK_PARSE("SpaceAfterControlStatementKeyword: true", SpaceBeforeParens,
   FormatStyle::SBPO_ControlStatements);
 
+  Style.SpaceAfterLogicalNot = false;
+  CHECK_PARSE("SpaceAfterLogicalNot: true", SpaceAfterLogicalNot,
+  true);
+  CHECK_PARSE("SpaceAfterLogicalNot: false", SpaceAfterLogicalNot,
+  false);
+
   Style.ColumnLimit = 123;
   FormatStyle BaseStyle = getLLVMStyle();
   CHECK_PARSE("BasedOnStyle: LLVM", ColumnLimit, BaseStyle.ColumnLimit);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2832,7 +2832,8 @@
 return true;
   }
   if (Left.is(TT_UnaryOperator))
-return Right.is(TT_BinaryOperator);
+return (Style.SpaceAfterLogicalNot && Left.is(tok::exclaim)) ||
+   Right.is(TT_BinaryOperator);
 
   // If the next token is a binary operator or a selector name, we have
   // incorrectly classified the parenthesis as a cast. FIXME: Detect correctly.
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -478,6 +478,7 @@
 IO.mapOptional("SortIncludes", Style.SortIncludes);
 IO.mapOptional("SortUsingDeclarations", Style.SortUsingDeclarations);
 IO.mapOptional("SpaceAfterCStyleCast", Style.SpaceAfterCStyleCast);
+IO.mapOptional("SpaceAfterLogicalNot", Style.SpaceAfterLogicalNot);
 IO.mapOptional("SpaceAfterTemplateKeyword",
Style.SpaceAfterTemplateKeyword);
 IO.mapOptional("SpaceBeforeAssignmentOperators",
@@ -730,6 +731,7 @@
   LLVMStyle.SpacesInContainerLiterals = true;
   LLVMStyle.SpacesInCStyleCastParentheses = false;
   LLVMStyle.SpaceAfterCStyleCast = false;
+  LLVMStyle.SpaceAfterLogicalNot = false;
   LLVMStyle.SpaceAfterTemplateKeyword = true;
   LLVMStyle.SpaceBeforeCtorInitializerColon = true;
   LLVMStyle.SpaceBeforeInheritanceColon = true;
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1628,6 +1628,13 @@
   /// \endcode
   bool SpaceAfterCStyleCast;
 
+  /// If ``true``, a space is inserted after the logical not operator (``!``).
+  /// \code
+  ///true:  false:
+  ///! someExpression();vs. !someExpression();
+  /// \endcode
+  bool SpaceAfterLogicalNot;
+
   /// If \c true, a space will be inserted after the 'template' keyword.
   /// \code
   ///true:  false:
@@ -1911,6 +1918,7 @@
PointerAlignment == R.PointerAlignment &&
RawStringFormats == R.RawStringFormats &&
SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
+   SpaceAfterLogicalNot == R.SpaceAfterLogicalNot &&
SpaceAfterTemplateKeyword == R.SpaceAfterTemplateKeyword &&
SpaceBeforeAssignmentOperators == R.SpaceBeforeAssignmentOperators &&
SpaceBeforeCpp11BracedList == R.SpaceBeforeCpp11BracedList &&
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1952,6 +1952,13 @@
  true:  false:
  (int) i;   vs. (int)i;
 
+**SpaceAfterLogicalNot** (``bool``)
+  If ``true``, a space is inserted after the logical not operator (``!``).
+  .. code-block:: c++
+
+ true:  false:
+ ! someExpression();vs. !someExpression();
+
 **SpaceAfterTemplateKeyword** (``bool``)
   If ``true``, a space will be 

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

2019-04-07 Thread Reuben Thomas via Phabricator via cfe-commits
reuk added a comment.

I updated `ClangFormatStyleOptions.rst` by hand, is there a way to do that 
automatically instead (for future patches)?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60320/new/

https://reviews.llvm.org/D60320



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


[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

2019-04-07 Thread Reuben Thomas via Phabricator via cfe-commits
reuk updated this revision to Diff 194055.
reuk added a comment.

Updated with fixes. Thanks for pointing out that the options are ordered 
alphabetically, I hadn't noticed that!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60320/new/

https://reviews.llvm.org/D60320

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9719,6 +9719,17 @@
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 }
 
+TEST_F(FormatTest, SpaceAfterLogicalNot) {
+  FormatStyle Spaces = getLLVMStyle();
+  Spaces.SpaceAfterLogicalNot = true;
+
+  verifyFormat("bool x = ! y", Spaces);
+  verifyFormat("if (! isFailure())", Spaces);
+  verifyFormat("if (! (a && b))", Spaces);
+  verifyFormat("\"Error!\"", Spaces);
+  verifyFormat("! ! x", Spaces);
+}
+
 TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
   FormatStyle Spaces = getLLVMStyle();
 
@@ -11511,6 +11522,12 @@
   CHECK_PARSE("SpaceAfterControlStatementKeyword: true", SpaceBeforeParens,
   FormatStyle::SBPO_ControlStatements);
 
+  Style.SpaceAfterLogicalNot = false;
+  CHECK_PARSE("SpaceAfterLogicalNot: true", SpaceAfterLogicalNot,
+  true);
+  CHECK_PARSE("SpaceAfterLogicalNot: false", SpaceAfterLogicalNot,
+  false);
+
   Style.ColumnLimit = 123;
   FormatStyle BaseStyle = getLLVMStyle();
   CHECK_PARSE("BasedOnStyle: LLVM", ColumnLimit, BaseStyle.ColumnLimit);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2831,8 +2831,10 @@
   return false;
 return true;
   }
-  if (Left.is(TT_UnaryOperator))
-return Right.is(TT_BinaryOperator);
+  if (Left.is(TT_UnaryOperator)) {
+return (Style.SpaceAfterLogicalNot && Left.is(tok::exclaim)) ||
+   Right.is(TT_BinaryOperator);
+  }
 
   // If the next token is a binary operator or a selector name, we have
   // incorrectly classified the parenthesis as a cast. FIXME: Detect correctly.
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -478,6 +478,7 @@
 IO.mapOptional("SortIncludes", Style.SortIncludes);
 IO.mapOptional("SortUsingDeclarations", Style.SortUsingDeclarations);
 IO.mapOptional("SpaceAfterCStyleCast", Style.SpaceAfterCStyleCast);
+IO.mapOptional("SpaceAfterLogicalNot", Style.SpaceAfterLogicalNot);
 IO.mapOptional("SpaceAfterTemplateKeyword",
Style.SpaceAfterTemplateKeyword);
 IO.mapOptional("SpaceBeforeAssignmentOperators",
@@ -730,6 +731,7 @@
   LLVMStyle.SpacesInContainerLiterals = true;
   LLVMStyle.SpacesInCStyleCastParentheses = false;
   LLVMStyle.SpaceAfterCStyleCast = false;
+  LLVMStyle.SpaceAfterLogicalNot = false;
   LLVMStyle.SpaceAfterTemplateKeyword = true;
   LLVMStyle.SpaceBeforeCtorInitializerColon = true;
   LLVMStyle.SpaceBeforeInheritanceColon = true;
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1628,6 +1628,13 @@
   /// \endcode
   bool SpaceAfterCStyleCast;
 
+  /// If ``true``, a space is inserted after the logical not operator (``!``).
+  /// \code
+  ///true:  false:
+  ///! someExpression();vs. !someExpression();
+  /// \endcode
+  bool SpaceAfterLogicalNot;
+
   /// If \c true, a space will be inserted after the 'template' keyword.
   /// \code
   ///true:  false:
@@ -1911,6 +1918,7 @@
PointerAlignment == R.PointerAlignment &&
RawStringFormats == R.RawStringFormats &&
SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
+   SpaceAfterLogicalNot == R.SpaceAfterLogicalNot &&
SpaceAfterTemplateKeyword == R.SpaceAfterTemplateKeyword &&
SpaceBeforeAssignmentOperators == R.SpaceBeforeAssignmentOperators &&
SpaceBeforeCpp11BracedList == R.SpaceBeforeCpp11BracedList &&
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1952,6 +1952,13 @@
  true:  false:
  (int) i;   vs. (int)i;
 
+**SpaceAfterLogicalNot** (``bool``)
+  If ``true``, a space is inserted after the logical not operator (``!``).
+  .. code-block:: c++
+
+ true:

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

2019-04-05 Thread Reuben Thomas via Phabricator via cfe-commits
reuk updated this revision to Diff 193889.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60320/new/

https://reviews.llvm.org/D60320

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9683,6 +9683,16 @@
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 }
 
+TEST_F(FormatTest, SpaceAfterLogicalNot) {
+  FormatStyle Spaces = getLLVMStyle();
+  Spaces.SpaceAfterLogicalNot = true;
+
+  verifyFormat("bool x = ! y", Spaces);
+  verifyFormat("if (! isFailure())", Spaces);
+  verifyFormat("if (! (a && b))", Spaces);
+  verifyFormat("\"Error!\"", Spaces);
+}
+
 TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
   FormatStyle Spaces = getLLVMStyle();
 
@@ -11475,6 +11485,12 @@
   CHECK_PARSE("SpaceAfterControlStatementKeyword: true", SpaceBeforeParens,
   FormatStyle::SBPO_ControlStatements);
 
+  Style.SpaceAfterLogicalNot = false;
+  CHECK_PARSE("SpaceAfterLogicalNot: true", SpaceAfterLogicalNot,
+  true);
+  CHECK_PARSE("SpaceAfterLogicalNot: false", SpaceAfterLogicalNot,
+  false);
+
   Style.ColumnLimit = 123;
   FormatStyle BaseStyle = getLLVMStyle();
   CHECK_PARSE("BasedOnStyle: LLVM", ColumnLimit, BaseStyle.ColumnLimit);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2831,8 +2831,10 @@
   return false;
 return true;
   }
-  if (Left.is(TT_UnaryOperator))
-return Right.is(TT_BinaryOperator);
+  if (Left.is(TT_UnaryOperator)) {
+return (Style.SpaceAfterLogicalNot && Left.is(tok::exclaim)) ||
+   Right.is(TT_BinaryOperator);
+  }
 
   // If the next token is a binary operator or a selector name, we have
   // incorrectly classified the parenthesis as a cast. FIXME: Detect correctly.
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -489,6 +489,7 @@
 IO.mapOptional("SpaceBeforeInheritanceColon",
Style.SpaceBeforeInheritanceColon);
 IO.mapOptional("SpaceBeforeParens", Style.SpaceBeforeParens);
+IO.mapOptional("SpaceAfterLogicalNot", Style.SpaceAfterLogicalNot);
 IO.mapOptional("SpaceBeforeRangeBasedForLoopColon",
Style.SpaceBeforeRangeBasedForLoopColon);
 IO.mapOptional("SpaceInEmptyParentheses", Style.SpaceInEmptyParentheses);
@@ -726,6 +727,7 @@
   LLVMStyle.ReflowComments = true;
   LLVMStyle.SpacesInParentheses = false;
   LLVMStyle.SpacesInSquareBrackets = false;
+  LLVMStyle.SpaceAfterLogicalNot = false;
   LLVMStyle.SpaceInEmptyParentheses = false;
   LLVMStyle.SpacesInContainerLiterals = true;
   LLVMStyle.SpacesInCStyleCastParentheses = false;
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1718,6 +1718,9 @@
   /// Defines in which cases to put a space before opening parentheses.
   SpaceBeforeParensOptions SpaceBeforeParens;
 
+  /// Defines whether a space should be placed after a logical `!`
+  bool SpaceAfterLogicalNot;
+
   /// If ``false``, spaces will be removed before range-based for loop
   /// colon.
   /// \code
@@ -1918,6 +1921,7 @@
R.SpaceBeforeCtorInitializerColon &&
SpaceBeforeInheritanceColon == R.SpaceBeforeInheritanceColon &&
SpaceBeforeParens == R.SpaceBeforeParens &&
+   SpaceAfterLogicalNot == R.SpaceAfterLogicalNot &&
SpaceBeforeRangeBasedForLoopColon ==
R.SpaceBeforeRangeBasedForLoopColon &&
SpaceInEmptyParentheses == R.SpaceInEmptyParentheses &&


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9683,6 +9683,16 @@
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 }
 
+TEST_F(FormatTest, SpaceAfterLogicalNot) {
+  FormatStyle Spaces = getLLVMStyle();
+  Spaces.SpaceAfterLogicalNot = true;
+
+  verifyFormat("bool x = ! y", Spaces);
+  verifyFormat("if (! isFailure())", Spaces);
+  verifyFormat("if (! (a && b))", Spaces);
+  verifyFormat("\"Error!\"", Spaces);
+}
+
 TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
   FormatStyle Spaces = getLLVMStyle();
 
@@ -11475,6 +11485,12 @@
   CHECK_PARSE("SpaceAfterControlStatementKeyword: true", SpaceBeforeParens,
   FormatStyle::SBPO_ControlStatements);
 
+  Style.SpaceAfterLogicalNot = false;
+  

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

2019-04-05 Thread Reuben Thomas via Phabricator via cfe-commits
reuk created this revision.
reuk added reviewers: MyDeveloperDay, klimek.
reuk added a project: clang-tools-extra.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch aims to support the following rule from the Juce coding standards 
:

> The ! operator should always be followed by a space, e.g. if (! foo)

Leaving a space after `!` stops it from blending into the rest of the 
expression, which makes it easier to tell at a glance what the expression is 
doing.

Patch by Reuben Thomas


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D60320

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
@@ -9683,6 +9683,16 @@
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 }
 
+TEST_F(FormatTest, SpaceAfterLogicalNot) {
+  FormatStyle Spaces = getLLVMStyle();
+  Spaces.SpaceAfterLogicalNot = true;
+
+  verifyFormat("bool x = ! y", Spaces);
+  verifyFormat("if (! isFailure())", Spaces);
+  verifyFormat("if (! (a && b))", Spaces);
+  verifyFormat("\"Error!\"", Spaces);
+}
+
 TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
   FormatStyle Spaces = getLLVMStyle();
 
@@ -11475,6 +11485,12 @@
   CHECK_PARSE("SpaceAfterControlStatementKeyword: true", SpaceBeforeParens,
   FormatStyle::SBPO_ControlStatements);
 
+  Style.SpaceAfterLogicalNot = false;
+  CHECK_PARSE("SpaceAfterLogicalNot: true", SpaceAfterLogicalNot,
+  true);
+  CHECK_PARSE("SpaceAfterLogicalNot: false", SpaceAfterLogicalNot,
+  false);
+
   Style.ColumnLimit = 123;
   FormatStyle BaseStyle = getLLVMStyle();
   CHECK_PARSE("BasedOnStyle: LLVM", ColumnLimit, BaseStyle.ColumnLimit);
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2831,8 +2831,10 @@
   return false;
 return true;
   }
-  if (Left.is(TT_UnaryOperator))
-return Right.is(TT_BinaryOperator);
+  if (Left.is(TT_UnaryOperator)) {
+return (Style.SpaceAfterLogicalNot && Left.is(tok::exclaim)) ||
+   Right.is(TT_BinaryOperator);
+  }
 
   // If the next token is a binary operator or a selector name, we have
   // incorrectly classified the parenthesis as a cast. FIXME: Detect correctly.
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -489,6 +489,7 @@
 IO.mapOptional("SpaceBeforeInheritanceColon",
Style.SpaceBeforeInheritanceColon);
 IO.mapOptional("SpaceBeforeParens", Style.SpaceBeforeParens);
+IO.mapOptional("SpaceAfterLogicalNot", Style.SpaceAfterLogicalNot);
 IO.mapOptional("SpaceBeforeRangeBasedForLoopColon",
Style.SpaceBeforeRangeBasedForLoopColon);
 IO.mapOptional("SpaceInEmptyParentheses", Style.SpaceInEmptyParentheses);
@@ -726,6 +727,7 @@
   LLVMStyle.ReflowComments = true;
   LLVMStyle.SpacesInParentheses = false;
   LLVMStyle.SpacesInSquareBrackets = false;
+  LLVMStyle.SpaceAfterLogicalNot = false;
   LLVMStyle.SpaceInEmptyParentheses = false;
   LLVMStyle.SpacesInContainerLiterals = true;
   LLVMStyle.SpacesInCStyleCastParentheses = false;
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -1718,6 +1718,9 @@
   /// Defines in which cases to put a space before opening parentheses.
   SpaceBeforeParensOptions SpaceBeforeParens;
 
+  /// Defines whether a space should be placed after a logical `!`
+  bool SpaceAfterLogicalNot;
+
   /// If ``false``, spaces will be removed before range-based for loop
   /// colon.
   /// \code
@@ -1726,7 +1729,7 @@
   /// \endcode
   bool SpaceBeforeRangeBasedForLoopColon;
 
-  /// If ``true``, spaces may be inserted into ``()``.
+  /// \brief If ``true``, spaces may be inserted into ``()``.
   /// \code
   ///true:false:
   ///void f( ) {vs.   void f() {
@@ -1918,6 +1921,7 @@
R.SpaceBeforeCtorInitializerColon &&
SpaceBeforeInheritanceColon == R.SpaceBeforeInheritanceColon &&
SpaceBeforeParens == R.SpaceBeforeParens &&
+   SpaceAfterLogicalNot == R.SpaceAfterLogicalNot &&
SpaceBeforeRangeBasedForLoopColon ==
R.SpaceBeforeRangeBasedForLoopColon &&
SpaceInEmptyParentheses == R.SpaceInEmptyParentheses &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-30 Thread Reuben Thomas via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL357344: [clang-format]: Add NonEmptyParentheses spacing 
option (authored by reuk, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55170?vs=191560=192969#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55170/new/

https://reviews.llvm.org/D55170

Files:
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/lib/Format/TokenAnnotator.h
  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
@@ -1690,6 +1690,17 @@
 ///}
 /// \endcode
 SBPO_ControlStatements,
+/// Put a space before opening parentheses only if the parentheses are not
+/// empty i.e. '()'
+/// \code
+///   void() {
+/// if (true) {
+///   f();
+///   g (x, y, z);
+/// }
+///   }
+/// \endcode
+SBPO_NonEmptyParentheses,
 /// Always put a space before opening parentheses, except when it's
 /// prohibited by the syntax rules (in function-like macro definitions) or
 /// when determined by other style rules (after unary operators, opening
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -285,6 +285,8 @@
 IO.enumCase(Value, "Never", FormatStyle::SBPO_Never);
 IO.enumCase(Value, "ControlStatements",
 FormatStyle::SBPO_ControlStatements);
+IO.enumCase(Value, "NonEmptyParentheses",
+FormatStyle::SBPO_NonEmptyParentheses);
 IO.enumCase(Value, "Always", FormatStyle::SBPO_Always);
 
 // For backward compatibility.
Index: cfe/trunk/lib/Format/TokenAnnotator.h
===
--- cfe/trunk/lib/Format/TokenAnnotator.h
+++ cfe/trunk/lib/Format/TokenAnnotator.h
@@ -164,6 +164,8 @@
   unsigned splitPenalty(const AnnotatedLine , const FormatToken ,
 bool InFunctionDecl);
 
+  bool spaceRequiredBeforeParens(const FormatToken ) const;
+
   bool spaceRequiredBetween(const AnnotatedLine , const FormatToken ,
 const FormatToken );
 
Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2453,6 +2453,12 @@
   return 3;
 }
 
+bool TokenAnnotator::spaceRequiredBeforeParens(const FormatToken ) const {
+  return Style.SpaceBeforeParens == FormatStyle::SBPO_Always ||
+ (Style.SpaceBeforeParens == FormatStyle::SBPO_NonEmptyParentheses &&
+  Right.ParameterCount > 0);
+}
+
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine ,
   const FormatToken ,
   const FormatToken ) {
@@ -2599,9 +2605,9 @@
  (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch,
tok::kw_new, tok::kw_delete) &&
   (!Left.Previous || Left.Previous->isNot(tok::period) ||
-   (Style.SpaceBeforeParens == FormatStyle::SBPO_Always &&
+   (spaceRequiredBeforeParens(Right) &&
 (Left.is(tok::identifier) || Left.isFunctionLikeKeyword() ||
- Left.is(tok::r_paren) ||
+ Left.is(tok::r_paren) || Left.isSimpleTypeSpecifier() ||
  (Left.is(tok::r_square) && Left.MatchingParen &&
   Left.MatchingParen->is(TT_LambdaLSquare))) &&
 Line.Type != LT_PreprocessorDirective);
@@ -2795,7 +2801,7 @@
   Left.isOneOf(TT_TrailingReturnArrow, TT_LambdaArrow))
 return true;
   if (Right.is(TT_OverloadedOperatorLParen))
-return Style.SpaceBeforeParens == FormatStyle::SBPO_Always;
+return spaceRequiredBeforeParens(Right);
   if (Left.is(tok::comma))
 return true;
   if (Right.is(tok::comma))
@@ -2879,7 +2885,7 @@
 return true;
   if (Left.is(TT_TemplateCloser) && Right.is(tok::l_paren) &&
   Right.isNot(TT_FunctionTypeLParen))
-return Style.SpaceBeforeParens == FormatStyle::SBPO_Always;
+return spaceRequiredBeforeParens(Right);
   if (Right.is(TT_TemplateOpener) && Left.is(tok::r_paren) &&
   Left.MatchingParen && Left.MatchingParen->is(TT_OverloadedOperatorLParen))
 return false;
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -9627,6 +9627,60 @@
   verifyFormat("T A::operator() 

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-20 Thread Reuben Thomas via Phabricator via cfe-commits
reuk updated this revision to Diff 191560.
reuk added a comment.

Removed unnecessary parens.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55170/new/

https://reviews.llvm.org/D55170

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9337,6 +9337,60 @@
   verifyFormat("T A::operator() ();", Space);
   verifyFormat("X A::operator++ (T);", Space);
   verifyFormat("auto lambda = [] () { return 0; };", Space);
+  verifyFormat("int x = int (y);", Space);
+
+  FormatStyle SomeSpace = getLLVMStyle();
+  SomeSpace.SpaceBeforeParens = FormatStyle::SBPO_NonEmptyParentheses;
+
+  verifyFormat("[]() -> float {}", SomeSpace);
+  verifyFormat("[] (auto foo) {}", SomeSpace);
+  verifyFormat("[foo]() -> int {}", SomeSpace);
+  verifyFormat("int f();", SomeSpace);
+  verifyFormat("void f (int a, T b) {\n"
+   "  while (true)\n"
+   "continue;\n"
+   "}",
+   SomeSpace);
+  verifyFormat("if (true)\n"
+   "  f();\n"
+   "else if (true)\n"
+   "  f();",
+   SomeSpace);
+  verifyFormat("do {\n"
+   "  do_something();\n"
+   "} while (something());",
+   SomeSpace);
+  verifyFormat("switch (x) {\n"
+   "default:\n"
+   "  break;\n"
+   "}",
+   SomeSpace);
+  verifyFormat("A::A() : a (1) {}", SomeSpace);
+  verifyFormat("void f() __attribute__ ((asdf));", SomeSpace);
+  verifyFormat("*( + 1);\n"
+   "&(()[1]);\n"
+   "a[(b + c) * d];\n"
+   "(((a + 1) * 2) + 3) * 4;",
+   SomeSpace);
+  verifyFormat("#define A(x) x", SomeSpace);
+  verifyFormat("#define A (x) x", SomeSpace);
+  verifyFormat("#if defined(x)\n"
+   "#endif",
+   SomeSpace);
+  verifyFormat("auto i = std::make_unique (5);", SomeSpace);
+  verifyFormat("size_t x = sizeof (x);", SomeSpace);
+  verifyFormat("auto f (int x) -> decltype (x);", SomeSpace);
+  verifyFormat("int f (T x) noexcept (x.create());", SomeSpace);
+  verifyFormat("alignas (128) char a[128];", SomeSpace);
+  verifyFormat("size_t x = alignof (MyType);", SomeSpace);
+  verifyFormat("static_assert (sizeof (char) == 1, \"Impossible!\");",
+   SomeSpace);
+  verifyFormat("int f() throw (Deprecated);", SomeSpace);
+  verifyFormat("typedef void (*cb) (int);", SomeSpace);
+  verifyFormat("T A::operator()();", SomeSpace);
+  verifyFormat("X A::operator++ (T);", SomeSpace);
+  verifyFormat("int x = int (y);", SomeSpace);
+  verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 }
 
 TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
@@ -11121,6 +11175,8 @@
   FormatStyle::SBPO_Always);
   CHECK_PARSE("SpaceBeforeParens: ControlStatements", SpaceBeforeParens,
   FormatStyle::SBPO_ControlStatements);
+  CHECK_PARSE("SpaceBeforeParens: NonEmptyParentheses", SpaceBeforeParens,
+  FormatStyle::SBPO_NonEmptyParentheses);
   // For backward compatibility:
   CHECK_PARSE("SpaceAfterControlStatementKeyword: false", SpaceBeforeParens,
   FormatStyle::SBPO_Never);
Index: lib/Format/TokenAnnotator.h
===
--- lib/Format/TokenAnnotator.h
+++ lib/Format/TokenAnnotator.h
@@ -164,6 +164,8 @@
   unsigned splitPenalty(const AnnotatedLine , const FormatToken ,
 bool InFunctionDecl);
 
+  bool spaceRequiredBeforeParens(const FormatToken ) const;
+
   bool spaceRequiredBetween(const AnnotatedLine , const FormatToken ,
 const FormatToken );
 
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2403,6 +2403,12 @@
   return 3;
 }
 
+bool TokenAnnotator::spaceRequiredBeforeParens(const FormatToken ) const {
+  return Style.SpaceBeforeParens == FormatStyle::SBPO_Always ||
+ (Style.SpaceBeforeParens == FormatStyle::SBPO_NonEmptyParentheses &&
+  Right.ParameterCount > 0);
+}
+
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine ,
   const FormatToken ,
   const FormatToken ) {
@@ -2549,9 +2555,9 @@
  (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch,
tok::kw_new, tok::kw_delete) &&
   (!Left.Previous || Left.Previous->isNot(tok::period) ||
-   (Style.SpaceBeforeParens == FormatStyle::SBPO_Always &&
+   (spaceRequiredBeforeParens(Right) &&
 (Left.is(tok::identifier) || 

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-20 Thread Reuben Thomas via Phabricator via cfe-commits
reuk marked an inline comment as done.
reuk added inline comments.



Comment at: lib/Format/TokenAnnotator.cpp:2546-2560
 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||
(Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
-(Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, tok::kw_while,
-  tok::kw_switch, tok::kw_case, TT_ForEachMacro,
-  TT_ObjCForIn) ||
- Left.endsSequence(tok::kw_constexpr, tok::kw_if) ||
- (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch,
-   tok::kw_new, tok::kw_delete) &&
-  (!Left.Previous || Left.Previous->isNot(tok::period) ||
-   (Style.SpaceBeforeParens == FormatStyle::SBPO_Always &&
-(Left.is(tok::identifier) || Left.isFunctionLikeKeyword() ||
- Left.is(tok::r_paren) ||
- (Left.is(tok::r_square) && Left.MatchingParen &&
-  Left.MatchingParen->is(TT_LambdaLSquare))) &&
-Line.Type != LT_PreprocessorDirective);
+((Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, 
tok::kw_while,
+   tok::kw_switch, tok::kw_case, TT_ForEachMacro,
+   TT_ObjCForIn) ||
+  Left.endsSequence(tok::kw_constexpr, tok::kw_if) ||
+  (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch,

klimek wrote:
> I'm really confused about the changes os parens. It seems like this should 
> just change the spaceRequiredBeforeParens(...), but instead seems to change 
> parens in a way that completely changes the logic? (or alternatively is 
> phabricator showing this wrong?)
I think this looks like a bigger change than it really is because I added an 
open-parens on line 2548, which then affected the formatting of the following 
lines. I also added the `isSimpleTypeSpecifier` check, so that functional-style 
casts (`int (foo)`) are given the correct spacing.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55170/new/

https://reviews.llvm.org/D55170



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


[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-18 Thread Reuben Thomas via Phabricator via cfe-commits
reuk added a comment.

@klimek I agree that the rule is somewhat arbitrary. However, it's the style 
rule of an established codebase with many users (I don't have a concrete 
number, but the project has 1400 stars on github 
). I've found this patch useful when 
contributing to JUCE and I thought others might too.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55170/new/

https://reviews.llvm.org/D55170



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


[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-17 Thread Reuben Thomas via Phabricator via cfe-commits
reuk updated this revision to Diff 191050.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55170/new/

https://reviews.llvm.org/D55170

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9337,6 +9337,60 @@
   verifyFormat("T A::operator() ();", Space);
   verifyFormat("X A::operator++ (T);", Space);
   verifyFormat("auto lambda = [] () { return 0; };", Space);
+  verifyFormat("int x = int (y);", Space);
+
+  FormatStyle SomeSpace = getLLVMStyle();
+  SomeSpace.SpaceBeforeParens = FormatStyle::SBPO_NonEmptyParentheses;
+
+  verifyFormat("[]() -> float {}", SomeSpace);
+  verifyFormat("[] (auto foo) {}", SomeSpace);
+  verifyFormat("[foo]() -> int {}", SomeSpace);
+  verifyFormat("int f();", SomeSpace);
+  verifyFormat("void f (int a, T b) {\n"
+   "  while (true)\n"
+   "continue;\n"
+   "}",
+   SomeSpace);
+  verifyFormat("if (true)\n"
+   "  f();\n"
+   "else if (true)\n"
+   "  f();",
+   SomeSpace);
+  verifyFormat("do {\n"
+   "  do_something();\n"
+   "} while (something());",
+   SomeSpace);
+  verifyFormat("switch (x) {\n"
+   "default:\n"
+   "  break;\n"
+   "}",
+   SomeSpace);
+  verifyFormat("A::A() : a (1) {}", SomeSpace);
+  verifyFormat("void f() __attribute__ ((asdf));", SomeSpace);
+  verifyFormat("*( + 1);\n"
+   "&(()[1]);\n"
+   "a[(b + c) * d];\n"
+   "(((a + 1) * 2) + 3) * 4;",
+   SomeSpace);
+  verifyFormat("#define A(x) x", SomeSpace);
+  verifyFormat("#define A (x) x", SomeSpace);
+  verifyFormat("#if defined(x)\n"
+   "#endif",
+   SomeSpace);
+  verifyFormat("auto i = std::make_unique (5);", SomeSpace);
+  verifyFormat("size_t x = sizeof (x);", SomeSpace);
+  verifyFormat("auto f (int x) -> decltype (x);", SomeSpace);
+  verifyFormat("int f (T x) noexcept (x.create());", SomeSpace);
+  verifyFormat("alignas (128) char a[128];", SomeSpace);
+  verifyFormat("size_t x = alignof (MyType);", SomeSpace);
+  verifyFormat("static_assert (sizeof (char) == 1, \"Impossible!\");",
+   SomeSpace);
+  verifyFormat("int f() throw (Deprecated);", SomeSpace);
+  verifyFormat("typedef void (*cb) (int);", SomeSpace);
+  verifyFormat("T A::operator()();", SomeSpace);
+  verifyFormat("X A::operator++ (T);", SomeSpace);
+  verifyFormat("int x = int (y);", SomeSpace);
+  verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 }
 
 TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
@@ -11121,6 +11175,8 @@
   FormatStyle::SBPO_Always);
   CHECK_PARSE("SpaceBeforeParens: ControlStatements", SpaceBeforeParens,
   FormatStyle::SBPO_ControlStatements);
+  CHECK_PARSE("SpaceBeforeParens: NonEmptyParentheses", SpaceBeforeParens,
+  FormatStyle::SBPO_NonEmptyParentheses);
   // For backward compatibility:
   CHECK_PARSE("SpaceAfterControlStatementKeyword: false", SpaceBeforeParens,
   FormatStyle::SBPO_Never);
Index: lib/Format/TokenAnnotator.h
===
--- lib/Format/TokenAnnotator.h
+++ lib/Format/TokenAnnotator.h
@@ -164,6 +164,8 @@
   unsigned splitPenalty(const AnnotatedLine , const FormatToken ,
 bool InFunctionDecl);
 
+  bool spaceRequiredBeforeParens(const FormatToken ) const;
+
   bool spaceRequiredBetween(const AnnotatedLine , const FormatToken ,
 const FormatToken );
 
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2400,6 +2400,12 @@
   return 3;
 }
 
+bool TokenAnnotator::spaceRequiredBeforeParens(const FormatToken ) const {
+  return Style.SpaceBeforeParens == FormatStyle::SBPO_Always ||
+ (Style.SpaceBeforeParens == FormatStyle::SBPO_NonEmptyParentheses &&
+  Right.ParameterCount > 0);
+}
+
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine ,
   const FormatToken ,
   const FormatToken ) {
@@ -2539,19 +2545,19 @@
   return true;
 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||
(Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
-(Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, tok::kw_while,
-  tok::kw_switch, tok::kw_case, TT_ForEachMacro,
-  TT_ObjCForIn) ||
- Left.endsSequence(tok::kw_constexpr, tok::kw_if) ||
- 

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-17 Thread Reuben Thomas via Phabricator via cfe-commits
reuk updated this revision to Diff 191015.
reuk added a comment.

@MyDeveloperDay I'm sorry, you're absolutely correct. I'd got some other 
JUCE-related changes mixed up in this PR. Should be fixed now.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55170/new/

https://reviews.llvm.org/D55170

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9337,6 +9337,60 @@
   verifyFormat("T A::operator() ();", Space);
   verifyFormat("X A::operator++ (T);", Space);
   verifyFormat("auto lambda = [] () { return 0; };", Space);
+  verifyFormat("int x = int (y);", Space);
+
+  FormatStyle SomeSpace = getLLVMStyle();
+  SomeSpace.SpaceBeforeParens = FormatStyle::SBPO_NonEmptyParentheses;
+
+  verifyFormat("[]() -> float {}", SomeSpace);
+  verifyFormat("[] (auto foo) {}", SomeSpace);
+  verifyFormat("[foo]() -> int {}", SomeSpace);
+  verifyFormat("int f();", SomeSpace);
+  verifyFormat("void f (int a, T b) {\n"
+   "  while (true)\n"
+   "continue;\n"
+   "}",
+   SomeSpace);
+  verifyFormat("if (true)\n"
+   "  f();\n"
+   "else if (true)\n"
+   "  f();",
+   SomeSpace);
+  verifyFormat("do {\n"
+   "  do_something();\n"
+   "} while (something());",
+   SomeSpace);
+  verifyFormat("switch (x) {\n"
+   "default:\n"
+   "  break;\n"
+   "}",
+   SomeSpace);
+  verifyFormat("A::A() : a (1) {}", SomeSpace);
+  verifyFormat("void f() __attribute__ ((asdf));", SomeSpace);
+  verifyFormat("*( + 1);\n"
+   "&(()[1]);\n"
+   "a[(b + c) * d];\n"
+   "(((a + 1) * 2) + 3) * 4;",
+   SomeSpace);
+  verifyFormat("#define A(x) x", SomeSpace);
+  verifyFormat("#define A (x) x", SomeSpace);
+  verifyFormat("#if defined(x)\n"
+   "#endif",
+   SomeSpace);
+  verifyFormat("auto i = std::make_unique (5);", SomeSpace);
+  verifyFormat("size_t x = sizeof (x);", SomeSpace);
+  verifyFormat("auto f (int x) -> decltype (x);", SomeSpace);
+  verifyFormat("int f (T x) noexcept (x.create());", SomeSpace);
+  verifyFormat("alignas (128) char a[128];", SomeSpace);
+  verifyFormat("size_t x = alignof (MyType);", SomeSpace);
+  verifyFormat("static_assert (sizeof (char) == 1, \"Impossible!\");",
+   SomeSpace);
+  verifyFormat("int f() throw (Deprecated);", SomeSpace);
+  verifyFormat("typedef void (*cb) (int);", SomeSpace);
+  verifyFormat("T A::operator()();", SomeSpace);
+  verifyFormat("X A::operator++ (T);", SomeSpace);
+  verifyFormat("int x = int (y);", SomeSpace);
+  verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 }
 
 TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
@@ -11121,6 +11175,8 @@
   FormatStyle::SBPO_Always);
   CHECK_PARSE("SpaceBeforeParens: ControlStatements", SpaceBeforeParens,
   FormatStyle::SBPO_ControlStatements);
+  CHECK_PARSE("SpaceBeforeParens: NonEmptyParentheses", SpaceBeforeParens,
+  FormatStyle::SBPO_NonEmptyParentheses);
   // For backward compatibility:
   CHECK_PARSE("SpaceAfterControlStatementKeyword: false", SpaceBeforeParens,
   FormatStyle::SBPO_Never);
Index: lib/Format/TokenAnnotator.h
===
--- lib/Format/TokenAnnotator.h
+++ lib/Format/TokenAnnotator.h
@@ -164,6 +164,8 @@
   unsigned splitPenalty(const AnnotatedLine , const FormatToken ,
 bool InFunctionDecl);
 
+  bool spaceRequiredBeforeParens(const FormatToken ) const;
+
   bool spaceRequiredBetween(const AnnotatedLine , const FormatToken ,
 const FormatToken );
 
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2400,6 +2400,12 @@
   return 3;
 }
 
+bool TokenAnnotator::spaceRequiredBeforeParens(const FormatToken ) const {
+  return Style.SpaceBeforeParens == FormatStyle::SBPO_Always ||
+ (Style.SpaceBeforeParens == FormatStyle::SBPO_NonEmptyParentheses &&
+  Right.ParameterCount > 0);
+}
+
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine ,
   const FormatToken ,
   const FormatToken ) {
@@ -2539,19 +2545,19 @@
   return true;
 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||
(Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
-(Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, tok::kw_while,
-  tok::kw_switch, 

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-15 Thread Reuben Thomas via Phabricator via cfe-commits
reuk added a comment.

@MyDeveloperDay I'm not sure you built this branch. Perhaps you applied this 
patch to an older version of the repo. For me, 
SpaceBeforeCpp11BracedListOptions is defined at Format.h:1570.

This builds fine for me, on macOS 10.14 with Xcode 10.1's clang. I just rebased 
onto the most recent clang master and updated llvm. I've been testing/building 
with this command:

  cmake --build build --target FormatTests && 
./build/tools/clang/unittests/Format/FormatTests && cmake --build build 
--target clang-format

@klimek That's disappointing, I thought other JUCE users might benefit from 
this patch (it's quite an established library), and the amount of code that I'm 
a adding is not that high.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55170/new/

https://reviews.llvm.org/D55170



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


[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

2019-03-12 Thread Reuben Thomas via Phabricator via cfe-commits
reuk accepted this revision.
reuk added a comment.
This revision is now accepted and ready to land.

LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59087/new/

https://reviews.llvm.org/D59087



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


[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

2019-03-11 Thread Reuben Thomas via Phabricator via cfe-commits
reuk added a comment.

The code looks good now. There's just a few places left where the formatting 
looks a bit suspect (sorry for missing those last time). I think once that's 
fixed this will be good to go.




Comment at: clang/unittests/Format/FormatTest.cpp:553
 
-  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = true;
+  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = 
FormatStyle::SIS_WithoutElse;
   AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true;

This line, along with some of the following changed lines, have more than 80 
characters. Make sure to clang-format changes! Consider using 
`git-clang-format` to format just this patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59087/new/

https://reviews.llvm.org/D59087



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


[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

2019-03-11 Thread Reuben Thomas via Phabricator via cfe-commits
reuk added a comment.

This looks much better now, thanks for taking another look! I've flagged some 
formatting/spelling nits, and I think the tests/docs could be a little more 
explicit about the behaviour of `WithoutElse`. Other than that, looks great!




Comment at: clang/docs/ClangFormatStyleOptions.rst:375
+  * ``SIS_Never`` (in configuration: ``Never``)
+Do not allow short if functions
+

For consistency, these descriptions should end with periods `.`



Comment at: clang/docs/ClangFormatStyleOptions.rst:386
+Allow short if functions on the same line, as long as else
+is not a compound statement
 

Missing `.`



Comment at: clang/docs/ClangFormatStyleOptions.rst:390
+
+   if (a) return;
+   else 

I think an example of the generated formatting *with* a compound else statement 
would be useful here too, in addition to the existing example.



Comment at: clang/docs/ClangFormatStyleOptions.rst:395
+  * ``SIS_Always`` (in configuration: ``Always``)
+Allow short if statements even if the else is a compounf statement
+

compounf -> compound



Comment at: clang/docs/ClangFormatStyleOptions.rst:395
+  * ``SIS_Always`` (in configuration: ``Always``)
+Allow short if statements even if the else is a compounf statement
+

reuk wrote:
> compounf -> compound
Missing `.`



Comment at: clang/include/clang/Format/Format.h:264
+/// Always put short ifs on the same line if
+/// the else is not a compound statement or not
+/// \code

Missing `.`



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:417
+// Only inline simple if's (no nested if or else), unless specified
+if (Style.AllowShortIfStatementsOnASingleLine!=FormatStyle::SIS_Always) {
+  if (I + 2 != E && Line.startsWith(tok::kw_if) &&

Has this line been clang-formatted? I'd expect spaces around the binop



Comment at: clang/unittests/Format/FormatTest.cpp:494
+  AllowsMergedIf.AllowShortIfStatementsOnASingleLine = 
FormatStyle::SIS_WithoutElse;
+  verifyFormat("if (a)\n"
+   "  f();\n"

I think having a test here which shows the expected behaviour for non-compound 
else statements with `WithoutElse` would be helpful, as in the documentation.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59087/new/

https://reviews.llvm.org/D59087



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


[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

2019-03-10 Thread Reuben Thomas via Phabricator via cfe-commits
reuk added a comment.

Does it make sense to allow `AllowShortIfElseStatementsOnASingleLine` to be 
enabled when `AllowShortIfStatementsOnASingleLine` is not?

Perhaps it would be better to have `AllowShortIfStatementsOnASingleLine` be 
represented by an enum with values `Never`, `WithoutElse`, `Always`. The old 
`true/false` values from config files prior to this change would need to be 
made to map to the appropriate enum keys.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59087/new/

https://reviews.llvm.org/D59087



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


[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-09 Thread Reuben Thomas via Phabricator via cfe-commits
reuk added a comment.

Thanks for the review, and for approving this PR. It's very much appreciated!

I don't have merge rights - could someone commit this for me please?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55170/new/

https://reviews.llvm.org/D55170



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


[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-08 Thread Reuben Thomas via Phabricator via cfe-commits
reuk updated this revision to Diff 189942.
reuk added a comment.

I've rebased onto master, and removed unrelated formatting changes. I've also 
tried to remove some of the duplicate parens-related expressions.

I agree that the heavy nested boolean expressions are a bit painful to read, 
but I'm not sure whether a more general tidy-up falls within the scope of this 
PR.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55170/new/

https://reviews.llvm.org/D55170

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9296,6 +9296,60 @@
   verifyFormat("T A::operator() ();", Space);
   verifyFormat("X A::operator++ (T);", Space);
   verifyFormat("auto lambda = [] () { return 0; };", Space);
+  verifyFormat("int x = int (y);", Space);
+
+  FormatStyle SomeSpace = getLLVMStyle();
+  SomeSpace.SpaceBeforeParens = FormatStyle::SBPO_NonEmptyParentheses;
+
+  verifyFormat("[]() -> float {}", SomeSpace);
+  verifyFormat("[] (auto foo) {}", SomeSpace);
+  verifyFormat("[foo]() -> int {}", SomeSpace);
+  verifyFormat("int f();", SomeSpace);
+  verifyFormat("void f (int a, T b) {\n"
+   "  while (true)\n"
+   "continue;\n"
+   "}",
+   SomeSpace);
+  verifyFormat("if (true)\n"
+   "  f();\n"
+   "else if (true)\n"
+   "  f();",
+   SomeSpace);
+  verifyFormat("do {\n"
+   "  do_something();\n"
+   "} while (something());",
+   SomeSpace);
+  verifyFormat("switch (x) {\n"
+   "default:\n"
+   "  break;\n"
+   "}",
+   SomeSpace);
+  verifyFormat("A::A() : a (1) {}", SomeSpace);
+  verifyFormat("void f() __attribute__ ((asdf));", SomeSpace);
+  verifyFormat("*( + 1);\n"
+   "&(()[1]);\n"
+   "a[(b + c) * d];\n"
+   "(((a + 1) * 2) + 3) * 4;",
+   SomeSpace);
+  verifyFormat("#define A(x) x", SomeSpace);
+  verifyFormat("#define A (x) x", SomeSpace);
+  verifyFormat("#if defined(x)\n"
+   "#endif",
+   SomeSpace);
+  verifyFormat("auto i = std::make_unique (5);", SomeSpace);
+  verifyFormat("size_t x = sizeof (x);", SomeSpace);
+  verifyFormat("auto f (int x) -> decltype (x);", SomeSpace);
+  verifyFormat("int f (T x) noexcept (x.create());", SomeSpace);
+  verifyFormat("alignas (128) char a[128];", SomeSpace);
+  verifyFormat("size_t x = alignof (MyType);", SomeSpace);
+  verifyFormat("static_assert (sizeof (char) == 1, \"Impossible!\");",
+   SomeSpace);
+  verifyFormat("int f() throw (Deprecated);", SomeSpace);
+  verifyFormat("typedef void (*cb) (int);", SomeSpace);
+  verifyFormat("T A::operator()();", SomeSpace);
+  verifyFormat("X A::operator++ (T);", SomeSpace);
+  verifyFormat("int x = int (y);", SomeSpace);
+  verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 }
 
 TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
@@ -11080,6 +11134,8 @@
   FormatStyle::SBPO_Always);
   CHECK_PARSE("SpaceBeforeParens: ControlStatements", SpaceBeforeParens,
   FormatStyle::SBPO_ControlStatements);
+  CHECK_PARSE("SpaceBeforeParens: NonEmptyParentheses", SpaceBeforeParens,
+  FormatStyle::SBPO_NonEmptyParentheses);
   // For backward compatibility:
   CHECK_PARSE("SpaceAfterControlStatementKeyword: false", SpaceBeforeParens,
   FormatStyle::SBPO_Never);
Index: lib/Format/TokenAnnotator.h
===
--- lib/Format/TokenAnnotator.h
+++ lib/Format/TokenAnnotator.h
@@ -164,6 +164,8 @@
   unsigned splitPenalty(const AnnotatedLine , const FormatToken ,
 bool InFunctionDecl);
 
+  bool spaceRequiredBeforeParens(const FormatToken ) const;
+
   bool spaceRequiredBetween(const AnnotatedLine , const FormatToken ,
 const FormatToken );
 
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2400,6 +2400,12 @@
   return 3;
 }
 
+bool TokenAnnotator::spaceRequiredBeforeParens(const FormatToken ) const {
+  return Style.SpaceBeforeParens == FormatStyle::SBPO_Always ||
+ (Style.SpaceBeforeParens == FormatStyle::SBPO_NonEmptyParentheses &&
+  Right.ParameterCount > 0);
+}
+
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine ,
   const FormatToken ,
   const FormatToken ) {
@@ -2539,19 +2545,19 @@
   return true;
 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2018-12-10 Thread Reuben Thomas via Phabricator via cfe-commits
reuk added a subscriber: klimek.
reuk added a comment.

Would someone review this please? I'm not sure who to add for review (sorry), 
maybe one of the following?
@klimek @Typz @krasimir


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55170/new/

https://reviews.llvm.org/D55170



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


[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2018-12-01 Thread Reuben Thomas via Phabricator via cfe-commits
reuk created this revision.
reuk added reviewers: Typz, krasimir, cfe-commits.

This patch aims to add support for the following rules from the JUCE coding 
standards:

- Always put a space before an open parenthesis that contains text - e.g. foo 
(123);
- Never put a space before an empty pair of open/close parenthesis - e.g. foo();

The entire set of JUCE coding guidelines can be found here 
. Unfortunately, 
clang-format can't enforce all of these style rules at the moment, so I'm 
trying to add support for them.

Patch by Reuben Thomas


Repository:
  rC Clang

https://reviews.llvm.org/D55170

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
@@ -9271,6 +9271,59 @@
   verifyFormat("typedef void (*cb) (int);", Space);
   verifyFormat("T A::operator() ();", Space);
   verifyFormat("X A::operator++ (T);", Space);
+  verifyFormat("int x = int (y);", Space);
+
+  FormatStyle SomeSpace = getLLVMStyle();
+  SomeSpace.SpaceBeforeParens = FormatStyle::SBPO_NonEmptyParentheses;
+
+  verifyFormat("[]() -> float {}", SomeSpace);
+  verifyFormat("[] (auto foo) {}", SomeSpace);
+  verifyFormat("[foo]() -> int {}", SomeSpace);
+  verifyFormat("int f();", SomeSpace);
+  verifyFormat("void f (int a, T b) {\n"
+   "  while (true)\n"
+   "continue;\n"
+   "}",
+   SomeSpace);
+  verifyFormat("if (true)\n"
+   "  f();\n"
+   "else if (true)\n"
+   "  f();",
+   SomeSpace);
+  verifyFormat("do {\n"
+   "  do_something();\n"
+   "} while (something());",
+   SomeSpace);
+  verifyFormat("switch (x) {\n"
+   "default:\n"
+   "  break;\n"
+   "}",
+   SomeSpace);
+  verifyFormat("A::A() : a (1) {}", SomeSpace);
+  verifyFormat("void f() __attribute__ ((asdf));", SomeSpace);
+  verifyFormat("*( + 1);\n"
+   "&(()[1]);\n"
+   "a[(b + c) * d];\n"
+   "(((a + 1) * 2) + 3) * 4;",
+   SomeSpace);
+  verifyFormat("#define A(x) x", SomeSpace);
+  verifyFormat("#define A (x) x", SomeSpace);
+  verifyFormat("#if defined(x)\n"
+   "#endif",
+   SomeSpace);
+  verifyFormat("auto i = std::make_unique (5);", SomeSpace);
+  verifyFormat("size_t x = sizeof (x);", SomeSpace);
+  verifyFormat("auto f (int x) -> decltype (x);", SomeSpace);
+  verifyFormat("int f (T x) noexcept (x.create());", SomeSpace);
+  verifyFormat("alignas (128) char a[128];", SomeSpace);
+  verifyFormat("size_t x = alignof (MyType);", SomeSpace);
+  verifyFormat("static_assert (sizeof (char) == 1, \"Impossible!\");",
+   SomeSpace);
+  verifyFormat("int f() throw (Deprecated);", SomeSpace);
+  verifyFormat("typedef void (*cb) (int);", SomeSpace);
+  verifyFormat("T A::operator()();", SomeSpace);
+  verifyFormat("X A::operator++ (T);", SomeSpace);
+  verifyFormat("int x = int (y);", SomeSpace);
 }
 
 TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
@@ -11055,6 +11108,8 @@
   FormatStyle::SBPO_Always);
   CHECK_PARSE("SpaceBeforeParens: ControlStatements", SpaceBeforeParens,
   FormatStyle::SBPO_ControlStatements);
+  CHECK_PARSE("SpaceBeforeParens: NonEmptyParentheses", SpaceBeforeParens,
+  FormatStyle::SBPO_NonEmptyParentheses);
   // For backward compatibility:
   CHECK_PARSE("SpaceAfterControlStatementKeyword: false", SpaceBeforeParens,
   FormatStyle::SBPO_Never);
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -62,7 +62,7 @@
 if (NonTemplateLess.count(CurrentToken->Previous))
   return false;
 
-const FormatToken  = *CurrentToken->Previous;  // The '<'.
+const FormatToken  = *CurrentToken->Previous; // The '<'.
 if (Previous.Previous) {
   if (Previous.Previous->Tok.isLiteral())
 return false;
@@ -366,7 +366,7 @@
   // specifier parameter, although this is technically valid:
   // [[foo(:)]]
   if (AttrTok->is(tok::colon) ||
-  AttrTok->startsSequence(tok::identifier, tok::identifier) || 
+  AttrTok->startsSequence(tok::identifier, tok::identifier) ||
   AttrTok->startsSequence(tok::r_paren, tok::identifier))
 return false;
   if (AttrTok->is(tok::ellipsis))
@@ -523,11 +523,11 @@
 // Here, we set FirstObjCSelectorName when the end of the method call is
 // reached, in case it was not set already.
 if (!Contexts.back().FirstObjCSelectorName) {
-FormatToken* Previous = CurrentToken->getPreviousNonComment();
-