r329916 - [clang-format] Always indent wrapped Objective-C selector names

Ben Hamilton via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 12 08:11:49 PDT 2018


Author: benhamilton
Date: Thu Apr 12 08:11:48 2018
New Revision: 329916

URL: http://llvm.org/viewvc/llvm-project?rev=329916&view=rev
Log:
[clang-format] Always indent wrapped Objective-C selector names

Summary:
Currently, indentation of Objective-C method names which are wrapped
onto the next line due to a long return type is controlled by the
style option `IndentWrappedFunctionNames`.

This diff changes the behavior so we always indent wrapped Objective-C
selector names.

NOTE: I partially reverted https://github.com/llvm-mirror/clang/commit/6159c0fbd1876c7f5f984b4830c664cc78f16e2e / rL242484, as it was causing wrapped selectors to be double-indented. Its tests in FormatTestObjC.cpp still pass.

Test Plan: Tests updated. Ran tests with:
  % make -j12 FormatTests && ./tools/clang/unittests/Format/FormatTests

Reviewers: djasper, jolesiak, stephanemoore, thakis

Reviewed By: djasper

Subscribers: stephanemoore, klimek, cfe-commits

Differential Revision: https://reviews.llvm.org/D45004

Modified:
    cfe/trunk/lib/Format/ContinuationIndenter.cpp
    cfe/trunk/unittests/Format/FormatTest.cpp
    cfe/trunk/unittests/Format/FormatTestObjC.cpp

Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=329916&r1=329915&r2=329916&view=diff
==============================================================================
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Thu Apr 12 08:11:48 2018
@@ -26,6 +26,13 @@
 namespace clang {
 namespace format {
 
+// Returns true if a TT_SelectorName should be indented when wrapped,
+// false otherwise.
+static bool shouldIndentWrappedSelectorName(const FormatStyle &Style,
+                                            LineType LineType) {
+  return Style.IndentWrappedFunctionNames || LineType == LT_ObjCMethodDecl;
+}
+
 // Returns the length of everything up to the first possible line break after
 // the ), ], } or > matching \c Tok.
 static unsigned getLengthToMatchingParen(const FormatToken &Tok) {
@@ -698,7 +705,7 @@ unsigned ContinuationIndenter::addTokenO
         State.Stack.back().AlignColons = false;
       } else {
         State.Stack.back().ColonPos =
-            (Style.IndentWrappedFunctionNames
+            (shouldIndentWrappedSelectorName(Style, State.Line->Type)
                  ? std::max(State.Stack.back().Indent,
                             State.FirstIndent + Style.ContinuationIndentWidth)
                  : State.Stack.back().Indent) +
@@ -897,7 +904,7 @@ unsigned ContinuationIndenter::getNewLin
   if (NextNonComment->is(TT_SelectorName)) {
     if (!State.Stack.back().ObjCSelectorNameFound) {
       unsigned MinIndent = State.Stack.back().Indent;
-      if (Style.IndentWrappedFunctionNames)
+      if (shouldIndentWrappedSelectorName(Style, State.Line->Type))
         MinIndent = std::max(MinIndent,
                              State.FirstIndent + Style.ContinuationIndentWidth);
       // If LongestObjCSelectorName is 0, we are indenting the first
@@ -1000,13 +1007,8 @@ unsigned ContinuationIndenter::moveState
   if (Current.isMemberAccess())
     State.Stack.back().StartOfFunctionCall =
         !Current.NextOperator ? 0 : State.Column;
-  if (Current.is(TT_SelectorName)) {
+  if (Current.is(TT_SelectorName))
     State.Stack.back().ObjCSelectorNameFound = true;
-    if (Style.IndentWrappedFunctionNames) {
-      State.Stack.back().Indent =
-          State.FirstIndent + Style.ContinuationIndentWidth;
-    }
-  }
   if (Current.is(TT_CtorInitializerColon) &&
       Style.BreakConstructorInitializers != FormatStyle::BCIS_AfterColon) {
     // Indent 2 from the column, so:

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=329916&r1=329915&r2=329916&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Apr 12 08:11:48 2018
@@ -7678,16 +7678,18 @@ TEST_F(FormatTest, FormatForObjectiveCMe
 
   // When the function name has to be wrapped.
   FormatStyle Style = getLLVMStyle();
+  // ObjC ignores IndentWrappedFunctionNames when wrapping methods
+  // and always indents instead.
   Style.IndentWrappedFunctionNames = false;
   verifyFormat("- (SomeLooooooooooooooooooooongType *)\n"
-               "veryLooooooooooongName:(NSString)aaaaaaaaaaaaaa\n"
-               "           anotherName:(NSString)bbbbbbbbbbbbbb {\n"
+               "    veryLooooooooooongName:(NSString)aaaaaaaaaaaaaa\n"
+               "               anotherName:(NSString)bbbbbbbbbbbbbb {\n"
                "}",
                Style);
   Style.IndentWrappedFunctionNames = true;
   verifyFormat("- (SomeLooooooooooooooooooooongType *)\n"
-               "    veryLooooooooooongName:(NSString)aaaaaaaaaaaaaa\n"
-               "               anotherName:(NSString)bbbbbbbbbbbbbb {\n"
+               "    veryLooooooooooongName:(NSString)cccccccccccccc\n"
+               "               anotherName:(NSString)dddddddddddddd {\n"
                "}",
                Style);
 

Modified: cfe/trunk/unittests/Format/FormatTestObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestObjC.cpp?rev=329916&r1=329915&r2=329916&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp Thu Apr 12 08:11:48 2018
@@ -536,28 +536,25 @@ TEST_F(FormatTestObjC, FormatObjCMethodD
                "            ofSize:(size_t)height\n"
                "                  :(size_t)width;");
   Style.ColumnLimit = 40;
-  // Make sure selectors with 0, 1, or more arguments are not indented
-  // when IndentWrappedFunctionNames is false.
-  Style.IndentWrappedFunctionNames = false;
+  // Make sure selectors with 0, 1, or more arguments are indented when wrapped.
   verifyFormat("- (aaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
-               "aaaaaaaaaaaaaaaaaaaaaaaaaaaa;\n");
+               "    aaaaaaaaaaaaaaaaaaaaaaaaaaaa;\n");
   verifyFormat("- (aaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
-               "aaaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a;\n");
+               "    aaaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a;\n");
   verifyFormat("- (aaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
-               "aaaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a\n"
-               "aaaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a;\n");
+               "    aaaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a\n"
+               "    aaaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a;\n");
   verifyFormat("- (aaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
-               " aaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a\n"
-               "aaaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a;\n");
+               "     aaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a\n"
+               "    aaaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a;\n");
   verifyFormat("- (aaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
-               "aaaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a\n"
-               " aaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a;\n");
+               "    aaaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a\n"
+               "     aaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a;\n");
 
   // Continuation indent width should win over aligning colons if the function
   // name is long.
   Style = getGoogleStyle(FormatStyle::LK_ObjC);
   Style.ColumnLimit = 40;
-  Style.IndentWrappedFunctionNames = true;
   verifyFormat("- (void)shortf:(GTMFoo *)theFoo\n"
                "    dontAlignNamef:(NSRect)theRect {\n"
                "}");
@@ -567,22 +564,6 @@ TEST_F(FormatTestObjC, FormatObjCMethodD
                "       aShortf:(NSRect)theRect {\n"
                "}");
 
-  // Make sure selectors with 0, 1, or more arguments are indented
-  // when IndentWrappedFunctionNames is true.
-  verifyFormat("- (aaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
-               "    aaaaaaaaaaaaaaaaaaaaaaaaaaaa;\n");
-  verifyFormat("- (aaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
-               "    aaaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a;\n");
-  verifyFormat("- (aaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
-               "    aaaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a\n"
-               "    aaaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a;\n");
-  verifyFormat("- (aaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
-               "     aaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a\n"
-               "    aaaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a;\n");
-  verifyFormat("- (aaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
-               "    aaaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a\n"
-               "     aaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a;\n");
-
   // Format pairs correctly.
   Style.ColumnLimit = 80;
   verifyFormat("- (void)drawRectOn:(id)surface\n"




More information about the cfe-commits mailing list