[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-dev
Subject:    [cfe-dev] [PATCH] clang-format: Add SpacesInParenthesesStyle
From:       Aaron Wishnick <aaron.s.wishnick () gmail ! com>
Date:       2014-03-31 17:13:15
Message-ID: CAA0GamCOdYYu5hvBjeysURq3GksB923vVZM14PTSQaXnCzqS_w () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


This patch adds an option to control when spaces are added inside of
parentheses. My employer's style guide mandates that spaces go inside the
outermost pair of parentheses, but not the rest. For example:

if( someFunction(a, b, c) ) {
  doThing( f(x), f(g(x)) );
}

My attached patch implements this feature with a new option,
SpacesInParenthesesStyle, which can either be "Always" (the previous
behavior, and the default), or "Outermost", the new behavior used by my
organization.

Does this seem like a reasonable strategy? The new option defaults to the
previous behavior. I see that a different approach was taken with
SpaceBeforeParensOptions. I went with this approach, because the option
applies to spaces inserted inside of parentheses due to
SpacesInParentheses, SpaceInEmptyParentheses,
SpacesInCStyleCastParentheses, etc.

Thanks,
Aaron

[Attachment #5 (text/html)]

<div dir="ltr">This patch adds an option to control when spaces are added inside of \
parentheses. My employer&#39;s style guide mandates that spaces go inside the \
outermost pair of parentheses, but not the rest. For example:<div> <br></div><div>if( \
someFunction(a, b, c) ) {</div><div>  doThing( f(x), f(g(x)) \
);</div><div>}</div><div><br></div><div>My attached patch implements this feature \
with a new option, SpacesInParenthesesStyle, which can either be &quot;Always&quot; \
(the previous behavior, and the default), or &quot;Outermost&quot;, the new behavior \
used by my organization.</div> <div><br></div><div>Does this seem like a reasonable \
strategy? The new option defaults to the previous behavior. I see that a different \
approach was taken with SpaceBeforeParensOptions. I went with this approach, because \
the option applies to spaces inserted inside of parentheses due to \
SpacesInParentheses, SpaceInEmptyParentheses, SpacesInCStyleCastParentheses, \
etc.</div> <div><br></div><div>Thanks,<br>Aaron</div></div>

--f46d04479f93c3623404f5ea2a9f--


["SpacesInParenthesesStyle.patch" (application/octet-stream)]

Index: include/clang/Format/Format.h
===================================================================
--- include/clang/Format/Format.h	(revision 204624)
+++ include/clang/Format/Format.h	(working copy)
@@ -275,6 +275,18 @@
   /// \brief If \c true, spaces may be inserted into C style casts.
   bool SpacesInCStyleCastParentheses;
 
+  /// \brief Different ways of inserting spaces inside parentheses.
+  enum ParenthesesSpacingStyle {
+    /// Always insert spaces inside parentheses.
+    PS_Always,
+    /// Only insert spaces inside the outermost level of parentheses.
+    PS_Outermost
+  };
+
+  /// \brief The style for inserting spaces inside of parentheses, when it is
+  /// enabled.
+  ParenthesesSpacingStyle SpacesInParenthesesStyle;
+
   /// \brief Different ways to put a space before opening parentheses.
   enum SpaceBeforeParensOptions {
     /// Never put a space before opening parentheses.
@@ -355,6 +367,7 @@
            SpaceInEmptyParentheses == R.SpaceInEmptyParentheses &&
            SpacesInContainerLiterals == R.SpacesInContainerLiterals &&
            SpacesInCStyleCastParentheses == R.SpacesInCStyleCastParentheses &&
+           SpacesInParenthesesStyle == R.SpacesInParenthesesStyle && 
            SpaceBeforeParens == R.SpaceBeforeParens &&
            SpaceBeforeAssignmentOperators == R.SpaceBeforeAssignmentOperators &&
            ContinuationIndentWidth == R.ContinuationIndentWidth &&
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp	(revision 204624)
+++ lib/Format/Format.cpp	(working copy)
@@ -84,6 +84,16 @@
 };
 
 template <>
+struct ScalarEnumerationTraits<FormatStyle::ParenthesesSpacingStyle> {
+  static void
+  enumeration(IO &IO,
+              clang::format::FormatStyle::ParenthesesSpacingStyle &Value) {
+    IO.enumCase(Value, "Always", clang::format::FormatStyle::PS_Always);
+    IO.enumCase(Value, "Outermost", clang::format::FormatStyle::PS_Outermost);
+  }
+};
+
+template <>
 struct ScalarEnumerationTraits<FormatStyle::SpaceBeforeParensOptions> {
   static void enumeration(IO &IO,
                           FormatStyle::SpaceBeforeParensOptions &Value) {
@@ -207,6 +217,7 @@
                      Style.SpaceBeforeParens);
     }
     IO.mapOptional("SpaceBeforeParens", Style.SpaceBeforeParens);
+    IO.mapOptional("SpacesInParenthesesStyle", Style.SpacesInParenthesesStyle);
   }
 };
 
@@ -281,6 +292,7 @@
   LLVMStyle.SpaceInEmptyParentheses = false;
   LLVMStyle.SpacesInContainerLiterals = true;
   LLVMStyle.SpacesInCStyleCastParentheses = false;
+  LLVMStyle.SpacesInParenthesesStyle = FormatStyle::PS_Always;
   LLVMStyle.SpaceBeforeParens = FormatStyle::SBPO_ControlStatements;
   LLVMStyle.SpaceBeforeAssignmentOperators = true;
   LLVMStyle.ContinuationIndentWidth = 4;
Index: lib/Format/FormatToken.h
===================================================================
--- lib/Format/FormatToken.h	(revision 204624)
+++ lib/Format/FormatToken.h	(working copy)
@@ -103,7 +103,7 @@
         SplitPenalty(0), LongestObjCSelectorName(0), FakeRParens(0),
         StartsBinaryExpression(false), EndsBinaryExpression(false),
         LastInChainOfCalls(false), PartOfMultiVariableDeclStmt(false),
-        MatchingParen(NULL), Previous(NULL), Next(NULL),
+        ParenNestingLevel(0), MatchingParen(NULL), Previous(NULL), Next(NULL),
         Decision(FD_Unformatted), Finalized(false) {}
 
   /// \brief The \c Token.
@@ -246,6 +246,11 @@
   /// Only set if \c Type == \c TT_StartOfName.
   bool PartOfMultiVariableDeclStmt;
 
+  /// \brief The number of pairs of parentheses containing this pair.
+  ///
+  /// Only set for "(" and ")".
+  unsigned ParenNestingLevel;
+
   bool is(tok::TokenKind Kind) const { return Tok.is(Kind); }
 
   bool isOneOf(tok::TokenKind K1, tok::TokenKind K2) const {
Index: lib/Format/TokenAnnotator.cpp
===================================================================
--- lib/Format/TokenAnnotator.cpp	(revision 204624)
+++ lib/Format/TokenAnnotator.cpp	(working copy)
@@ -16,6 +16,7 @@
 #include "TokenAnnotator.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/Support/Debug.h"
+#include <algorithm>
 
 namespace clang {
 namespace format {
@@ -76,6 +77,12 @@
   bool parseParens(bool LookForDecls = false) {
     if (CurrentToken == NULL)
       return false;
+
+    unsigned NestingLevel = std::count_if(
+        Contexts.begin(), Contexts.end(),
+        [](const Context & C) { return C.ContextKind == tok::l_paren; });
+    CurrentToken->Previous->ParenNestingLevel = NestingLevel;
+
     bool AfterCaret = Contexts.back().CaretFound;
     Contexts.back().CaretFound = false;
 
@@ -154,6 +161,8 @@
         Left->MatchingParen = CurrentToken;
         CurrentToken->MatchingParen = Left;
 
+        CurrentToken->ParenNestingLevel = Left->ParenNestingLevel;
+
         if (StartsObjCMethodExpr) {
           CurrentToken->Type = TT_ObjCMethodExpr;
           if (Contexts.back().FirstObjCSelectorName != NULL) {
@@ -1314,6 +1323,10 @@
     return Left.is(tok::hash);
   if (Left.isOneOf(tok::hashhash, tok::hash))
     return Right.is(tok::hash);
+  if (Style.SpacesInParenthesesStyle == FormatStyle::PS_Outermost &&
+      ((Left.is(tok::l_paren) && Left.ParenNestingLevel != 0) ||
+       (Right.is(tok::r_paren) && Right.ParenNestingLevel != 0)))
+    return false;
   if (Left.is(tok::l_paren) && Right.is(tok::r_paren))
     return Style.SpaceInEmptyParentheses;
   if (Left.is(tok::l_paren) || Right.is(tok::r_paren))
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp	(revision 204624)
+++ unittests/Format/FormatTest.cpp	(working copy)
@@ -7063,19 +7063,19 @@
   FormatStyle Spaces = getLLVMStyle();
 
   Spaces.SpacesInParentheses = true;
-  verifyFormat("call( x, y, z );", Spaces);
+  verifyFormat("call( f( x ), y, z );", Spaces);
   verifyFormat("while ( (bool)1 )\n"
                "  continue;", Spaces);
   verifyFormat("for ( ;; )\n"
                "  continue;", Spaces);
   verifyFormat("if ( true )\n"
                "  f();\n"
-               "else if ( true )\n"
+               "else if ( call( f( x ), 1 / ( 2 + y ), z ) )\n"
                "  f();", Spaces);
   verifyFormat("do {\n"
                "  do_something( (int)i );\n"
                "} while ( something() );", Spaces);
-  verifyFormat("switch ( x ) {\n"
+  verifyFormat("switch ( call( f( x ), y, z ) ) {\n"
                "default:\n"
                "  break;\n"
                "}", Spaces);
@@ -7092,7 +7092,7 @@
 
   Spaces.SpacesInParentheses = false;
   Spaces.SpaceInEmptyParentheses = true;
-  verifyFormat("call(x, y, z);", Spaces);
+  verifyFormat("call(f(x), y, z);", Spaces);
   verifyFormat("call( )", Spaces);
 
   // Run the first set of tests again with
@@ -7103,21 +7103,48 @@
   Spaces.SpaceInEmptyParentheses = true;
   Spaces.SpacesInCStyleCastParentheses = true;
   verifyFormat("call(x, y, z);", Spaces);
+  verifyFormat("call(f(x), y, z);", Spaces);
   verifyFormat("while (( bool )1)\n"
                "  continue;", Spaces);
   verifyFormat("for (;;)\n"
                "  continue;", Spaces);
   verifyFormat("if (true)\n"
                "  f( );\n"
-               "else if (true)\n"
+               "else if (call(f(x), 1 / (2 + y), z))\n"
                "  f( );", Spaces);
   verifyFormat("do {\n"
                "  do_something(( int )i);\n"
                "} while (something( ));", Spaces);
-  verifyFormat("switch (x) {\n"
+  verifyFormat("switch (call(f(x), y, z)) {\n"
                "default:\n"
                "  break;\n"
                "}", Spaces);
+
+  // Run the first set of tests again with
+  // Spaces.SpacesInParentheses = true,
+  // Spaces.SpaceInEmptyParentheses = false,
+  // Spaces.SpacesInCStyleCastParentheses = false and
+  // Spaces.SpacesInParenthesesStyle = Outermost
+  Spaces.SpacesInParentheses = true;
+  Spaces.SpaceInEmptyParentheses = false;
+  Spaces.SpacesInCStyleCastParentheses = false;
+  Spaces.SpacesInParenthesesStyle = FormatStyle::PS_Outermost;
+  verifyFormat("call( f(x), y, z );", Spaces);
+  verifyFormat("while ( (bool)1 )\n"
+               "  continue;", Spaces);
+  verifyFormat("for ( ;; )\n"
+               "  continue;", Spaces);
+  verifyFormat("if ( true )\n"
+               "  f();\n"
+               "else if ( call(f(x), 1 / (2 + y), z) )\n"
+               "  f();", Spaces);
+  verifyFormat("do {\n"
+               "  do_something( (int)i );\n"
+               "} while ( something() );", Spaces);
+  verifyFormat("switch ( call(f(x), y, z) ) {\n"
+               "default:\n"
+               "  break;\n"
+               "}", Spaces);
 }
 
 TEST_F(FormatTest, ConfigurableSpaceBeforeAssignmentOperators) {
@@ -7587,6 +7614,12 @@
               FormatStyle::NI_Inner);
   CHECK_PARSE("NamespaceIndentation: All", NamespaceIndentation,
               FormatStyle::NI_All);
+
+  Style.SpacesInParenthesesStyle = FormatStyle::PS_Outermost;
+  CHECK_PARSE("SpacesInParenthesesStyle: Always", SpacesInParenthesesStyle,
+              FormatStyle::PS_Always);
+  CHECK_PARSE("SpacesInParenthesesStyle: Outermost", SpacesInParenthesesStyle,
+              FormatStyle::PS_Outermost);
 }
 
 TEST_F(FormatTest, ParsesConfigurationWithLanguages) {


_______________________________________________
cfe-dev mailing list
cfe-dev@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic